misra.py: Fix 5.3 FP

[jubnzv]

This will close https://trac.cppcheck.net/ticket/9497

[jubnzv]

Hmm, it will be somewhat difficult to keep the whole misra test suite in single file. There are already conflicts in names and number of annoying "false positives" from unrelated rules like this.

I suppose we need a CLI option for inclusive rules select (something like --enabled-rules 5.3) and separated C files for testing particular rules. This makes test suite cleaner and allows us easily extend it.

@danmar @versat @orbitcowboy what do you think?

[versat]

Hmm, it will be somewhat difficult to keep the whole misra test suite in single file. There are already conflicts in names and number of annoying "false positives" from unrelated rules like this.

If it is a false positive it should be marked as such and a ticket should be created I suggest.
Maybe the conflicts in the names could be solved by using the rule number in the names more consequently? Not sure if that is always possible.
I also thought about making the file better readable. For example by using an easily visible comment in front of each rule, something like

// #######################
// ## MISRA Rule 5.2
// #######################

I suppose we need a CLI option for inclusive rules select (something like --enabled-rules 5.3)

I am not sure I got you. Do you mean that some rules should be disabled by default and only be explicitly enabled by a command-line option?
This is something I would not vote for.

and separated C files for testing particular rules. This makes test suite cleaner and allows us easily extend it.

I thought about that too. IMHO currently it is still acceptable to have it in one file, but if it grows on and on it could be a good idea to split it up. What is important is that it is at least as easy as it is now to run the verification. So if there is on script that can be called to run all verifications that would be ok for me.

[jubnzv]

Do you mean that some rules should be disabled by default and only be explicitly enabled by a command-line option?

Yeah, that's what I mean. The idea is to run addon for particular source file with only one rule enabled. This source file should contains tests only for one rule. Just like it's done in test suite. For example, if we want to test only rule 5.3, we don't have to run check functions for other rules and should suppress them. Otherwise we'll get useless alarms, like this or that.

But it can be achieved without extra cli option. We can use --suppress-rules to disable all rules excepted selected one and wrap it in shell script. For example:

rules_list=`python misra.py -generate-table | grep -v cppcheck | awk '{print $1}' | paste -sd ',' -`
rules_exl_53=`echo $rules_list | sed -e 's/5\.3,//g'`
python misra.py --suppress-rules="$rules_exl_53" -verify test-5-3.c

I'm not sure how best to integrate this in CI. Maybe tox with commands in tox.ini could be useful?

[danmar]

Do you mean that some rules should be disabled by default and only be explicitly enabled by a command-line option?

Yeah, that's what I mean.

Sounds bad to me, that would break the scripts that people have made, wouldn't it? We can tweak how the addon works when --verify is used but not during normal usage.

I don't have a very strong opinion about having separated test files but spontaneosly I prefer 1 file. The advantage with running all rules on all code is that the testing coverage will be greater. We don't test the misra addon much.

To get better testing maybe there is some open source project we can scan in daca@home. It would have to be some project that tries to be misra-compatible. I'm not sure what good candidates there are.

[danmar]

@jubnzv I don't remember why I added such checking in both misra and cppcheck. Do you know if this checking extends the warnings given by Cppcheck shadowArgument and shadowVariable warnings somehow?

[danmar]

To get better testing maybe there is some open source project we can scan in daca@home. It would have to be some project that tries to be misra-compatible. I'm not sure what good candidates there are.

One possible project is chibios.

They check that their code is misra 2012 compliant. See for instance this configuration file:
./test/nil/testbuild/pclint/au-misra3.lnt

[jubnzv]

Sounds bad to me, that would break the scripts that people have made, wouldn't it? We can tweak how the addon works when --verify is used but not during normal usage.

No, it wouldn't, if we implement this as additional cli option for debugging purpose. The idea with -verify is even better. But using of this option makes sense only with separated files, otherwise it would be useless.

I don't have a very strong opinion about having separated test files but spontaneosly I prefer 1 file.

Okay, so far this does not create any serious problems. May be we could split test file later, when it would be really large and clumsy.

To get better testing maybe there is some open source project we can scan in daca@home.

I've tried scanning popular github C repositories with my own scripts. And I can say for sure that the current implementation of addons is not ready for this. We definitely need to fix some critical problems:

1. Memory usage (addons: Reduce memory consumption #2395), since the size of generated files is often would be more than few hundreds MB. This means that cppcheck process would be killed by OOM frequently, on relatively high-performance machines. At least 16 GB RAM is not enough for random scanning. After this patch it becomes possible to use addons with 3-4 GB RAM. It's should be additionally tested, of course.

2. Unexcepted exceptions with recursive functions of misra.py (trac 9487). This can be fixed with iterative implementation of troublesome functions (or with small hack with max recursion depth). Otherwise we need to suppress few rules or catch a large stacks from python interpreter.

3. @orbitcowboy noticed the problem with large dump files (trac 9499) created during the check. There may be situations when daca users don't have a few GB free space on hard drives. I'm not sure how relevant this problem is.

I think when we solve these problems, it could be possible to tweak daca scripts to add an additional option for addons testing.

It would have to be some project that tries to be misra-compatible. I'm not sure what good candidates there are.

I know about zephyr os efforts to add MISRA compatibility. They also mentioned cppcheck here.

There are also FreeRTOS with some deviations from original MISRA and few small libraries, mostly for embedded devices, e.g. libuavcan.

[jubnzv]

@jubnzv I don't remember why I added such checking in both misra and cppcheck. Do you know if this checking extends the warnings given by Cppcheck shadowArgument and shadowVariable warnings somehow?

I can't get cppcheck output from addon script. How to better integrate it? May be I can report errors in cppcheck itself based on addon output when we start it through --addon=? I would add this in this function.