Menu

#58 Run cppcheck on quazip

v1.0_(example)
open
nobody
None
5
2015-01-19
2015-01-16
legalize
No

If you run cppcheck on quazip, it tells you about uninitialized members in c'tors, etc. These are things that should be fixed in the library.

Discussion

  • Sergey A. Tachenov

    I just did cppcheck -f ., but it only printed out a few possible null pointers, and even that outside of QuaZIP, in Minizip.

    Which options do you suggest to use?

     
  • legalize

    legalize - 2015-01-16

    Run with --enable=warning to see that your classes have POD members that are uninitialized. I see these on an older version of quazip, but it looks like the same problem exists in the current version (from browsing the code). POD members are not initialized to zero in a class unless they are explicitly initialized, so in a release build on Windows, you'll get POD members filled with junk.

     
  • legalize

    legalize - 2015-01-16

    and Thanks for the library :)

     
  • Sergey A. Tachenov

    I don't seem to have the warning parameter. Available IDs are style, performance, portability, information, unusedFunction and missingInclude. Which version of cppcheck do you use?

     
  • legalize

    legalize - 2015-01-16

    I'm using the latest version. If you are on ubuntu and just did apt-get for cppcheck, you can get one that is up to 3 years old. Latest is 1.68 and you really want to use the latest because they improve the checks all the time.

     
  • Sergey A. Tachenov

    OK, did that, still got only few minor style and performance warnings. Well, I did get some uninitialized members, but all of them inside qhash.h, which is kind of outside my jurisdiction.

    I am also a bit surprised that you have found uninitialized members somewhere, because I am quite picky about that sort of thing. The only thing that comes to mind are constructorless C-style structures like QuaZipFileInfo (in fact, I can't think of any other). But those are initialized by passing pointers to them to some functions anyway. Which is kind of bad design, but I can't just change it right now because of the compatibility issues. It should not cause any problems, though, unless someone creates such a structure, and then starts to use it without ever calling any function to fill it.

     
  • legalize

    legalize - 2015-01-19

    Well cool, perhaps you fixed them in the latest version of the library. I was seeing these:

    quazip-0.2.3\quazip\quazipfile.cpp(37): warning : uninitMemberVar Member variable 'QuaZipFile::caseSensitivity' is not initialized in the constructor.
    quazip-0.2.3\quazip\quazipfile.cpp(37): warning : uninitMemberVar Member variable 'QuaZipFile::raw' is not initialized in the constructor.
    quazip-0.2.3\quazip\quazipfile.cpp(42): warning : uninitMemberVar Member variable 'QuaZipFile::caseSensitivity' is not initialized in the constructor.
    quazip-0.2.3\quazip\quazipfile.cpp(42): warning : uninitMemberVar Member variable 'QuaZipFile::raw' is not initialized in the constructor.
    quazip-0.2.3\quazip\quazipfile.cpp(47): warning : uninitMemberVar Member variable 'QuaZipFile::caseSensitivity' is not initialized in the constructor.
    quazip-0.2.3\quazip\quazipfile.cpp(47): warning : uninitMemberVar Member variable 'QuaZipFile::raw' is not initialized in the constructor.
    quazip-0.2.3\quazip\quazipfile.cpp(54): warning : uninitMemberVar Member variable 'QuaZipFile::raw' is not initialized in the constructor.
    quazip-0.2.3\quazip\quazipfile.cpp(64): warning : uninitMemberVar Member variable 'QuaZipFile::caseSensitivity' is not initialized in the constructor.
    quazip-0.2.3\quazip\quazipfile.cpp(64): warning : uninitMemberVar Member variable 'QuaZipFile::raw' is not initialized in the constructor.

    I think you've replaced QuaZipFile with QuaZipFileInfo from what I was seeing when browsing the code. If you think these are false positives, you can suppress then with inline comments before the offending line.

    Thanks again for the library.

     
  • Sergey A. Tachenov

    Oh, these... I totally forgot about them. These were initially in QuaZipFile, but then got moved to the pimpl class. Since it is only used internally, I wasn't too careful about it. The same goes for QuaZipPrivate, I guess.

    The problem is, cppcheck doesn't seem to detect it anyway. I'm using the latest cppcheck (1.68), tried even --enable=all, but all I get is the lack of the copy constructor for the pimpl classes (OK, good thing to disable copying explicitly) and a weird message about the member q being initialized with itself, even though it is actually initialized by the formal parameter with the same name.

    I'll go over the pimple classes and add initialization where necessary.

    As for QuaZipFileInfo, it's an entirely different thing. That's one of the C-style structs that don't even have constructors. These aren't exactly false positives, it's just I don't want to break the ABI by fixing them.

     
MongoDB Logo MongoDB