Re: [PATCH v2 2/5] xen/scripts: add cppcheck tool to the xen-analysis.py script

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 6 Dec 2022, Luca Fancellu wrote:
> > On 6 Dec 2022, at 17:06, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
> > On Tue, 6 Dec 2022, Luca Fancellu wrote:
> >> Hi Stefano,
> >>>> 
> >>>> +++ b/docs/misra/false-positive-cppcheck.json
> >>>> @@ -0,0 +1,12 @@
> >>>> +{
> >>>> +    "version": "1.0",
> >>>> +    "content": [
> >>>> +        {
> >>>> +            "id": "SAF-0-false-positive-cppcheck",
> >>>> +            "violation-id": "",
> >>>> +            "tool-version": "",
> >>>> +            "name": "Sentinel",
> >>>> +            "text": "Next ID to be used"
> >>>> +        }
> >>>> +    ]
> >>>> +}
> >>> 
> >>> I think we need to add to the cppcheck document how to figure out the
> >>> cppcheck id for a given violation in the html report
> >> 
> >> I’m planning to send some patches with cppcheck false positive fixes, 
> >> would them be enough?
> >> 
> >> We already have a section in documenting-violation.rst on how to document 
> >> the finding, for
> >> cppcheck it’s just a matter to get the text report, do you think it’s 
> >> better to add a part to that section
> >> on how to locate the cppcheck violation id from its text report?
> > 
> > Examples would certainly help a lot. Looking at the html results it
> > wasn't clear to me what the violation-id actually was. It took me a few
> > tries to understand that "shadowVariable" was the cppcheck violation-id.
> > 
> > Maybe just add: look under the column "Defect ID" amoung the html
> > results to find the violation-id, such as "variableScope".
> 
> I was thinking about showing where to locate the violation ID from the text 
> report, do you think it’s better
> to give an example from the HTML report instead?

I haven't used the text report, only the HTML report, so far. Maybe you
could document both.


> So far I have added this part to the bottom of documenting-violations.rst:
> 
> Also, the same tag can be used on other symbols from the linker that are
> declared in the codebase, because the justification holds for them too.
> 
> A possible violation found by Cppcheck can be handled in the same way, from 
> the
> cppcheck report it is possible to identify the violation id:
> 
> | include/public/arch-arm.h(226,0):misra-c2012-20.7:style:Expressions 
> resulting from the expansion of macro parameters shall be enclosed in 
> parentheses (Misra rule 20.7)
> 
> Given the violation id "misra-c2012-20.7", we can follow the procedure above 
> to
> justify the finding.

Yes that makes sense, and maybe add something similar for the html
report