LoRaWAN cppcheck warnings / bugs

[bertrik]

When running the code checker tool 'cppcheck' on the LoRaWAN code, several warnings and style issues are produced. Some (most?) of these are probably false positives, but there are also some things that look like genuine bugs.

The bug
In particular, it warns about a for-loop in https://github.com/jgromes/RadioLib/blob/master/src/protocols/LoRaWAN/LoRaWAN.cpp#L2834
for(uint8_t j = 0; j < 8; i++) {

This for-loop looks like a simple 8-time iteration, except it increments variable i while it checks variable j.
This looks like a simple copy-paste error, and might result in an infinite loop or hang.

To Reproduce
install cppcheck, e.g. using your distribution's package manager
enter directory RadioLib/src/protocols/LoRaWAN and run
cppcheck *.cpp --enable=all

Expected behavior
No warnings about 'is always true', 'shadow variable', 'assigned a value that is never used', etc.

[StevenCellist]

Hey @bertrik, cool to see you here and nice catch! Will include in #1017. Fortunately, we don't often see that channel mask happening, but even worse would be the result because we'd hardly ever catch it...
As soon as #1017 compiles, please feel free to run your checker on it as well and notify me of potential problems.

[jgromes]

Strange that the CodeQL action (https://github.com/jgromes/RadioLib/actions/runs/8311068463) did not find such an obvious problem. IT also seems that I ahve missed the deprecation notice.

I will add cppcheck as a separate CI job.

[jgromes]

The action is in, here's the output when scanning all RadioLib sources: https://github.com/jgromes/RadioLib/actions/runs/8313947188/job/22750505703?pr=1019

There's one error which seems like a bug in cppcheck itself (https://trac.cppcheck.net/ticket/11023), and most of the warnings are due to incorrect printf format specifiers.

@bertrik strangely enough, it does not find the loop you did - did you do any extra configuration? Also, which cppcheck verrsion are you using?

[bertrik]

I ran:
cppcheck *.cpp --enable=all
in the LoRaWAN directory.

My cppcheck version is Cppcheck 2.13.0

I know from experience that code scanning tools can be a mixed blessing.
In this case, the problem only becomes visible at the more elaborate check level, giving warnings about harmless things too.
Before you know, you're fixing non-issues / investigating code-scanning tool bugs / putting a lot of effort in suppressing false positives / accidentally introducing actual bugs trying to silence a warning, etc.

[jgromes]

I know from experience that code scanning tools can be a mixed blessing.

I'm painfully aware of that. That's why I didn't add any logic to the code scan CI that would trigger a job failure when something is detected.

I will leave this issue open until the PR fixing this is merged.