Last Updated on December 27, 2013 by nghiaho12
Every now and then when I’m free and bored, I do a daily git fetch on the OpenCV branch to keep up to date with the latest and greatest. One of my favourite things to do is run cppcheck on the source code to see what new bugs have appeared (I should find a new hobby). For those who don’t know what cppcheck is, it is an open source static code analyzer for C/C++. It will try to find coding mistakes eg. using an uinitialised variable, memory leak, and more. In other words, it is a must have tool, use it, and use it often.
To demonstrate its effectiveness, I just updated my OpenCV 2.4 branch as of 15th Dec 2013 and ran cppcheck on it, resulting in:
$ cppcheck -q -j 4 . [features2d/src/orb.cpp:179]: (error) Uninitialized variable: ix [features2d/src/orb.cpp:199]: (error) Uninitialized variable: ix [features2d/src/orb.cpp:235]: (error) Uninitialized variable: ix [features2d/src/orb.cpp:179]: (error) Uninitialized variable: iy [features2d/src/orb.cpp:199]: (error) Uninitialized variable: iy [features2d/src/orb.cpp:235]: (error) Uninitialized variable: iy [imgproc/src/color.cpp:773]: (error) Array 'coeffs[2]' accessed at index 2, which is out of bounds. [imgproc/test/test_cvtyuv.cpp:590]: (style) Class 'ConversionYUV' is unsafe, 'ConversionYUV::yuvReader_' can leak by wrong usage. [imgproc/test/test_cvtyuv.cpp:591]: (style) Class 'ConversionYUV' is unsafe, 'ConversionYUV::yuvWriter_' can leak by wrong usage. [imgproc/test/test_cvtyuv.cpp:592]: (style) Class 'ConversionYUV' is unsafe, 'ConversionYUV::rgbReader_' can leak by wrong usage. [imgproc/test/test_cvtyuv.cpp:593]: (style) Class 'ConversionYUV' is unsafe, 'ConversionYUV::rgbWriter_' can leak by wrong usage. [imgproc/test/test_cvtyuv.cpp:594]: (style) Class 'ConversionYUV' is unsafe, 'ConversionYUV::grayWriter_' can leak by wrong usage. [legacy/src/calibfilter.cpp:725]: (error) Resource leak: f [legacy/src/epilines.cpp:3005]: (error) Memory leak: objectPoints_64d [legacy/src/epilines.cpp:3005]: (error) Memory leak: rotMatrs1_64d [legacy/src/epilines.cpp:3005]: (error) Memory leak: rotMatrs2_64d [legacy/src/epilines.cpp:3005]: (error) Memory leak: transVects1_64d [legacy/src/epilines.cpp:3005]: (error) Memory leak: transVects2_64d [legacy/src/vecfacetracking.cpp] -> [legacy/src/vecfacetracking.cpp:670]: (error) Internal error. Token::Match called with varid 0. Please report this to Cppcheck developers [ml/src/svm.cpp:1338]: (error) Possible null pointer dereference: df [objdetect/src/hog.cpp:2564]: (error) Resource leak: modelfl : (error) Division by zero. [ts/src/ts_gtest.cpp:7518]: (error) Address of local auto-variable assigned to a function parameter. [ts/src/ts_gtest.cpp:7518]: (error) Uninitialized variable: dummy [ts/src/ts_gtest.cpp:7525]: (error) Uninitialized variable: dummy
cppcheck -q – j 4, calls cppcheck in quiet mode (only reporting errors) using 4 threads.
The orb.cpp errors are fairly new. The others have been there for a while because I didn’t bother sending a pull request for the legacy functions, because well, they’re legacy. But I should.
The error in tvl1flow.cpp is a false alert, which I’ve reported and has been resolved. Basically, someone used a variable called div, which cppcheck confuses with the stdlib.h div function, because they both have the same parameter count and type, naughty.
vecfecetracking.cpp is an interesting one, cppcheck basically failed for some unknown reason. Though it rarely occurs. I should report that to the cppcheck team.
hog.cpp reports a resource leak because an fopen was called prior but the function calls a throw if something goes wrong without calling fclose, as shown below:
void HOGDescriptor::readALTModel(std::string modelfile)
{
// read model from SVMlight format..
FILE *modelfl;
if ((modelfl = fopen(modelfile.c_str(), "rb")) == NULL)
{
std::string eerr("file not exist");
std::string efile(__FILE__);
std::string efunc(__FUNCTION__);
throw Exception(CV_StsError, eerr, efile, efunc, __LINE__);
}
char version_buffer[10];
if (!fread (&version_buffer,sizeof(char),10,modelfl))
{
std::string eerr("version?");
std::string efile(__FILE__);
std::string efunc(__FUNCTION__);
// doing an fclose(modefl) would fix the error
throw Exception(CV_StsError, eerr, efile, efunc, __LINE__);
}
With holidays coming up in a week I’ll probably get off my lazy ass and submit some more fixes. What I find funny is the fact that I’ve been seeing the same errors for the past months (years even?). It seems to suggest cppcheck needs to be publicised more and possibly become part of the code submission guideline. I feel like I’m the only one running cppcheck on OpenCV.
UPDATE:
Running with the latest cppcheck 1.62 produces less false alerts than before (was running 1.61). I now get:
[highgui/src/cap_images.cpp:197]: (warning) %u in format string (no. 1) requires 'unsigned int *' but the argument type is 'int *'. [imgproc/test/test_cvtyuv.cpp:590]: (style) Class 'ConversionYUV' is unsafe, 'ConversionYUV::yuvReader_' can leak by wrong usage. [imgproc/test/test_cvtyuv.cpp:591]: (style) Class 'ConversionYUV' is unsafe, 'ConversionYUV::yuvWriter_' can leak by wrong usage. [imgproc/test/test_cvtyuv.cpp:592]: (style) Class 'ConversionYUV' is unsafe, 'ConversionYUV::rgbReader_' can leak by wrong usage. [imgproc/test/test_cvtyuv.cpp:593]: (style) Class 'ConversionYUV' is unsafe, 'ConversionYUV::rgbWriter_' can leak by wrong usage. [imgproc/test/test_cvtyuv.cpp:594]: (style) Class 'ConversionYUV' is unsafe, 'ConversionYUV::grayWriter_' can leak by wrong usage. [legacy/src/calibfilter.cpp:725]: (error) Resource leak: f [legacy/src/epilines.cpp:3005]: (error) Memory leak: objectPoints_64d [legacy/src/epilines.cpp:3005]: (error) Memory leak: rotMatrs1_64d [legacy/src/epilines.cpp:3005]: (error) Memory leak: rotMatrs2_64d [legacy/src/epilines.cpp:3005]: (error) Memory leak: transVects1_64d [legacy/src/epilines.cpp:3005]: (error) Memory leak: transVects2_64d [ml/src/svm.cpp:1338]: (error) Possible null pointer dereference: df [objdetect/src/hog.cpp:2564]: (error) Resource leak: modelfl [ts/src/ts_gtest.cpp:7518]: (error) Address of local auto-variable assigned to a function parameter. [ts/src/ts_gtest.cpp:7518]: (error) Uninitialized variable: dummy [ts/src/ts_gtest.cpp:7525]: (error) Uninitialized variable: dummy