|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 2/2] xen/misra: add entries to exclude-list.json
> On 16 Feb 2023, at 08:19, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 16.02.2023 00:49, Stefano Stabellini wrote:
>> On Wed, 15 Feb 2023, Julien Grall wrote:
>>> On 14/02/2023 22:25, Stefano Stabellini wrote:
>>>>> Patch 1's example has a "comment" field, which no entry makes use of.
>>>>> Without that, how does it become clear _why_ a particular file is to
>>>>> be excluded? The patch description here also doesn't provide any
>>>>> justification ...
>>>>
>>>> It would be good to have a couple of pre-canned justifications as the
>>>> reason for excluding one file could be different from the reason for
>>>> excluding another file. Some of the reasons:
>>>
>>> I think the reasons should be ambiguous. This is ...
>>>
>>>> - imported from Linux
>>>
>>> ... the case here but...
>>>
>>> This reason is pretty clear to me but...
>>>
>>>> - too many false positives
>>>
>>> ... not here. What is too many?
>>>
>>>> That said, we don't necessarily need to know the exact reason for
>>>> excluding one file to be able to start scanning xen with cppcheck
>>>> automatically. If someone wants to remove a file from the exclude list
>>>> in the future they just need to show that cppcheck does a good job at
>>>> scanning the file and we can handle the number of violations.
>>>
>>> I disagree. A good reasoning from the start will be helpful to decide when
>>> we
>>> can remove a file from the list. Furthermore, we need to set good example
>>> for
>>> any new file we want to exclude.
>>>
>>> Furthermore, if we exclude some files, then it will be difficult for the
>>> reviewers to know when they can be removed from the list. What if this is
>>> fine
>>> with CPPCheck but not EClair (or any other)?
>>
>> Yes, the reason would help. In previous incarnations of this work, there
>> was a request for detailed information on external files, such as:
>> - where this file is coming from
>> - if coming from Linux, which version of Linux
>> - maintenance status
>> - coding style
>>
>> But this is not what you are asking. You are only asking for a reason
>> and "imported from Linux" would be good enough. Please correct me if I
>> am wrong.
>
> I guess you mean s/would/could/. Personally I find "imported from Linux"
> as an entirely unacceptable justification: Why would the origin of a file
> matter on whether it has violations? Dealing with the violations may be
> more cumbersome (because preferably the adjustments would go to the
> original files first). Yet not dealing with them - especially if there
> are many - reduces the benefit of the work we do quite a bit, because it
> may leave much more work for downstreams to do to actually be able to do
> any certification. That may go to the extent of questioning why we would
> bother dealing with a few dozen violations if hundreds remain but are
> hidden.
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?
{
"version": "1.0",
"content": [
{
"rel_path": "arch/arm/arm64/cpufeature.c",
"comment": "Imported from Linux"
},
{
"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"
},
{
"rel_path": "arch/x86/acpi/boot.c",
"comment": "Imported from Linux"
},
{
"rel_path": "arch/x86/acpi/cpu_idle.c",
"comment": "Imported from Linux"
},
{
"rel_path": "arch/x86/acpi/cpufreq/cpufreq.c",
"comment": "Imported from Linux"
},
{
"rel_path": "arch/x86/acpi/cpuidle_menu.c",
"comment": "Imported from Linux"
},
{
"rel_path": "arch/x86/acpi/lib.c",
"comment": "Imported from Linux"
},
{
"rel_path": "arch/x86/cpu/amd.c",
"comment": "Imported from Linux"
},
{
"rel_path": "arch/x86/cpu/centaur.c",
"comment": "Imported from Linux"
},
{
"rel_path": "arch/x86/cpu/common.c",
"comment": "Imported from Linux"
},
{
"rel_path": "arch/x86/cpu/hygon.c",
"comment": "Imported from Linux"
},
{
"rel_path": "arch/x86/cpu/intel.c",
"comment": "Imported from Linux"
},
{
"rel_path": "arch/x86/cpu/intel_cacheinfo.c",
"comment": "Imported from Linux"
},
{
"rel_path": "arch/x86/cpu/mcheck/non-fatal.c",
"comment": "Imported from Linux"
},
{
"rel_path": "arch/x86/cpu/mtrr/*",
"comment": "Imported from Linux"
},
{
"rel_path": "arch/x86/cpu/mwait-idle.c",
"comment": "Imported from Linux"
},
{
"rel_path": "arch/x86/delay.c",
"comment": "Imported from Linux"
},
{
"rel_path": "arch/x86/dmi_scan.c",
"comment": "Imported from Linux"
},
{
"rel_path": "arch/x86/mpparse.c",
"comment": "Imported from Linux"
},
{
"rel_path": "arch/x86/srat.c",
"comment": "Imported from Linux"
},
{
"rel_path": "arch/x86/time.c",
"comment": "Imported from Linux"
},
{
"rel_path": "arch/x86/x86_64/mmconf-fam10h.c",
"comment": "Imported from Linux"
},
{
"rel_path": "common/bitmap.c",
"comment": "Imported from Linux"
},
{
"rel_path": "common/bunzip2.c",
"comment": "Imported from Linux"
},
{
"rel_path": "common/earlycpio.c",
"comment": "Imported from Linux"
},
{
"rel_path": "common/inflate.c",
"comment": "Imported from Linux"
},
{
"rel_path": "common/libfdt/*",
"comment": "External library"
},
{
"rel_path": "common/lz4/decompress.c",
"comment": "Imported from Linux"
},
{
"rel_path": "common/radix-tree.c",
"comment": "Imported from Linux"
},
{
"rel_path": "common/ubsan/ubsan.c",
"comment": "Imported from Linux"
},
{
"rel_path": "common/un*.c",
"comment": "unlz4.c implementation by Yann Collet, the others un*
are from Linux"
},
{
"rel_path": "common/xz/*",
"comment": "Imported from Linux"
},
{
"rel_path": "common/zstd/*",
"comment": "Imported from Linux"
},
{
"rel_path": "crypto/*",
"comment": "Origin is external and documented in
crypto/README.source"
},
{
"rel_path": "drivers/acpi/apei/*",
"comment": "Imported from Linux"
},
{
"rel_path": "drivers/acpi/hwregs.c",
"comment": "Imported from Linux"
},
{
"rel_path": "drivers/acpi/numa.c",
"comment": "Imported from Linux"
},
{
"rel_path": "drivers/acpi/osl.c",
"comment": "Imported from Linux"
},
{
"rel_path": "drivers/acpi/tables.c",
"comment": "Imported from Linux"
},
{
"rel_path": "drivers/acpi/tables/*",
"comment": "Imported from Linux"
},
{
"rel_path": "drivers/acpi/utilities/*",
"comment": "Imported from Linux"
},
{
"rel_path": "drivers/video/font_*",
"comment": "Imported from Linux"
},
{
"rel_path": "lib/list-sort.c",
"comment": "Imported from Linux"
},
{
"rel_path": "lib/rbtree.c",
"comment": "Imported from Linux"
},
{
"rel_path": "lib/xxhash*.c",
"comment": "Imported from Linux"
}
]
}
Cheers,
Luca
>
> Jan
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |