kwiver icon indicating copy to clipboard operation
kwiver copied to clipboard

Potential seg-fault in initialize_cameras_landmarks

Open OwenMcGee opened this issue 3 years ago • 1 comments

In arrows/mvg/algo/initialize_cameras_landmarks.h, the initialize function gives a default value for the constraints parameter as nullptr. However, as far as I can tell actually calling the function with the default parameter will always result in a crash.

The constraints parameter will be passed into initialize_keyframes, which will then pass it into metadata_centric_keyframe_initialization, which will then reach reach line 2440, and crash.

I'm unsure if the solution for this is to simply remove the default value for the constraints parameter, or if additional checks and cases for a value of nullptr are needed throughout the code called by the function. I believe the default value for the parameter was ported over from the arrows/mvg/algo/initialize_cameras_landmarks_basic.h file.

OwenMcGee avatar Jun 29 '22 19:06 OwenMcGee

Additional checks are needed. Anywhere we use pointer or shared_pointer we need to check that the pointer is not null before dereferencing it unless there was already an earlier check guaranteeing it was not null. The contraints are intended to be optional so null is valid input. Line 2440 and other places like it should start like this

if(constraints &&
   constraints->get_...

mleotta avatar Jun 30 '22 13:06 mleotta