search

hengli / camodocal

CamOdoCal: Automatic Intrinsic and Extrinsic Calibration of a Rig with Multiple Generic Cameras and Odometry
Other
1.18k stars 390 forks source link

Logic Errors found by Xcode Static Analysis #16

Closed ahundt closed 9 years ago

ahundt commented 9 years ago

There are a number of logic errors found by static analysis that lead to junk data and calculations in the code. Some of them are in functions that look pretty important, I included the most important ones below. 

Logic error Group
camodocal/src/dbow2/DUtilsCV/Transformations.cpp:125:26: The left operand of '*' is a garbage value
camodocal/src/dbow2/DUtilsCV/Transformations.cpp:126:26: The right operand of '*' is a garbage value
camodocal/src/dbow2/DUtilsCV/Transformations.cpp:127:26: The right operand of '*' is a garbage value
Logic error Group
camodocal/src/camera_models/CostFunctionFactory.cc:396:9: Assigned value is garbage or undefined (within a call to 'spaceToPlane')
Logic error Group
camodocal/src/infrastr_calib/InfrastructureCalibration.cc:1047:9: The result of the '>>' expression is undefined (within a call to 'random_shuffle')
Logic error Group
camodocal/src/examples/extrinsic_calib.cc:329:38: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage (within a call to 'rotation')
camodocal/src/dbow2/DUtils/Profiler.cpp:207:15: The left operand of '*' is a garbage value (mean)
camodocal/src/gpl/gpl.cc:853:39: The left operand of '-' is a garbage value (ZoneLetter)
Logic error Group
camodocal/src/ceres-solver/internal/ceres/corrector.cc:108:18: Array access (from variable 'residuals') results in a null pointer dereference
camodocal/src/ceres-solver/internal/ceres/corrector.cc:133:24: Array access (from variable 'jacobian') results in a null pointer dereference
camodocal/src/ceres-solver/internal/ceres/corrector.cc:133:53: Array access (from variable 'residuals') results in a null pointer dereference
camodocal/src/ceres-solver/internal/ceres/problem_impl.cc:530:3: Called C++ object pointer is null (within a call to 'SetParameterization')
camodocal/src/ceres-solver/internal/ceres/visibility_based_preconditioner.cc:404:17: Access to field 'values' results in a dereference of a null pointer (loaded from variable 'cell_info')
Memory Error Group
camodocal/src/sparse_graph/SparseGraph.cc:1425:5: Memory allocated by 'new[]' should be deallocated by 'delete[]', not 'delete'
Logic error Group
camodocal/src/visual_odometry/SlidingWindowBA.cc:940:9: The result of the '>>' expression is undefined (within a call to 'random_shuffle')

Some of these are from ceres, so I don't know if it may be worthwhile to update the version.

There are also quite a few "dead stores", or values that are set or defined but never read, they just eat up CPU/memory and are less critical so I left them out to keep the scope of problems to the important ones for this issue.

hengli commented 9 years ago

Thanks @ahundt for doing a static code analysis. I am leaving the DBoW2 code unchanged, as it is a 3rd-party library. For Ceres, yes, we should update the version as the currently used version differs substantially from the latest version. However, I will KIV this as I do not have time, and CamOdoCal works fine with the currently used version. I fixed the memory deallocation issue in camodocal/src/sparse_graph/SparseGraph.cc. I checked the rest of the errors, and I think they are not valid, as the highlighted code seems correct to me.