Re: [PATCH 3/3] xen/misra: xen-analysis.py: Fix cppcheck report relative paths

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

> On 25 May 2023, at 01:46, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
> 
> On Fri, 19 May 2023, Luca Fancellu wrote:
>> Fix the generation of the relative path from the repo, for cppcheck
>> reports, when the script is launching make with in-tree build.
>> 
>> Fixes: b046f7e37489 ("xen/misra: xen-analysis.py: use the relative path from 
>> the ...")
>> Reported-by: Michal Orzel <michal.orzel@xxxxxxx>
>> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
>> ---
>> .../xen_analysis/cppcheck_report_utils.py     | 25 ++++++++++++++++---
>> 1 file changed, 21 insertions(+), 4 deletions(-)
>> 
>> diff --git a/xen/scripts/xen_analysis/cppcheck_report_utils.py 
>> b/xen/scripts/xen_analysis/cppcheck_report_utils.py
>> index fdc299c7e029..10100f6c6a57 100644
>> --- a/xen/scripts/xen_analysis/cppcheck_report_utils.py
>> +++ b/xen/scripts/xen_analysis/cppcheck_report_utils.py
>> @@ -1,6 +1,7 @@
>> #!/usr/bin/env python3
>> 
>> -import os
>> +import os, re
>> +from . import settings
>> from xml.etree import ElementTree
>> 
>> class CppcheckHTMLReportError(Exception):
>> @@ -101,12 +102,28 @@ def cppcheck_merge_txt_fragments(fragments_list, 
>> out_txt_file, strip_paths):
>>             text_report_content = list(text_report_content)
>>             # Strip path from report lines
>>             for i in list(range(0, len(text_report_content))):
>> -                for path in strip_paths:
>> -                    text_report_content[i] = text_report_content[i].replace(
>> -                                                                path + "/", 
>> "")
>>                 # Split by : separator
>>                 text_report_content[i] = text_report_content[i].split(":")
>> 
>> +                for path in strip_paths:
>> +                    text_report_content[i][0] = \
>> +                        text_report_content[i][0].replace(path + "/", "")
> 
> Why moving this for loop after "Split by : separator"? If it is
> necessary, would it make sense to do it in the previous patch?

Hi Stefano,

In the patch before I was fixing the bug, so I thought it was better to don’t 
introduce other changes,
here I moved the loop after the split because in this way we take into account 
only the first element
of the ‘:’ separated string, that is the path + “(line,col)”.

Before this path it was ok to do the replace on the full string because we were 
going just to remove the
path, now instead we use that path to check if it actually exists.

> 
> 
>> +                # When the compilation is in-tree, the makefile places
>> +                # the directory in /xen/xen, making cppcheck produce
>> +                # relative path from there, so check if "xen/" is a prefix
>> +                # of the path and if it's not, check if it can be added to
>> +                # have a relative path from the repository instead of from
>> +                # /xen/xen
>> +                if not text_report_content[i][0].startswith("xen/"):
>> +                    # cppcheck first entry is in this format:
>> +                    # path/to/file(line,cols), remove (line,cols)
>> +                    cppcheck_file = re.sub(r'\(.*\)', '',
>> +                                           text_report_content[i][0])
>> +                    if os.path.isfile(settings.xen_dir + "/" + 
>> cppcheck_file):
>> +                        text_report_content[i][0] = \
>> +                            "xen/" + text_report_content[i][0]
>> +
>>             # sort alphabetically for second field (misra rule) and as second
>>             # criteria for the first field (file name)
>>             text_report_content.sort(key = lambda x: (x[1], x[0]))
>> -- 
>> 2.34.1