Re: [PATCH 2/2] xen/misra: add entries to exclude-list.json

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

Hi Stefano,

>> Hi Jan,
>> 
>> my personal opinion is that we can’t handle them for files that needs to be 
>> kept
>> in sync from an external source, because we can’t justify findings or tag 
>> false
>> positives, if we are lucky we might fix the non compliances but even in that 
>> case
>> there might be times when the fix goes through and cases where the fix is 
>> objected
>> by the maintainers just because their project is not following the MISRA 
>> standard.
>> 
>> On our side, what we can do is track these files and from time to time check 
>> that
>> they are still eligible to backport, because when they are not anymore we 
>> could
>> just port them to Xen coding style and start doing direct changes.
>> 
>> 
>> @Stefano/@Bertrand/@Julien thanks for your effort on the list, yesterday 
>> morning
>> I’ve also had a look on the Michal’s file list and I’ve tracked down the 
>> origin, here
>> my findings, maybe we could merge your list with mine?
> 
> Yes to merge the two lists, but I think we should keep only items that we
> actually need for the sake of having a cleaner baseline. So I would only
> add things we need today to reduce the noise on cppcheck results
> (excluding 20.7 and also excluding unusedStructMember which I think we
> should probably stop scanning with cppcheck altogether).

Yes I thought about excluding unusedStructMember, I see there are a lot of 
findings on x86
which are not real findings, it’s just that the tool has not the complete 
picture.

Here the ways are two:
1) exclude globally unusedStructMember
2) exclude unusedStructMember only on some selected files (available only on 
cppcheck)
    this requires some work to add a field to this list, like 
“cppcheck-error-exclude” and a list,
    comma separated, of error-id to be suppressed from the corresponding file.

Regarding the list, I merged your list with mine (and Michal’s work) to create 
a complete list,
I think it’s better to have it complete because all those files are external 
and even if today we don’t
have findings for some of these files, we could have some in the future, and 
since we know today 
that we can’t do direct changes to them, in my opinion it’s better to list them 
now instead of forgetting
them later.

I left out for now these files to start a discussion for them, because I think 
they should be included in
the analysis:

common/symbols-dummy.c:
  this file accepts direct changes, cppcheck complains about misra-c2012-8.4 
but I think it is right, also
  Coverity complains about the same findings, they can be fixed adding 
declarations on symbols.h I think
  and removing the declarations from symbol.c module

common/version.c:
  Apart from unusedStructMember, cppcheck is confused only on 2 findings that 
compares one local symbol
  and one linker defined symbol, could we have just one tag to suppress those 
two findings instead of removing
  completely the file? This file is under our control, so we could push changes.

include/xen/lib.h:
  Findings are from the bsearch function, which is derived from Linux, but I 
can see the codestyle is the Xen style
  and it seems (I might be wrong) that it has diverged from Linux, so we might 
do changes and fix the findings that
  are correct, void* arithmetic is working because gcc make it work assigning a 
sizeof of 1, using char* pointers we
  have the same result without having the undefined behaviour (correct me if I 
am wrong).

include/xen/sort.h:
  Also this one is derived from Linux, but seems that we converted it to our 
coding style and we can do direct changes,
  the same reasoning about void* pointers arithmetic applies here.


What are your thoughts?

Here the merged list, capturing also Jan comments:

{
   "version": "1.0",
   "content": [
       {
           "rel_path": "arch/arm/arm64/cpufeature.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/arm/arm64/insn.c",
           "comment": "Imported on Linux"
       },
       {
           "rel_path": "arch/arm/arm64/lib/find_next_bit.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/x86/acpi/boot.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/x86/acpi/cpu_idle.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/x86/acpi/cpufreq/cpufreq.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/x86/acpi/cpuidle_menu.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/x86/acpi/lib.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/x86/cpu/amd.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/x86/cpu/centaur.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/x86/cpu/common.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/x86/cpu/hygon.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/x86/cpu/intel.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/x86/cpu/intel_cacheinfo.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/x86/cpu/mcheck/non-fatal.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/x86/cpu/mtrr/*",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/x86/cpu/mwait-idle.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/x86/delay.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/x86/dmi_scan.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/x86/mpparse.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/x86/srat.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/x86/time.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "arch/x86/x86_64/mmconf-fam10h.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "common/bitmap.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "common/bunzip2.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "common/earlycpio.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "common/inflate.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "common/libfdt/*",
           "comment": "External library"
       },
       {
           "rel_path": "common/livepatch_elf.c",
           "comment" : "Not in scope initially as it generates many violations 
and it is not enabled in safety configurations"
       },
       {
           "rel_path": "common/lzo.c",
           "comment" : "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "common/lz4/decompress.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "common/radix-tree.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "common/ubsan/ubsan.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "common/un*.c",
           "comment": "unlz4.c implementation by Yann Collet, the others un* 
are from Linux, ignore for now"
       },
       {
           "rel_path": "common/xz/*",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "common/zstd/*",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "crypto/*",
           "comment": "Origin is external and documented in 
crypto/README.source"
       },
       {
           "rel_path": "drivers/acpi/apei/*",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "drivers/acpi/hwregs.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "drivers/acpi/numa.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "drivers/acpi/osl.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "drivers/acpi/tables.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "drivers/acpi/tables/*",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "drivers/acpi/utilities/*",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "drivers/video/font_*",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "lib/list-sort.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "lib/rbtree.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "lib/xxhash*.c",
           "comment": "Imported from Linux, ignore for now"
       },
       {
           "rel_path": "xsm/flask/*",
           "comment" : "Not in scope initially as it generates many violations 
and it is not enabled in safety configurations"
       }
   ]
}