Fix division by 0 width in PointCloud structured assign#5113
Merged
mvieth merged 2 commits intoPointCloudLibrary:masterfrom Jan 2, 2022
Merged
Fix division by 0 width in PointCloud structured assign#5113mvieth merged 2 commits intoPointCloudLibrary:masterfrom
PointCloud structured assign#5113mvieth merged 2 commits intoPointCloudLibrary:masterfrom
Conversation
kunaltyagi
requested changes
Dec 30, 2021
Member
kunaltyagi
left a comment
kunaltyagi
left a comment
There was a problem hiding this comment.
What do you think about handling the 0 case specifically without throwing?
If use input new_width is 0, we call PCL_WARN and call the assign which doesn't take a user input for width
| { | ||
| points.assign(std::move(ilist)); | ||
| width = new_width; | ||
| if (width == 0) |
Member
There was a problem hiding this comment.
Let's move this check to the top (in both places) and test new_width instead.
mvieth
reviewed
Dec 31, 2021
kunaltyagi
previously approved these changes
Dec 31, 2021
larshg
previously approved these changes
Dec 31, 2021
mvieth
reviewed
Jan 1, 2022
| { | ||
| if (new_width == 0) { | ||
| PCL_WARN("Assignment with new_width equal to 0," | ||
| "setting width to size of the cloud and height to 1"); |
Member
There was a problem hiding this comment.
Suggested change
| "setting width to size of the cloud and height to 1"); | |
| "setting width to size of the cloud and height to 1\n"); |
Same below, otherwise looks good
kunaltyagi
changed the title
PointCloud structured assign
Co-authored-by: Markus Vieth <39675748+mvieth@users.noreply.github.com>
Qannaf
approved these changes
Jan 2, 2022
kunaltyagi
approved these changes
Jan 2, 2022
larshg
approved these changes
Jan 2, 2022
themightyoarfish
pushed a commit
to themightyoarfish/pcl
that referenced
this pull request
Jan 5, 2022
…udLibrary#5113) * Check for width in assign(). * Update common/include/pcl/point_cloud.h Co-authored-by: Markus Vieth <39675748+mvieth@users.noreply.github.com> Co-authored-by: Markus Vieth <39675748+mvieth@users.noreply.github.com>
3 tasks
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds some checks to prevent divisions by zero when using the assign operator