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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.05.2022 02:35, Stefano Stabellini wrote:
> --- a/CODING_STYLE
> +++ b/CODING_STYLE
> @@ -235,3 +235,9 @@ callstack between the initial function call and the 
> failure, no error
>  is returned.  Using domain_crash() requires careful inspection and
>  documentation of the code to make sure all callers at the stack handle
>  a newly-dead domain gracefully.
> +
> +MISRA C
> +-------
> +
> +The Xen Project hypervisor follows the MISRA C coding rules and
> +directives listed under docs/misra/rules.rst.

Putting this at the very bottom isn't helpful, I'm afraid. I'd rather
see this go directly after the initial paragraphs, before "Indentation".

> --- /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.

> +- 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.

> +- 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.

Jan

> +- Rule: Rule 8.8
> +  - Severity:  Required
> +  - Summary:  The static storage class specifier shall be used in all 
> declarations of objects and functions that have internal linkage
> +  - Link:  
> https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_08_08.c
> +- Rule: Rule 8.12
> +  - Severity:  Required
> +  - Summary:  Within an enumerator list the value of an implicitly-specified 
> enumeration constant shall be unique
> +  - Link:  
> https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_08_12.c