Bugfix/parsing wrong vector yields segfault #1
Conversation
Co-authored-by: sunfish <[email protected]>
Previously the documentation mistakenly stated the argument `h` was used for getting the `diagnostic_period` parameter. I fixed the documentation.
The data returned from a socket is `bytes`. So to properly deal with this data we'd have to concatenate as bytes and decode to string at the end. The old (wrong) way of concatenating results in some stray `'b'` characters showing up in the drive names.
MCFurry
requested review from
Rayman,
rokusottervanger,
JosjaG and
LarsJanssenTUe
| @@ -278,7 +278,8 @@ void Aggregator::publishData() | |||
| { | |||
| diag_array.status.push_back(*processed_other[i]); | |||
|
|
|||
| const uint depth = static_cast<uint>(std::count(processed[i]->name.begin(), processed[i]->name.end(), '/')); | |||
There was a problem hiding this comment.
Here is the actual fix, the rest is a sync with the forked remote repo
There was a problem hiding this comment.
TBH; I don't know what the implications of this change are down the line since I don't know the inner workings of this code. But I agree that this looks like a bug.
There was a problem hiding this comment.
Now something useful happens since we access elements from the vector we are actually looping through. Before we didn't and that could potentially throw a segfault...
So this change prevents a segfault and also makes sure we actually report something useful ;-)
| @@ -278,7 +278,8 @@ void Aggregator::publishData() | |||
| { | |||
| diag_array.status.push_back(*processed_other[i]); | |||
|
|
|||
| const uint depth = static_cast<uint>(std::count(processed[i]->name.begin(), processed[i]->name.end(), '/')); | |||
There was a problem hiding this comment.
TBH; I don't know what the implications of this change are down the line since I don't know the inner workings of this code. But I agree that this looks like a bug.
|
Are you planning on opening a PR for this to the upstream repo? |
Already pending for quite a while: ros#197 |
Aha! So it seems we did introduce a segfault ourselves!