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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 24 Feb 2023, Luca Fancellu wrote:
> 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.

For now, I would exclude globally


> 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?

I am good with this


> 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"
>        }
>    ]
> }
> 
> 
>