Hi Jan, > On 31 May 2022, at 14:50, Jan Beulich <jbeul...@suse.com> wrote: > > 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?
Sure will do. > > 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. Ok. > >> @@ -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. Will fix > >> +# 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). I am a bit lost on the $@, previous patch adding cppcheck was changed to use $(Q) instead. So can you justify this request ? For the rest, thanks for the finding, I agree and will fix. Cheers Bertrand > > Jan >
