Re: [PATCH] xen: Add MISRA support to cppcheck make rule

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.05.2022 15:30, Bertrand Marquis wrote:
> --- a/.gitignore
> +++ b/.gitignore
> @@ -297,6 +297,8 @@ xen/.banner
>  xen/.config
>  xen/.config.old
>  xen/.xen.elf32
> +xen/cppcheck-misra.txt
> +xen/cppcheck-misra.json
>  xen/xen-cppcheck.xml
>  xen/System.map
>  xen/arch/x86/boot/mkelf32

Please can you obey to sorting here, inserting next to
xen/cppcheck-htmlreport? Seeing that xen/xen-cppcheck.xml was added
here by you at the same time as xen/cppcheck-htmlreport, may I further
ask that you move that line to where it belongs as well?

Additionally I wonder if you couldn't use just one line, specifying
xen/cppcheck-misra.* ; this could then also hold for the _clean rule
addition further down in the patch.

> @@ -703,6 +716,21 @@ cppcheck-version:
>               exit 1; \
>       fi
>  
> +# List of Misra rules to respect is written inside a doc.
> +# In order to have some helpful text in the cppcheck output, generate a text
> +# file containing the rules identifier, classification and text from the Xen
> +# documentation file. Also generate a json file with the right arguments for
> +# cppcheck in json format including the list of rules to ignore

Nit: Missing full stop at the end.

> +# Replace current by goal in the dependency to generate an analysis for all
> +# rules we would like to respect.
> +cppcheck-misra.json cppcheck-misra.txt: $(XEN_ROOT)/docs/misra/rules.rst
> +     $(Q)$(srctree)/tools/convert_misra_doc.py -i $< -o cppcheck-misra.txt \
> +             -j cppcheck-misra.json
> +
> +# Prevent parallel make issues as script is generating both files
> +cppcheck-misra.json: cppcheck-misra.txt

With this dependency the earlier rule should not list multiple targets
(and it generally should not, for not being a pattern rule). If I'm not
mistaken the way you have it the Python script would be invoked twice,
and all you prevent is that it is invoked twice in parallel. And then
please use $@ inside the rule. Additionally, with the script being an
in-tree one, the rule should also have a dependency on that script
(such that the targets would be rebuilt if the script alone changes).

Jan