Re: [PATCH 1/2] docs/misra: introduce rules.rst

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.05.2022 03:12, Stefano Stabellini wrote:
> On Wed, 25 May 2022, Jan Beulich wrote:
>> On 25.05.2022 02:35, Stefano Stabellini wrote:
>>> --- /dev/null
>>> +++ b/docs/misra/rules.rst
>>> @@ -0,0 +1,65 @@
>>> +=====================
>>> +MISRA C rules for Xen
>>> +=====================
>>> +
>>> +**IMPORTANT** All MISRA C rules, text, and examples are copyrighted by the
>>> +MISRA Consortium Limited and used with permission.
>>> +
>>> +Please refer to https://www.misra.org.uk/ to obtain a copy of MISRA C, or 
>>> for
>>> +licensing options for other use of the rules.
>>> +
>>> +The following is the list of MISRA C rules that apply to the Xen Project
>>> +hypervisor.
>>> +
>>> +- Rule: Dir 2.1
>>> +  - Severity:  Required
>>> +  - Summary:  All source files shall compile without any compilation errors
>>> +  - Link:  
>>> https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_02_01.c
>>> +- Rule: Dir 4.7
>>> +  - Severity:  Required
>>> +  - Summary:  If a function returns error information then that error 
>>> information shall be tested
>>> +  - Link:  
>>> https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_04_07.c
>>> +- Rule: Dir 4.10
>>> +  - Severity:  Required
>>> +  - Summary:  Precautions shall be taken in order to prevent the contents 
>>> of a header file being included more than once
>>> +  - Link:  
>>> https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_04_10.c
>>
>> Like Julien has already pointed out for 4.7, this and perhaps other ones
>> also want clarifying somewhere that we expect certain exceptions. Without
>> saying so explicitly, someone could come forward with a patch eliminating
>> some uses (and perhaps crippling the code) just to satisfy such a rule.
>> This would then be a waste of both their and our time.
> 
> Yes, and also Julien pointed out something similar. I'll add a statement
> at the top of the file to say that there can be deviations as long as
> they are commented.

We need to determine where such comments are to go. I hope you don't
mean code comments on each and every instance of similar-kind
deviations.

> I wouldn't go as far as adding a note to each specific rule where we
> expect deviations because I actually imagine that many of them will end
> up having at least one deviation or two. It would be easier to mark the
> ones where we expect to have 100% compliance and intend to keep it that
> way (once we reach 100% compliance).
> 
> We are still in the early days of this process. For now, what about
> adding the following statement at the top of the file (in addition to
> the one saying that deviations are permissible):
> 
> """
> The existing codebase is not 100% compliant with the rules. Some of the
> violations are meant to be documented as deviations, while some others
> should be fixed. Both compliance and documenting deviations on the
> existing codebase is work-in-progress.
> """
> 
> 
>>> +- Rule: Dir 4.14
>>> +  - Severity:  Required
>>> +  - Summary:  The validity of values received from external sources shall 
>>> be checked
>>> +  - Link:  
>>> https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_04_14.c
>>> +- Rule: Rule 1.3
>>> +  - Severity:  Required
>>> +  - Summary:  There shall be no occurrence of undefined or critical 
>>> unspecified behaviour
>>> +  - Link:  
>>> https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_01_03.c
>>> +- Rule: Rule 3.2
>>> +  - Severity:  Required
>>> +  - Summary:  Line-splicing shall not be used in // comments
>>> +  - Link:  
>>> https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_03_02.c
>>
>> To aid easily looking up presence of a rule here, I think the table wants
>> sorting numerically.
> 
> They are sorted numerically, first the "Dir" (directives) then the
> "Rules".

Oh, I see. I didn't recognize the distinction. Maybe have a visual
separator between the two classes?

>>> +- Rule: Rule 6.2
>>> +  - Severity:  Required
>>> +  - Summary:  Single-bit named bit fields shall not be of a signed type
>>> +  - Link:  
>>> https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_06_02.c
>>> +- Rule: Rule 8.1
>>> +  - Severity:  Required
>>> +  - Summary:  Types shall be explicitly specified
>>> +  - Link:  
>>> https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_08_01.c
>>> +- Rule: Rule 8.4
>>> +  - Severity:  Required
>>> +  - Summary:  A compatible declaration shall be visible when an object or 
>>> function with external linkage is defined
>>> +  - Link:  
>>> https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_08_04.c
>>> +- Rule: Rule 8.5
>>> +  - Severity:  Required
>>> +  - Summary:  An external object or function shall be declared once in one 
>>> and only one file
>>> +  - Link:  
>>> https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_08_05_2.c
>>> +- Rule: Rule 8.6
>>> +  - Severity:  Required
>>> +  - Summary:  An identifier with external linkage shall have exactly one 
>>> external definition
>>> +  - Link:  
>>> https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_08_06_2.c
>>
>> I don't think this was uncontroversial, as we've got a lot of uses of
>> declarations when we expect DCE to actually take out all uses. There
>> are also almost a thousand violations, which - imo - by itself speaks
>> against adoption.
> 
> Ah yes, good catch. We spoke about DCE in the context of Rule 2.1, not
> this one. My preference would be to keep Rule 8.6 with a note allowing
> DCE:
> 
> - Note: declarations without definitions are allowed (specifically when
>   the definition is compiled-out or optimized-out by the compiler)

I'd be fine with that.

Jan