| Message ID | 5467d017fa1c6721436e21f8cc10c5d74adeb5bf.1654002661.git.bertrand.marquis@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: Add MISRA support to cppcheck make rule | expand |
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
Hi Jan, > On 31 May 2022, at 14:50, Jan Beulich <jbeulich@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 >
On 31.05.2022 16:14, Bertrand Marquis wrote: >> On 31 May 2022, at 14:50, Jan Beulich <jbeulich@suse.com> wrote: >> On 31.05.2022 15:30, Bertrand Marquis wrote: >>> +# 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 ? I'm talking of $@ (a macro expansion), not @ as a prefix to suppress command echoing. Jan
Hi, > On 31 May 2022, at 15:28, Jan Beulich <jbeulich@suse.com> wrote: > > On 31.05.2022 16:14, Bertrand Marquis wrote: >>> On 31 May 2022, at 14:50, Jan Beulich <jbeulich@suse.com> wrote: >>> On 31.05.2022 15:30, Bertrand Marquis wrote: >>>> +# 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 ? > > I'm talking of $@ (a macro expansion), not @ as a prefix to suppress > command echoing. > That make a lot more sense. Thanks Bertrand > Jan > >
diff --git a/.gitignore b/.gitignore index 18ef56a780..65a58ba7e4 100644 --- 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 diff --git a/xen/Makefile b/xen/Makefile index 82f5310b12..aee6660556 100644 --- a/xen/Makefile +++ b/xen/Makefile @@ -548,7 +548,7 @@ _clean: rm -f include/asm $(TARGET) $(TARGET).gz $(TARGET).efi $(TARGET).efi.map $(TARGET)-syms $(TARGET)-syms.map rm -f asm-offsets.s arch/*/include/asm/asm-offsets.h rm -f .banner .allconfig.tmp include/xen/compile.h - rm -f xen-cppcheck.xml + rm -f cppcheck-misra.json cppcheck-misra.txt xen-cppcheck.xml .PHONY: _distclean _distclean: clean @@ -642,6 +642,10 @@ CPPCHECK_HTMLREPORT ?= cppcheck-htmlreport # build directory. This can be changed by giving a directory in this variable. CPPCHECK_HTMLREPORT_OUTDIR ?= cppcheck-htmlreport +# By default we do not check misra rules, to enable pass "CPPCHECK_MISRA=y" to +# make command line. +CPPCHECK_MISRA ?= n + # Compile flags to pass to cppcheck: # - include directories and defines Xen Makefile is passing (from CFLAGS) # - include config.h as this is passed directly to the compiler. @@ -666,6 +670,15 @@ CPPCHECKFILES := $(wildcard $(patsubst $(objtree)/%.o,$(srctree)/%.c, \ $(filter-out $(objtree)/tools/%, \ $(shell find $(objtree) -name "*.o")))) +# Headers and files required to run cppcheck on a file +CPPCHECKDEPS := $(objtree)/include/generated/autoconf.h \ + $(objtree)/include/generated/compiler-def.h + +ifeq ($(CPPCHECK_MISRA),y) + CPPCHECKFLAGS += --addon=cppcheck-misra.json + CPPCHECKDEPS += cppcheck-misra.json +endif + quiet_cmd_cppcheck_xml = CPPCHECK $(patsubst $(srctree)/%,%,$<) cmd_cppcheck_xml = $(CPPCHECK) -v -q --xml $(CPPCHECKFLAGS) \ --output-file=$@ $< @@ -690,7 +703,7 @@ ifeq ($(CPPCHECKFILES),) endif $(call if_changed,merge_cppcheck_reports) -$(objtree)/%.c.cppcheck: $(srctree)/%.c $(objtree)/include/generated/autoconf.h $(objtree)/include/generated/compiler-def.h | cppcheck-version +$(objtree)/%.c.cppcheck: $(srctree)/%.c $(CPPCHECKDEPS) | cppcheck-version $(call if_changed,cppcheck_xml) cppcheck-version: @@ -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 +# +# 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 + # Put this in generated headers this way it is cleaned by include/Makefile $(objtree)/include/generated/compiler-def.h: $(Q)$(CC) -dM -E -o $@ - < /dev/null diff --git a/xen/tools/convert_misra_doc.py b/xen/tools/convert_misra_doc.py new file mode 100755 index 0000000000..47133a33a6 --- /dev/null +++ b/xen/tools/convert_misra_doc.py @@ -0,0 +1,139 @@ +#!/usr/bin/env python + +""" +This script is converting the misra documentation RST file into a text file +that can be used as text-rules for cppcheck. +Usage: + convert_misr_doc.py -i INPUT [-o OUTPUT] [-j JSON] + + INPUT - RST file containing the list of misra rules. + OUTPUT - file to store the text output to be used by cppcheck. + If not specified, the result will be printed to stdout. + JSON - cppcheck json file to be created (optional). +""" + +import sys, getopt, re + +def main(argv): + infile = '' + outfile = '' + outstr = sys.stdout + jsonfile = '' + + try: + opts, args = getopt.getopt(argv,"hi:o:j:",["input=","output=","json="]) + except getopt.GetoptError: + print('convert-misra.py -i <input> [-o <output>] [-j <json>') + sys.exit(2) + for opt, arg in opts: + if opt == '-h': + print('convert-misra.py -i <input> [-o <output>] [-j <json>') + print(' If output is not specified, print to stdout') + sys.exit(1) + elif opt in ("-i", "--input"): + infile = arg + elif opt in ("-o", "--output"): + outfile = arg + elif opt in ("-j", "--json"): + jsonfile = arg + + try: + file_stream = open(infile, 'rt') + except: + print('Error opening ' + infile) + sys.exit(1) + + if outfile: + try: + outstr = open(outfile, "w") + except: + print('Error creating ' + outfile) + sys.exit(1) + + # Each rule start with '- Rule: Dir' + pattern_dir = re.compile(r'^- Rule: Dir ([0-9]+.[0-9]+).*$') + pattern_rule = re.compile(r'^- Rule: Rule ([0-9]+.[0-9]+).*$') + pattern_severity = re.compile(r'^ - Severity: (.*)$') + pattern_text = re.compile(r'^ - Summary: (.*)$') + + rule_number = '' + rule_state = 0 + rule_list = [] + + # Start search by cppcheck misra + outstr.write('Appendix A Summary of guidelines\n') + + for line in file_stream: + + line = line.replace('\r', '').replace('\n', '') + + if len(line) == 0: + continue + + # New rule + if rule_state == 0: + res = pattern_rule.match(line) + if res: + rule_state = 1 + rule_number = res.group(1) + rule_list.append(rule_number) + continue + res = pattern_dir.match(line) + if res: + rule_state = 1 + rule_number = res.group(1) + rule_list.append(rule_number) + continue + + # Severity + elif rule_state == 1: + res = pattern_severity.match(line) + if res: + outstr.write('Rule ' + rule_number + ' ' + res.group(1) + '\n') + rule_state = 2 + continue + + # Summary + elif rule_state == 2: + res = pattern_text.match(line) + if res: + outstr.write(res.group(1) + ' (Misra rule ' + rule_number + + ')\n') + rule_state = 0 + rule_number = '' + continue + else: + print('Error impossible case !!!') + + skip_list = [] + + # Search for missing rules and add a dummy text with the rule number + for i in list(range(1,22)): + for j in list(range(1,22)): + if str(i) + '.' + str(j) not in rule_list: + outstr.write('Rule ' + str(i) + '.' + str(j) + '\n') + outstr.write('No description for rule ' + str(i) + '.' + str(j) + + '\n') + skip_list.append(str(i) + '.' + str(j)) + + # Make cppcheck happy by starting the appendix + outstr.write('Appendix B\n') + outstr.write('\n') + if outfile: + outstr.close() + + if jsonfile: + with open(jsonfile, "w") as f: + f.write('{\n') + f.write(' "script": "misra.py",\n') + f.write(' "args": [\n') + if outfile: + f.write(' "--rule-texts=' + outfile + '",\n') + + f.write(' "--suppress-rules=' + ",".join(skip_list) + '"\n') + f.write(' ]\n') + f.write('}\n') + f.close() + +if __name__ == "__main__": + main(sys.argv[1:])
cppcheck MISRA addon can be used to check for non compliance to some of the MISRA standard rules. Add a CPPCHECK_MISRA variable that can be set to "y" using make command line to generate a cppcheck report including cppcheck misra checks. When MISRA checking is enabled, a file with a text description suitable for cppcheck misra addon is generated out of Xen documentation file which lists the rules followed by Xen (docs/misra/rules.rst). By default MISRA checking is turned off. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> --- This change depends on the patch from Stefano Stabellini adding the list of misra rules in the documentation (docs/misra: introduce rules.rst). --- .gitignore | 2 + xen/Makefile | 32 +++++++- xen/tools/convert_misra_doc.py | 139 +++++++++++++++++++++++++++++++++ 3 files changed, 171 insertions(+), 2 deletions(-) create mode 100755 xen/tools/convert_misra_doc.py