Re: [RFC PATCH 4/4] automation/eclair: add deviation for certain backwards goto

[Julien Grall <julien@xxxxxxx>]

On 07/11/2023 14:45, Nicola Vetrini wrote:
> Hi Julien,
Hi,

> On 2023-11-07 13:44, Julien Grall wrote:
> > > +in the community."
> > > +-config=MC3R1.R15.2,reports+={deliberate, 
> > > "any_area(any_loc(text(^.*goto (again|retry).*$)))"}
> > > +-doc_end
> > > +
> > >   #
> > >   # Series 20.
> > >   #
> > > diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> > > index 8511a189253b..7d1e1f0d09b3 100644
> > > --- a/docs/misra/deviations.rst
> > > +++ b/docs/misra/deviations.rst
> > > @@ -208,6 +208,16 @@ Deviations related to MISRA C:2012 Rules:
> > >          statements are deliberate.
> > >        - Project-wide deviation; tagged as `disapplied` for ECLAIR.
> > >   +   * - R15.2
> > > +     - The possible prevention of developer confusion as a result of 
> > > using
> > > +       control flow structures other than backwards goto-s in some 
> > > instances was
> > > +       deemed not strong enough to justify the additional complexity 
> > > introduced
> > > +       in the code. Such instances are the uses of the following 
> > > labels:
> > > +
> > > +       - again
> > > +       - retry
> > Have you investigated the possibility to use SAF-X in the code? If so, 
> > what's the problem to use them?
> > Cheers,
> This is another viable option: putting the SAF comment on top of the 
> label should suffice,
> as shown below:
>
> /* SAF-2-safe */
> repeat:
>      ++fmt;          /* this also skips first '%' */
>      switch (*fmt) {
>      case '-': flags |= LEFT; goto repeat;
>      case '+': flags |= PLUS; goto repeat;
>      case ' ': flags |= SPACE; goto repeat;
>      case '#': flags |= SPECIAL; goto repeat;
>      case '0': flags |= ZEROPAD; goto repeat;
>      }
>
> I think it ultimately boils down to whether Xen wants to promote the use 
> of certain labels
> as the designated alternative when no other control flow mechanism is 
> clearer from a
> readability perspective (in which case there should be a consistent 
> naming to capture and deviate
> all of them, such as "retry") or do so on a case-by-case basis with a 
> SAF, which is ok,
I would prefer a case-by-case basis because it adds an additional 
review. With deviating by keywords, the reviewrs/developpers may not be 
aware of the deviation (which may be fine for some, but IMHO not this one).
> but then
> it needs someone to check each one and either fix them or mark them as ok.
Don't we technically already need to go through all the existing use of 
ready & co even if we deviate by keyword?
> Yet another route could be to mark with a SAF all those present right 
> now to establish a baseline.
How many do we have?

Cheers,

--
Julien Grall