Adding a test with cppcheck for Travis CI

[ghost]

No description provided.

[brarcher]

Are these style changes or logic changes? Are these because cppcheck indicated an issue?

As least for cppcheck 1.74 no issues were detected in the src dir. The test dir did mention the following:

[tests/check_check_pack.c:323] -> [tests/check_check_pack.c:328]: (warning) Either the condition 'rmsg!=0' is redundant or there is possible null pointer dereference: rmsg.

Which seems to indicate that there is a redundant null check. The following resolves this:

diff --git a/tests/check_check_pack.c b/tests/check_check_pack.c
index 7da7e57..dd5d557 100644
--- a/tests/check_check_pack.c
+++ b/tests/check_check_pack.c
@@ -325,7 +325,6 @@ START_TEST(test_ppack_onlyctx)
   ck_assert_msg (rmsg->test_line == -1,
               "Result loc line should be -1 with only CTX");

-  if (rmsg != NULL)
     free (rmsg);
   fclose(result_file);
 }

Would you mind including this into your pull request, to remove all errors/warnings before enabling the check?

[ghost]

No it is very small modification, so I putted it with cppcheck. else is useless as return in the previous exit the function if the test is true.

[brarcher]

Interesting, I've not heard of cppcheck. It sounds good to give it a shot and see how it goes.

This project currently uses Clang's static code analysis tool in a Jenkins job here. I wonder if cppcheck will be able to catch other issues which Clang's tool does not.

[brarcher]

This part is a formality, so that the Jenkins-hosted automated tests will run:

Jenkins: ok to test

[ghost]

You may also be interested by vera++: https://bitbucket.org/verateam/vera/overview
An example: https://gitlab.com/RyDroid/PlanetWars2dRT-core/blob/master/tools/vera++-tests.sh

[brarcher]

Are you still interested in getting this change in? If so, could you update the branch so that it can be evaluated against master?

[ghost]

I am still interested. I put the commit on top.

[brarcher]

I do not think that the change was successful on Linux. The following was reported on Travis-CI:

$ if [ ${TRAVIS_OS_NAME} = 'linux' ]; then cppcheck --verbose --quiet --error-exitcode=1 --force --enable=performance,portability src/ tests/; fi
[tests/check_check_sub.c:846]: (error) Division by zero
[tests/check_check_sub.c:858]: (error) Division by zero
[tests/check_check_sub.c:869]: (error) Division by zero
[tests/check_check_sub.c:894]: (error) Division by zero
[tests/check_check_sub.c:1316]: (error) Division by zero
[tests/check_check_sub.c:1328]: (error) Division by zero
[tests/check_check_sub.c:1339]: (error) Division by zero
[tests/check_check_sub.c:1364]: (error) Division by zero
[tests/check_check_sub.c:1788]: (error) Division by zero
[tests/check_check_sub.c:1800]: (error) Division by zero
[tests/check_check_sub.c:1811]: (error) Division by zero
[tests/check_check_sub.c:1836]: (error) Division by zero
The command "if [ ${TRAVIS_OS_NAME} = 'linux' ]; then cppcheck --verbose --quiet --error-exitcode=1 --force --enable=performance,portability src/ tests/; fi" exited with 1.

cppcheck was smart enough to determine that there were some division by zeros, for example:

  float x = 0.0001f;
  ck_assert_float_finite(x);
  // MS VS doesn't allow explicit division by zero
  float t = 1.0f;
  x = 1.0f / (1.0f - t);
  record_failure_line_num(__LINE__);
  ck_assert_float_finite(x);

However, in this case it is intentional, as the test checks that the result is INF.

Is there a way to disable the divide by zero check?

[ghost]

The version of cppcheck of Debian Jessie returns a different output:

$ cppcheck --verbose --quiet --error-exitcode=1 --force --enable=performance,portability src/ tests/
[tests/check_check_sub.c:656] -> [tests/check_check_sub.c:658]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:660] -> [tests/check_check_sub.c:664]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:666] -> [tests/check_check_sub.c:668]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:670] -> [tests/check_check_sub.c:672]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:674] -> [tests/check_check_sub.c:676]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:820] -> [tests/check_check_sub.c:822]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:824] -> [tests/check_check_sub.c:828]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:830] -> [tests/check_check_sub.c:832]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:1126] -> [tests/check_check_sub.c:1128]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:1130] -> [tests/check_check_sub.c:1134]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:1136] -> [tests/check_check_sub.c:1138]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:1140] -> [tests/check_check_sub.c:1142]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:1144] -> [tests/check_check_sub.c:1146]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:1290] -> [tests/check_check_sub.c:1292]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:1294] -> [tests/check_check_sub.c:1298]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:1300] -> [tests/check_check_sub.c:1302]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:1598] -> [tests/check_check_sub.c:1600]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:1602] -> [tests/check_check_sub.c:1606]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:1608] -> [tests/check_check_sub.c:1610]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:1612] -> [tests/check_check_sub.c:1614]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:1616] -> [tests/check_check_sub.c:1618]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:1762] -> [tests/check_check_sub.c:1764]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:1766] -> [tests/check_check_sub.c:1770]: (performance) Variable 'i' is reassigned a value before the old one has been used.
[tests/check_check_sub.c:1772] -> [tests/check_check_sub.c:1774]: (performance) Variable 'i' is reassigned a value before the old one has been used.

I can not check currently on an other OS (but I will be able on Trisquel, a fork of Ubuntu, in January). But it seems to be possible: https://stackoverflow.com/questions/2956331/how-to-use-cppchecks-inline-suppression-filter-option-for-c-code

[brarcher]

Interesting. Perhaps the version of cppcheck on Travis-CI is a more recent version, or at least a different version?

If we were able to filter some of the items at will, then cppcheck may make a good addition. Let me know if you find time to check that out further. Further, feel free to push changes to this branch and see how Travis-CI runs with them, if it is helpful.

[mikkoi]

I added a Cppcheck run into the CMake build. #222