Re: [PATCH v2 1/2] xen/cppcheck: sort alphabetically cppcheck report entries

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

> On 30 Jan 2023, at 11:22, Michal Orzel <michal.orzel@xxxxxxx> wrote:
> 
> Hi Luca,
> 
> On 30/01/2023 12:01, Luca Fancellu wrote:
>> 
>> 
>> Sort alphabetically cppcheck report entries when producing the text
>> report, this will help comparing different reports and will group
>> together findings from the same file.
>> 
>> The sort operation is performed with two criteria, the first one is
>> sorting by misra rule, the second one is sorting by file.
>> 
>> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
>> ---
>> Changes in v2:
>> - Sort with two criteria, first misra rule, second filename
>>   (Michal, Jan)
>> ---
>> ---
>> xen/scripts/xen_analysis/cppcheck_report_utils.py | 7 +++++++
>> 1 file changed, 7 insertions(+)
>> 
>> diff --git a/xen/scripts/xen_analysis/cppcheck_report_utils.py 
>> b/xen/scripts/xen_analysis/cppcheck_report_utils.py
>> index 02440aefdfec..0b6cc72b9ac1 100644
>> --- a/xen/scripts/xen_analysis/cppcheck_report_utils.py
>> +++ b/xen/scripts/xen_analysis/cppcheck_report_utils.py
>> @@ -104,6 +104,13 @@ def cppcheck_merge_txt_fragments(fragments_list, 
>> out_txt_file, strip_paths):
>>                 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(":")
> This is where the for loop body ends so it should be separated from the rest 
> by an empty line.
> 
>> +            # sort alphabetically for second field (misra rule) and as 
>> second
> The second field is not necessary a "misra rule". It is just an error id 
> (e.g. unreadVariable).
> However this is just a python script and we use cppcheck mostly for MISRA so 
> I do not object.

Yes you are right, if the committer is willing to change it on commit I’ll 
appreciate, otherwise I can
fix it and respin the patch.

> 
>> +            # criteria for the first field (file name)
>> +            text_report_content.sort(key = lambda x: (x[1], x[0]))
>> +            # merge back with : separator
>> +            text_report_content = [":".join(x) for x in text_report_content]
>>             # Write the final text report
>>             outfile.writelines(text_report_content)
>>     except OSError as e:
>> --
>> 2.25.1
>> 
> 
> With the first remark fixed (e.g. on commit),
> Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

Thank you for your review

> 
> ~Michal