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

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2023-11-07 18:35, Julien Grall wrote:
> 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).
Ok, I'll keep this in mind when the rule will be discussed.

> > 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?
my hope was trying to extract a common well-known pattern that can be
defensible as a deviation and then fix the rest.

> > Yet another route could be to mark with a SAF all those present right 
> > now to establish a baseline.
> How many do we have?
~30 in Arm (half of which are in common code) and ~250 in x86. The 
actual count of labels
is lower, because a report is given for each use of a backward jump, but 
those under
x86e_emulate likely do not (~40 total on x86 remain if we exclude 
x86_emulate/.*).
https://saas.eclairit.com:3787/fs/var/local/eclair/XEN.ecdf/ECLAIR_normal/arm/for-4.19/ARM64-Set3/372/PROJECT.ecd;/by_service/MC3R1.R15.2.html

https://saas.eclairit.com:3787/fs/var/local/eclair/XEN.ecdf/ECLAIR_normal/x86_64/staging/X86_64-Set3/372/PROJECT.ecd;/by_service/MC3R1.R15.2.html#

> Cheers,
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)