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

[Stefano Stabellini <sstabellini@kernel.org>]

On Thu, 26 May 2022, Jan Beulich wrote:
> 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'll reply to this in the other thread.


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

I'll try but the layout changed significantly to become "proper RST"
following Andrew's comments. I'll see if I can come up with something.
If not, I could create two tables. First Dir, then Rules.


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

Cool, I'll add it in.