Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[segmentation] Improve error handling in OrganizedMultiPlaneSegmentation::segment #3855

Closed
daniel-packard opened this issue Apr 3, 2020 · 1 comment · Fixed by #3861
Closed
Assignees

Comments

@daniel-packard
Copy link
Contributor

daniel-packard commented Apr 3, 2020

When using OrganizedMultiPlaneSegmentation, if the user does not specify both an input cloud and normals they will experience a segfault. This is because those pointers are dereferenced on line 95, without first checking if the pointers are valid:

I suggest adding additional guard clauses before line 95 to check for valid inputs.

  if ( !input_ )
  {
    PCL_ERROR ("[pcl::%s::segment] You must provide a valid input cloud.\n",
               getClassName ().c_str ());
    return;
  }

  if ( !normals_ )
  {
    PCL_ERROR ("[pcl::%s::segment] You must provide a valid input normals.\n",
               getClassName ().c_str ());
    return;
  }

Alternatively it could throw an exception... but I figured it's better to match the other error handling inside this file.

I'm happy to open a PR if one or the other approach sounds good

@daniel-packard daniel-packard added the status: triage Labels incomplete label Apr 3, 2020
@SergioRAgostinho SergioRAgostinho added kind: proposal Type of issue module: segmentation and removed status: triage Labels incomplete labels Apr 3, 2020
@SergioRAgostinho
Copy link
Member

Sounds good. Go for it 👍 Thanks!

@kunaltyagi kunaltyagi changed the title [custom] Improve error handling in OrganizedMultiPlaneSegmentation::segment [segmentation] Improve error handling in OrganizedMultiPlaneSegmentation::segment Apr 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants