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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 25 May 2022, Julien Grall wrote:
> Hi Stefano,
> 
> On 25/05/2022 01:35, Stefano Stabellini wrote:
> > From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> > 
> > Introduce a list of MISRA C rules that apply to the Xen hypervisor. The
> > list is in RST format.
> > 
> > Add a mention of the new list to CODING_STYLE.
> > 
> > Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> 
> NIT: I was under the impression that the first Signed-off-by is usually
> author. But the From doesn't match.
> 
> > ---
> >   CODING_STYLE         |  6 ++++
> >   docs/misra/rules.rst | 65 ++++++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 71 insertions(+)
> >   create mode 100644 docs/misra/rules.rst
> > 
> > diff --git a/CODING_STYLE b/CODING_STYLE
> > index 9f50d9cec4..1ef35ee8d0 100644
> > --- 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.
> > diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
> > new file mode 100644
> > index 0000000000..c0ee58ab25
> > --- /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.
> I was under the impression that we would still allow deviations on those rules
> in some cases. In particular...
> > +- 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
> 
> 
> ... this one. We are using (void) + a comment when the return is ignored on
> purpose. This is technically not-compliant with MISRA but the best we can do
> in some situation.
> 
> With your proposed wording, we would technically have to remove them (or not
> introduce new one). So I think we need to document that we are allowing
> deviations so long they are commented.

Absolutely yes. All of these rules can have deviations as long as they
make sense and they are commented. Note that we still have to work out
a good tagging system so that ECLAIR and cppcheck can recognize the
deviations automatically but for now saying that they need to be
commented is sufficient I think.

So I'll add the following on top of the file:

"""
It is possible that in specific circumstances it is best not to follow a
rule because it is not possible or because the alternative leads to
better code quality. Those cases are called "deviations". They are
permissible as long as they are documented with an in-code comment.
"""