Fix type mismatch of line and column number and add extra information in misra-reports

[PeterSchops]

Make sure that useline and usecolumn are always of type integer in the python scripts. Add extra information in misra-reports, for a number of checks, to make it easier to know what is exactly wrong.

[danmar]

could you show some example output for the extra info.

Technically I am not against this extra information.

It's just that misra foundation wants that we write the exact error message that Misra has written in the PDF. With NO changes. It was important for them.

[PeterSchops]

When you look at the reportError function, you can see that there are 2 possible ways it can write the extra information into the file. In case --cli the extra information is written separately in the json-file, else I add the extra information after the message. (If this would be an issue, this could be removed)

This is how it looks in my reports:

In my case I'm using the first option, and the extra information is available separately. And the user needs to click the [+] to reveal the extra information. And by default only the user provided misra information is displayed.

[danmar]

This is how it looks in my reports:

Sorry that is not clear to me.

I wonder what the xml looks like. Is the extra information added directly to the message text or as extra notes or what.. I don't remember off the top of my head.

[danmar]

Please give me the details.

• A small example code. The shorter the better.
• The exact xml output from cppcheck.

I do not want to write an example code myself and then apply these changes and then testrun it to see exactly what it looks like :-(

Run cppcheck like so:

cppcheck --addon=misra --enable=style yourtestfile.c  --xml 2> out.xml
cppcheck --addon=misra --enable=style yourtestfile.c  2> out.txt

then please show what yourtestfile.c, out.xml and out.txt says..

[PeterSchops]

Please give me the details.

* A small example code. The shorter the better.

* The exact xml output from cppcheck.

I do not want to write an example code myself and then apply these changes and then testrun it to see exactly what it looks like :-(

Run cppcheck like so:

cppcheck --addon=misra --enable=style yourtestfile.c  --xml 2> out.xml
cppcheck --addon=misra --enable=style yourtestfile.c  2> out.txt

then please show what yourtestfile.c, out.xml and out.txt says..

Please give me the details.

* A small example code. The shorter the better.

* The exact xml output from cppcheck.

I do not want to write an example code myself and then apply these changes and then testrun it to see exactly what it looks like :-(

Run cppcheck like so:

cppcheck --addon=misra --enable=style yourtestfile.c  --xml 2> out.xml
cppcheck --addon=misra --enable=style yourtestfile.c  2> out.txt

then please show what yourtestfile.c, out.xml and out.txt says..

I usually run misra.py on the dump-files created by cppcheck. But I can also try this, and provide the output. In my inbox I also found another comment that I also found interesting and useful. But I can't find it here anymore.

I appreciate your feedback. And I realize the way I updated the reportError function was not the best.

[PeterSchops]

cppcheck --addon=misra --enable=style misra-test-c.cpp --xml 2> out.xml cppcheck --addon=misra --enable=style misra-test-c.cpp 2> out.txt

demo.zip

In the XML-file I see use --rule-texts=, but when I try that, cppcheck tells me it is a unrecognized option.

So I tried then with using the dump-file as intermediate step, so that I can call the python code directly. cppcheck --enable=style --dump misra-test-c.cpp python3 misra.py --rule-texts=misra.txt misra-test-c.cpp.dump

And this gives me this: [misra-test-c.cpp:2] (style) Octal constants shall not be used (style) [misra-c2012-7.1] [misra-test-c.cpp:7] (style) A "u" or "U" suffix shall be applied to all integer constants that are represented in an unsigned type (style) (suffix 'u' missing for 0x80000000) [misra-c2012-7.2]

MISRA rules violations found: Required: 2

MISRA rules violated: misra-c2012-7.1 (-): 1 misra-c2012-7.2 (-): 1

Where the 7.2 has extra information.

I'm always using the dump-file method, because that is working for me, but I was wondering

• when I call the misra.py directly, and only pass the dump-files, does it also perform the ctu-info checks?
• when I call the misra.py script directly with dump-files, there seems to be no option to output XML data, so I think that is the reason why I added that on my side.

I also figured out that I need to use misra.json to pass the --rule-texts to cppcheck cppcheck --addon=misra.json --enable=style misra-test-c.cpp --xml 2> json.xml cppcheck --addon=misra.json --enable=style misra-test-c.cpp 2> json.txt

And to finish it, I included the output my script generated. This XML only contains the misra errors. (I wanted cppcheck and misra seperate, since misra is more strict than cppcheck. So an error flagged by cppcheck is more serious then one flagged by misra.py) python3 custom_rules.py --rule-texts=custom_rules.txt --file=custom.xml misra-test-c.cpp.dump When no extra info is added msg and verbose are identical, when extra info is added verbose contains the extra information.

Conclusion:

• the extra information I added does not end up in the reports created by cppcheck (this was expected, since I did not touch if --cli part
• the extra information does get written to the raw dump-file

So the msg in the if --cli part of cppcheckdata.py needs to be updated in one way or the other, to make this feature useful for all users. Would the output of custom.xml be ok for you? (msg-part should remain untouched, verbose-part can be changed)

[PeterSchops]

The parsing of reportError data from cppcheckdata.py seems to be done here in cppcheck.cpp https://github.com/danmar/cppcheck/blob/5a21851bb7943fb29268043209d1a8e4e2780396/lib/cppcheck.cpp#L1458

def reportError(location, severity, message, addon, errorId, misra_severity='', extra=''):
    if '--cli' in sys.argv:
        msg = { 'file': location.file,
                'linenr': location.linenr,
                'column': location.column,
                'severity': severity,
                'message': message,
                'addon': addon,
                'errorId': errorId,
                'extra': misra_severity}
        sys.stdout.write(json.dumps(msg) + '\n')

I see multiple things written in the json-dump getting decoded.

• file
• linenr
• column
• addon
• errorId
• message
• severity

But extra seems not to be used. So if you can give me a proposal of how I could update the python code and the cppcheck I'm willing to give it a try.

[danmar]

when I call the misra.py directly, and only pass the dump-files, does it also perform the ctu-info checks?

If I am not mistaken the misra.py will save the ctu-info summaries when you check the dump files. But no warning is reported.

To perform the analysis and get warnings you need to invoke misra.py at the end after all dump files have been checked and provide the list of ctu-info files.

when I call the misra.py script directly with dump-files, there seems to be no option to output XML data, so I think that is the reason why I added that on my side.

That is correct. I think that using cppcheck as frontend is the "recommended" way. I am not against that you call misra.py directly but I basically feel I am not very interested to duplicate and maintain the same frontend features in both the addon and core cppcheck.

[danmar]

So if you can give me a proposal of how I could update the python code and the cppcheck I'm willing to give it a try.

I am actually not sure.

If we really want that the warning text will be identical with the rule text in the misra specification then I think it would be best to add the extra information in an additional note.

    <error id="misra-c2012-7.1" severity="style" msg="rule text" verbose="rule text" file0="misra-test-c.cpp">
        <location file="misra-test-c.cpp" line="2" column="11" info="extra information"/>
        <location file="misra-test-c.cpp" line="2" column="11" info="rule text"/>
    </error>

[danmar]

If the expected cppcheck xml output is this:

<error id="misra-c2012-7.1" severity="style" msg="rule text" verbose="rule text" file0="misra-test-c.cpp">
    <location file="misra-test-c.cpp" line="2" column="11" info="extra information"/>
    <location file="misra-test-c.cpp" line="2" column="11" info="rule text"/>
</error>

then maybe the corresponding addon output could look something like this:

{
    "addon": "misra",
    "errorId": "c2012-7.1",
    "severity": "style",
    "message": "rule text",
    "locations": [
        { "file":"misra-test-c.cpp", "linenr":2, "column":11, "info": "extra information" },
        { "file":"misra-test-c.cpp", "linenr":2, "column":11, "info": "rule text" }
     ]
}

without newlines. Cppcheck could support both formats.. cppcheckdata.reportError could print the old compact format when extra notes are not needed.

[danmar]

But extra seems not to be used.

ok I understand. It feels good. Probably it would be OK to remove extra from the --cli output.

[PeterSchops]

If the expected cppcheck xml output is this:

<error id="misra-c2012-7.1" severity="style" msg="rule text" verbose="rule text" file0="misra-test-c.cpp">
    <location file="misra-test-c.cpp" line="2" column="11" info="extra information"/>
    <location file="misra-test-c.cpp" line="2" column="11" info="rule text"/>
</error>

then maybe the corresponding addon output could look something like this:

{
    "addon": "misra",
    "errorId": "c2012-7.1",
    "severity": "style",
    "message": "rule text",
    "locations": [
        { "file":"misra-test-c.cpp", "linenr":2, "column":11, "info": "extra information" },
        { "file":"misra-test-c.cpp", "linenr":2, "column":11, "info": "rule text" }
     ]
}

without newlines. Cppcheck could support both formats.. cppcheckdata.reportError could print the old compact format when extra notes are not needed.

When I look at the XML-code generated by cppcheck, I see that the msg and verbose are not always identical.

        <error id="passedByValue" severity="performance" msg="Function parameter &apos;subsensors&apos; should be passed by const reference." verbose="Parameter &apos;subsensors&apos; is passed by value. It could be passed as a const reference which is usually faster and recommended in C++." cwe="398" file0="/9037x/Tests/Hall3D/Bubble_SMP4/support_MBUBBLE_SMP4.cpp">
            <location file="/9037x/Tests/Hall3D/Bubble_SMP4/support_MBUBBLE_SMP4.h" line="238" column="99"/>
            <symbol>subsensors</symbol>
        </error>
        <error id="variableScope" severity="style" msg="The scope of the variable &apos;curTime&apos; can be reduced." verbose="The scope of the variable &apos;curTime&apos; can be reduced. Warning: Be careful when fixing this message, especially when there are inner loops. Here is an example where cppcheck will write that the scope for &apos;i&apos; can be reduced:\012void f(int x)\012{\012    int i = 0;\012    if (x) {\012        // it&apos;s safe to move &apos;int i = 0;&apos; here\012        for (int n = 0; n &lt; 10; ++n) {\012            // it is possible but not safe to move &apos;int i = 0;&apos; here\012            do_something(&amp;i);\012        }\012    }\012}\012When you see this message it is always safe to reduce the variable scope 1 level." cwe="398" file0="/9037x/Tests/dutSupport.cpp">
            <location file="/9037x/Tests/dutSupport.cpp" line="5313" column="10"/>
            <symbol>curTime</symbol>
        </error>

And I wanted to get the extra information to end up in the verbose section. Since that is where the helping is provided to the user is ending up for cppcheck reports today. Your proposal will also work, but it would require an update in the html-generation to display the extra information.

[PeterSchops]

when I call the misra.py directly, and only pass the dump-files, does it also perform the ctu-info checks?

If I am not mistaken the misra.py will save the ctu-info summaries when you check the dump files. But no warning is reported.

To perform the analysis and get warnings you need to invoke misra.py at the end after all dump files have been checked and provide the list of ctu-info files.

I'm passing only a list of all the dump-files. So I will need to make some changes.

• option1: call misra.py through cppcheck, and split the XML afterwards based on file-info to separate the cppcheck report from the others -> use it like everybody else and avoid headache
• option2: pass dump + ctu-info files to misra.py -> Not standard way of working, but ideal for debugging of misra check issues.

Could I use multiple scripts at once with cppcheck? It is a pity that in the XML the addon is not mentioned that found the error.

when I call the misra.py script directly with dump-files, there seems to be no option to output XML data, so I think that is the reason why I added that on my side.

That is correct. I think that using cppcheck as frontend is the "recommended" way. I am not against that you call misra.py directly but I basically feel I am not very interested to duplicate and maintain the same frontend features in both the addon and core cppcheck.

I completely understand that. It is already a big code base, where C++ and Python are combined. And I tried to fix a number of mismatches between the cppcheckdata.py and the information written in the dump-files. It was not a small task.

[danmar]

And I wanted to get the extra information to end up in the verbose section.

ok that is an option.. but then the warning text will not be identical to the rule text in the misra specification.

If you call cppcheck like this:

cppcheck --addon=misra.json file.c

Then cppcheck will print the "message" text. without the extra information.

If you call cppcheck like this:

cppcheck --addon=misra.json --verbose file.c

Then the "message" is not written. But the "verbose" message is written instead. If that just contains your extra information then cppcheck will not write the rule text at all.

[danmar]

Your proposal will also work, but it would require an update in the html-generation to display the extra information.

It sounds like the html-generation should be updated so it's consistent with cppcheck. If it prints both the "msg" string and "verbose" string that is not consistent.

[danmar]

It is a pity that in the XML the addon is not mentioned that found the error.

well it's mentioned but it's hard to extract the information. the "id" in the xml is "{addon}-{errorId}" from the addon cli output. at least you can say that:

• if errorId in xml file starts with "misra-" then it likely comes from the misra addon.
• if errorId in xml file does not start with "misra-" it is not coming from the misra addon.

[PeterSchops]

Thanks for the input, I don't have time to continue with this for the moment. But when I have some spare time I will continue.