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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

Jan