Run cppcheck in Windows and linux configuration successfully on the s…

[cfehse]

Scope

The scope of this PR is to integrate static code checks with Cppcheck. This integration will be done in two steps:

1. Determine a sensible Cppcheck configuration (as Cppcheck GUI projects) and initially change sources to remove any warning found with this configuration. This configuration should be able to run against an unconfigured source tree.
2. Integrate the Cppcheck checks into the cmake build process itself to be run as part of the configured build process.

What has changed?

Source Files:

CMakeLists.txt scantool/CMakeLists.txt

• Changed the minimum required Cmake version to 3.10 to be able to use the build-in cppcheck integration.
• Added The Cppcheck target to the build process. The analysis will now run before any compilation of a source file.
• The Cppcheck integration is optional, which means if Cppcheck is not found in during Cmake configuration the Cppcheck target will not be created.
• The analysis target is configured to fail if any warnings are found meaning that the software can only be build successfully if warning free.

cppcheck\win32.cppcheck cppcheck\linux.cppcheck cppcheck\run-cppckeck.bat

• Moved the central project file from the root directory into the subdirectory cppcheck and split it into two platform specific configuration. This has the advantage that the sources will be tested more closely to the actual platform configured build process.
• Added defines to enable the checks to test as much source code as possible.
• The unsignedLessThanZero warning was added to the exclude list. This warning can be addressed in an later PR/commit to reduce the number of changed files for this PR.
• I included a Windows batch file to run both test from the command line for convenience.

scantool/diag.h scantool/diag_os.h cconf.h.in

• Added the combination of defines (defined(_WIN32) || defined(__WIN32__) || defined(WIN32)) to define the Windows platform. This is done to unify the Windows detection in the header files and therefore only the _WIN32 define must be included in the Cppcheck project file.

scantool/diag_l2_iso14230.c

• Removed a nullPointerRedundantCheck warning. This change should be reviewed.
• Removed a unreadVariable warning by moving the msg.data = data; to make the intention of the code clear to Cppcheck.

scantool/diag_l2_saej1979.c

• Removed a unreadVariable warning by moving the msg.data = data; to make the intention of the code clear to Cppcheck.

scantool/scantool_cli.c

• Replaced calls to asctime() with equivalent calls to the strftime() function to remove an obsolete warning.
• Removed an uninitializedVariable warning caused by some defines in the code.

scantool/diag_os_win.c

• Corrected a portability warning issued for the Borland compiler environment because the formatter was signed and the parameter was unsigned (LONGLONG).

scantool/diag_cfg.c

• Removed an uninitializedVariable warning issued for the Borland bcc32x/bcc64 compiler environment caused by the assert in the default branch of the switch statement.

Tested Environments:

All changes where tested using these environments:

Cppcheck:

• Cppcheck 2.3 on Windows
• Cppcheck 1.86 on Debian (Limited tests because the command line version requires version 1.90 or later to be able to use the Cppcheck GUI project file format. Currently my Debian 10 systems have no Cppcheck 1.90 or better prebuild packages available)

Compiler/Build Environment:

• Visual Studio 2019 (x86/x64) (integrated cmake and cmake 3.19.3)
• Visual Studio Code on Windows (MinGW, MinGW64, BCC, MSVC) (cmake 3.10.0 and higher)
• Visual Studio Code remote on WSL (cmake 3.13.4 default installation Debian)
• Debian (cmake 3.13.4 default installation Debian)

Perhaps you like the changes and accept this merge request.

Thanks!

@fenugrec @nsajko

[fenugrec]

Wow, thanks a lot for not only implementing this, but addressing some of those warnings !!

[cfehse]

@fenugrec

I implemented the Cmake integration for the Cppcheck analysis. This point are to be discussed from my point of view:

• Changed the minimum required Cmake version to 3.10 to be able to use the build-in cppcheck integration.

This can be solved differently by checking the cmake version before integration Cppcheck

• Added The Cppcheck target to the build process. The analysis will now run before any compilation of a source file.

This can be implemented differently for example by having a cmake configuration option to enable the checks.

• The analysis target is configured to fail if any warnings are found meaning that the software can only be build successfully if warning free.

This can also implemented differently to have only informative checks.

What are your options about this topics?

[fenugrec]

• Changed the minimum required Cmake version to 3.10 to be able to use the build-in cppcheck integration.

I think 3.10 is a reasonable requirement. Even ubuntu 18.04 has 3.10; if someone runs into problems because their distro doesn't have at least 3.10, we can do something different.

This can be implemented differently for example by having a cmake configuration option to enable the checks.

Hmm. On my system cppcheck is reasonably fast, but perhaps an option (default-enabled) would be nice, if it can be done without you ripping out all your additions and starting over? Otherwise I don't mind manually setting cppcheck to "NOTFOUND" to disable it.

• The analysis target is configured to fail if any warnings are found

Hehe, that's strong, but I think we have acceptable suppressions for this not to be too severe. I assume if cppcheck isn't found (or set to "NOTFOUND"), the analysis target is ignored ? (i.e. build can still succeed )

[cfehse]

Hmm. On my system cppcheck is reasonably fast, but perhaps an option (default-enabled) would be nice, if it can be done without you ripping out all your additions and starting over? Otherwise I don't mind manually setting cppcheck to "NOTFOUND" to disable it.

This can be done but there is a caveat: Cmake treats the CMAKE_CPPCHECK like the CMAKE_COMPILER variables. Meaning you cannot change it after the build directory has been created. So a change of that new option after the build directory is initially configured results in a not working build environment. This seems a bit of a flaw in Cmake (or is intended by design?). Let's leave the mechanism as is.

[cfehse]

I assume if cppcheck isn't found (or set to "NOTFOUND"), the analysis target is ignored ? (i.e. build can still succeed )

Yes. If cppcheck is not found during the initial generation of the build directory the analysis targets are not created. To create them after the initial configuration the build directory must be removed.

[cfehse]

I added a corrected Embarcadero platform definition file to the cmake directory. This is a bit out of place here but the platform file which comes with the C++ Builder Community edition does not work correctly with all compilers so I corrected the file.

[fenugrec]

I added a corrected Embarcadero platform definition file to the cmake directory.

Ok. Let me know when you're ready for this to be merged! (if you have a chance, can you check your whitespace - there seems to be one or two "missing newlines", and a bit of trailing space on a few lines)

[cfehse]

Let me know when you're ready for this to be merged!

This PR is ready

(if you have a chance, can you check your whitespace - there seems to be one or two "missing newlines", and a bit of trailing space on a few lines)

Do you have an example in which file the formatting is off?

[fenugrec]

Do you have an example in which file the formatting is off?

It was minor EOL whitespace mostly. Fixed + rebased + merged , thanks !

[cfehse]

My pleasure...