Re: [RFC PATCH] xen/build: Add cppcheck and cppcheck-html make rules

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 25 Mar 2022, Michal Orzel wrote:
> On 25.03.2022 02:32, Stefano Stabellini wrote:
> > On Thu, 24 Mar 2022, Bertrand Marquis wrote:
> >> cppcheck can be used to check Xen code quality.
> >>
> >> To create a report do "make cppcheck" on a built tree adding any options
> >> you added during the process you used to build xen (like CROSS_COMPILE
> >> or XEN_TARGET_ARCH). This will generate an xml report xen-cppcheck.xml.
> >>
> >> To create a html report do "make cppcheck-html" in the same way and a
> >> full report to be seen in a browser will be generated in
> >> cppcheck-htmlreport/index.html.
> >>
> >> For better results it is recommended to build your own cppcheck from the
> >> latest sources that you can find at [1].
> >> Development and result analysis has been done with cppcheck 2.7.
> >>
> >> The Makefile rule is searching for all C files which have been compiled
> >> (ie which have a generated .o file) and is running cppcheck on all of
> >> them using the current configuration of xen so only the code actually
> >> compiled is checked.
> >>
> >> A new tool is introduced to merge all cppcheck reports into one global
> >> report including all findings and removing duplicates.
> >>
> >> Some extra variables can be used to customize the report:
> >> - CPPCHECK can be used to give the full path to the cppcheck binary to
> >> use (default is to use the one from the standard path).
> >> - CPPCHECK_HTMLREPORT can be used to give the full path to
> >> cppcheck-htmlreport (default is to use the one from the standard path).
> >>
> >> This has been tested on several arm configurations (x86 should work but
> >> has not been tested).
> >>
> >> [1] https://cppcheck.sourceforge.io/
> >>
> >> Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> >> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
> > 
> > Very cool, I was looking forward to this :-)
> > 
> > 
> >> diff --git a/xen/tools/merge_cppcheck_reports.py 
> >> b/xen/tools/merge_cppcheck_reports.py
> >> new file mode 100755
> >> index 0000000000..ef055f6925
> >> --- /dev/null
> >> +++ b/xen/tools/merge_cppcheck_reports.py
> >> @@ -0,0 +1,73 @@
> >> +#!/usr/bin/env python
> >> +
> >> +"""
> >> +This script acts as a tool to merge XML files created by cppcheck.
> >> +Usage:
> >> +    merge_cppcheck_reports.py [FILES] [OUTPUT]
> >> +
> >> +    FILES  - list of XML files with extension .cppcheck
> >> +    OUTPUT - file to store results (with .xml extension).
> >> +             If not specified, the script will print results to stdout.
> >> +"""
> >> +
> >> +import sys
> >> +from xml.etree import ElementTree
> >> +
> >> +def elements_equal(el1, el2):
> >> +    if type(el1) != type(el2): return False
> >> +
> >> +    el1_location = str(el1.find('location').attrib)
> >> +    el2_location = str(el2.find('location').attrib)
> >> +
> >> +    if el1_location != el2_location: return False
> >> +
> >> +    return True
> >> +
> >> +def remove_duplicates(xml_root_element):
> >> +    elems_to_remove = []
> >> +    index = 0
> >> +    elems_list = list(xml_root_element.findall("errors")[0])
> >> +    for elem1 in elems_list:
> >> +        index += 1
> >> +        for elem2 in elems_list[index:]:
> >> +            if elements_equal(elem1, elem2) and elem2 not in 
> >> elems_to_remove:
> >> +                elems_to_remove.append(elem2)
> >> +                continue
> >> +
> >> +    for elem in elems_to_remove:
> >> +        xml_root_element.findall("errors")[0].remove(elem)
> >> +
> >> +def merge(files):
> >> +    result_xml_root = None
> >> +    for xml_file in files:
> >> +        xml_root = ElementTree.parse(xml_file).getroot()
> > 
> > 
> > Traceback (most recent call last):
> >   File "/local/repos/xen-upstream/xen/tools/merge_cppcheck_reports.py", 
> > line 73, in <module>
> >     run()
> >   File "/local/repos/xen-upstream/xen/tools/merge_cppcheck_reports.py", 
> > line 60, in run
> >     result = merge(files)
> >   File "/local/repos/xen-upstream/xen/tools/merge_cppcheck_reports.py", 
> > line 43, in merge
> >     xml_root = ElementTree.parse(xml_file).getroot()
> >   File "/usr/lib/python2.7/xml/etree/ElementTree.py", line 1182, in parse
> >     tree.parse(source, parser)
> >   File "/usr/lib/python2.7/xml/etree/ElementTree.py", line 657, in parse
> >     self._root = parser.close()
> >   File "/usr/lib/python2.7/xml/etree/ElementTree.py", line 1671, in close
> >     self._raiseerror(v)
> >   File "/usr/lib/python2.7/xml/etree/ElementTree.py", line 1523, in 
> > _raiseerror
> >     raise err
> > xml.etree.ElementTree.ParseError: no element found: line 11, column 0
> > make: *** [Makefile:576: xen-cppcheck.xml] Error 1
> > 
> > I think we should catch the xml.etree.ElementTree.ParseError exception and 
> > continue?
> 
> Well, this is of course something that we might do but this error clearly 
> warns us that
> some XML files is not well formatted and therefore is not parsable. This 
> could mean that
> you are using some old cppcheck version. Is it correct assumption?

I confirm it was an issue with the cppcheck version. I updated to the
latest main branch and it worked fine, thanks!

Then, I suggest we catch the exception and print an informative error
like "please upgrade your cppcheck to version xxx or greater"