Re: [PATCH] docs/misra: add Rule 16.4

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 13 Mar 2024, Jan Beulich wrote:
> On 13.03.2024 01:28, Stefano Stabellini wrote:
> > --- a/docs/misra/rules.rst
> > +++ b/docs/misra/rules.rst
> > @@ -478,6 +478,30 @@ maintainers if you want to suggest a change.
> >       - In addition to break, also other unconditional flow control 
> > statements
> >         such as continue, return, goto are allowed.
> >  
> > +   * - `Rule 16.4 
> > <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_16_04.c>`_
> > +     - Required
> > +     - Every switch statement shall have a default label
> > +     - Switch statements with enums as controlling expression don't need
> > +       a default label as gcc -Wall enables -Wswitch which warns (and
> > +       breaks the build)
> 
> I think this could do with mentioning -Werror.

No problem


> > if one of the enum labels is missing from the
> > +       switch.
> > +
> > +       Switch statements with integer types as controlling expression
> > +       should have a default label:
> > +
> > +       - if the switch is expected to handle all possible cases
> > +         explicitly, then a default label shall be added to handle
> > +         unexpected error conditions, using BUG(), ASSERT(), WARN(),
> > +         domain_crash(), or other appropriate methods;
> > +
> > +       - if the switch is expected to handle a subset of all
> > +         possible cases, then a default label shall be added with an
> > +         in-code comment as follows::
> > +
> > +             /* only handle a subset of the possible cases */
> > +             default:
> > +                 break;
> 
> Unless it being made crystal clear that mechanically reproducing this
> comment isn't going to do, I'm going to have a hard time picking
> between actively vetoing or just accepting if someone else acks this.
> At the very least, though, the suggested (or, as requested, example)
> comment should match ./CODING_STYLE. And it may need placing
> differently if I understood correctly what Misra / Eclair demand
> (between default: and break; rather than ahead of both).
> 
> The only place I'd accept a pre-cooked comment is to cover the
> "notifier pattern".

Hi Jan, 

During the MISRA C call we discussed two distinct situations:

1) when the default is not supposed to happen (it could be an BUG_ON)
2) when we only handle a subset of cases on purpose

For 2), it is useful to have a comment to clarify that we are dealing
with 2) instead of 1). It is not a pre-cooked comment. The comment
should be reviewed and checked that it is actually true that for this
specific switch the default is expected and we should do nothing.

However, the comment is indeed pre-cooked in the sense that we don't
need to have several different variations of them to explain why we only
handle a subset of cases.

The majority of people on the call agreed with this (or so I understood).