dist/tools/cppcheck: improve warnings

[maribu]

Contribution description

• provide include paths and defines of samr21-xpro
• provide platform configuration for Cortex-M MCUs
• detect include paths of all modules by searching for include folders in core, drivers, and sys
• filter out files in cpu/* and boards/* before checking, as those depend on board/architecture specific include paths and defines
• re-enable cppcheck in the CI
• drop some suppressions of false positives in sys/net/grnc that should no longer pop up with the proper type definitions now around

Testing procedure

At least some of the dropped suppressions should no longer be needed. (Note: None of these is needed with the latest cppcheck, but other warnings are generated instead.)

Issues/PRs references

#17282

[maribu]

Here the warning of cppcheck is correct: tcb must be NULL, since the while (tcb) has no break in it except when MODULE_GNRC_IPV6 is defined (which cppcheck assumes to not be the case).

[maribu]

@brummer-simon @miri64: What was the reasoning behind this:

RIOT/sys/net/gnrc/transport_layer/tcp/gnrc_tcp_eventloop.c

Lines 239 to 245 in 44ebc38

	#else
	/* Suppress compiler warnings if TCP is built without network layer */
	TCP_DEBUG_ERROR("Missing network layer. Add module to makefile.");
	(void) syn;
	(void) src;
	(void) dst;
	#endif

Isn't this one of the "we know your configuration is broken at build time, but we still emit a firmware that you can spent on hours debugging" cases that @benpicco is such a huge fan of?

Why not add

#ifndef MODULE_GNRC_IPV6
#error "module gnrc_ipv6 required but not used"
#endif

for ease of mind and model the dependency so that this cannot happen?

[brummer-simon]

@brummer-simon @miri64: What was the reasoning behind this:

RIOT/sys/net/gnrc/transport_layer/tcp/gnrc_tcp_eventloop.c

Lines 239 to 245 in 44ebc38

	#else
	/* Suppress compiler warnings if TCP is built without network layer */
	TCP_DEBUG_ERROR("Missing network layer. Add module to makefile.");
	(void) syn;
	(void) src;
	(void) dst;
	#endif

Isn't this one of the "we know your configuration is broken at build time, but we still emit a firmware that you can spent on hours debugging" cases that @benpicco is such a huge fan of?

Why not add

#ifndef MODULE_GNRC_IPV6
#error "module gnrc_ipv6 required but not used"
#endif

for ease of mind and model the dependency so that this cannot happen?

Good question regarding the reasoning. In general I am favor of breaking things at build time.
Feel free to remove these and similar macros. There should be a few like this within GNRC_TCP.

[benpicco]

Is this waiting for anything?
The changes look good!

[maribu]

I dropped the last commit disabling some of the cppcheck suppression, which were mostly still needed. I don't have locally the old version of cppcheck used in the CI installed, so it is some effort for me to remove only unneeded suppressions. That should be easier once the CI is updated.

[benpicco]

Ugh if this brings back the need to litter the code with cppcheck suppression comments just to get it though CI, we should wait with merging it until CI has been updated.

[maribu]

+1

[benpicco]

Is CI sufficiently up to date now that we can give this another try?

[maribu]

good question. Maybe @kaspar030 knows?

[maribu]

Maybe it would also make sense to just build it from source. Having top notch static analysis is helpful. Speaking of that, both LLVM and GCC have static analysis tools that at least from the outside look more promising than cppcheck. That isn't supposed to be an argument against also using cppcheck in addition. Once the false positive rate gets acceptable, any additional tool that increase coverage will help.