Use CMake's own Cppcheck mechanism (DO NOT MERGE)

[mikkoi]

Enable only when CMAKE_BUILD_TYPE is Debug.
Thus forcing the developer to see the output
if cppcheck program is available.

Signed-off-by: Mikko Johannes Koivunalho [email protected]

[mikkoi]

Travis needs to install 'cppcheck' at the beginning of the run. Example: http://cppcheck.sourceforge.net/

[mikkoi]

N.B. This change does not cause a CI build to fail if Cppcheck finds problems. But it does show them.

[mikkoi]

The current parameters are:
--quiet
--std=c89
--enable=warning,style,performance,portability

If we want to make the build fail, we just add parameter --error-exitcode=1.

[brarcher]

There is another PR for C99. Should this be C99 instead?

[mikkoi]

Fixed in the newest commit "Use CMAKE_C_STANDARD with Cppcheck run".
Thanks for pointing that out.

[brarcher]

Is the goal to run cppcheck during the CMake build, and fail it if an issue is found? Should that be a build step instead of something else? My concern is that a scan with cppcheck would work today and the build would work, and in the future if cppcheck updates and find an issue it would prevent existing builds for working.

Instead, should this scan be a part of the Travis-CI automated build? If cppcheck updates and finds new issues it can be handled in a way that does not break clients of Check. It would also help prevent new breakage from being introduced to Check. For example, the Travis-CI build is using scan-build to find issues.

[mikkoi]

The goal is to run Cppcheck during CMake build when developing (when the build type is "Debug"). I have configured the build so that Cppcheck will not fail the build if errors are found. So the purpose is merely to warn or inform the developer that problems or weaknesses have been found. It's like using the -Wall (warning) flags in GCC.

This should also keep the build future proof. Cppcheck has the following parameter:
--error-exitcode= If errors are found, integer [n] is returned instead of
the default '0'. '1' is returned
if arguments are not valid or if no input files are
provided. Note that your operating system can modify
this value, e.g. '256' can become '0'.
I added this parameter with value '0' so Cppcheck will never break the build.

This scan is also part of the build in Travis-CI. All Travis builds are 'Debug' unless otherwise defined.

[brarcher]

So the purpose is merely to warn or inform the developer that problems or weaknesses have been found.

I have configured the build so that Cppcheck will not fail the build if errors are found.

Are these two competing points? I can see that if a new contributor to the project submits a change, the output of this new check may go unnoticed by both the contributor (may not notice the messages) and myself (as the build did not break). The check will be most valuable if it helps me as the maintainer from either introducing or accepting changes with new issues.

I'm find with using cppcheck to provide more checks on the codebase. I think, though, that they would be better if they broke Check's CI build without also breaking other's builds. If an issue is found someone could follow the same steps locally and reproduce the issue. I'm not sure if adding a check before unit tests are run which ignores the found issues (or only reports them to stdout) helps prevent new issues from being introduced.

What are your thoughts?

[mikkoi]

I pushed new commits which can solve that issue.
The CMake option ENABLE_TREAT_WARNINGS_AS_ERRORS in OFF by default, i.e. Cppcheck only reports. But when building in Travis we enable it and the build fails if errors are detected. Unfortunately, there is no easy way (at present, anyway) to print out all the errors and then fail. Cppcheck will break the build immediately at the first detected fault.

I just realized there is an awful lot of false positives among the tests, mostly concerning style as tests are often written in a verbose manner.

If an issue is found someone could follow the same steps locally and reproduce the issue.

Up to a certain point because Cppcheck might not be present in some systems or have different version.

I am thinking about using the same CMake option ENABLE_TREAT_WARNINGS_AS_ERRORS with the compiler warnings.

[mikkoi]

Interesting, Travis build is not failing even when errors are detected. Locally it worked well. Needs more investigation...