Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.4

[Federico Serafini <federico.serafini@xxxxxxxxxxx>]

On 23/04/24 12:26, Jan Beulich wrote:
> On 23.04.2024 12:02, Federico Serafini wrote:
> > --- a/docs/misra/deviations.rst
> > +++ b/docs/misra/deviations.rst
> > @@ -302,6 +302,19 @@ Deviations related to MISRA C:2012 Rules:
> >          leave such files as is.
> >        - Tagged as `deliberate` for ECLAIR.
> >   
> > +   * - R16.4
> > +     - Switch statements having a controlling expression of enum type
> > +       deliberately do not have a default case: gcc -Wall enables -Wswitch
> > +       which warns (and breaks the build as we use -Werror) if one of the enum
> > +       labels is missing from the switch.
> > +     - Tagged as `deliberate` for ECLAIR.
> > +
> > +   * - R16.4
> > +     - A switch statement with a single switch clause and no default label may
> > +       be used in place of an equivalent if statement if it is considered to
> > +       improve readability."
(I placed Rule 16.4 before Rule 16.3.
I will propose a new version with the correct ordering.)

> First a terminology related comment here: I'm afraid "switch clause" can be
> interpreted multiple ways, when I think we want to leave no room for
> interpretation here. It's not even clear to me whether
>
>      switch ( x )
>      {
>      case 1: case 2: case 3: case 4:
>          ...
>          break;
>      }
>
> would be covered by the deviation, or whether the multiple case labels
> wouldn't already be too much.
The MISRA C document, within Rule 16.1 ("A switch statement shall be
well-formed") defines the syntax rules that can be used to define a
"well formed" switch statement.
When I say "switch clause", I refer to the same entity the MISRA
document refers to in the definition of such syntax rules.
In the example above, we have a single switch clause with multiple
labels and no default label: this is a violation of Rule 16.4
("Every `switch' statement shall have a `default' label") which will
be covered by the deviation.
Do you think inserting the example in rules.rst or deviations.rst could
be useful?

> And then it is not clear to me why
>
>      switch ( x )
>      {
>      case 1:
>          ...
>          break;
>      default:
>          ...
>          break;
>      }
>
> shouldn't also be covered, as potentially a readability improvement /
> future change simplification over
>
>      if ( x == 1 )
>      {
>          ...
>      }
>      else
>      {
>          ...
>      }
Here there are two switch clauses,
each of them terminated by a break statement,
and the default label is present:
the switch is well formed, no violations of series 16 will
be reported.

--
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)