Re: [XEN PATCH 3/7] xen/sched: address a violation of MISRA C:2012 Rule 16.3

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 3 Apr 2024, Nicola Vetrini wrote:
> On 2024-04-03 08:33, Jan Beulich wrote:
> > On 02.04.2024 09:22, Federico Serafini wrote:
> > > Use pseudo-keyword fallthrough to meet the requirements to deviate
> > > MISRA C:2012 Rule 16.3 ("An unconditional `break' statement shall
> > > terminate every switch-clause").
> > > 
> > > No functional change.
> > > 
> > > Signed-off-by: Federico Serafini <federico.serafini@xxxxxxxxxxx>
> > > ---
> > >  xen/common/sched/credit2.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/xen/common/sched/credit2.c b/xen/common/sched/credit2.c
> > > index c76330d79d..0962b52415 100644
> > > --- a/xen/common/sched/credit2.c
> > > +++ b/xen/common/sched/credit2.c
> > > @@ -3152,8 +3152,8 @@ static int cf_check csched2_sys_cntl(
> > >              printk(XENLOG_INFO "Disabling context switch rate
> > > limiting\n");
> > >          prv->ratelimit_us = params->ratelimit_us;
> > >          write_unlock_irqrestore(&prv->lock, flags);
> > > +        fallthrough;
> > > 
> > > -    /* FALLTHRU */
> > >      case XEN_SYSCTL_SCHEDOP_getinfo:
> > >          params->ratelimit_us = prv->ratelimit_us;
> > >          break;
> > 
> > Hmm, the description doesn't say what's wrong with the comment. Furthermore
> > docs/misra/rules.rst doesn't mention "fallthrough" at all, nor the
> > alternative of using comments. I notice docs/misra/deviations.rst does, and
> > there the specific comment used here isn't covered. That would want saying
> > in the description.
> > 
> > Stefano (and others) - in this context it becomes noticeable that having
> > stuff scattered across multiple doc files isn't necessarily helpful. Other
> > permissible keywords are mentioned in rules.rst. The pseudo-keyword
> > "fallthrough" as well as comments are mentioned on deviations.rst. Could
> > you remind me of the reason(s) why things aren't recorded in a single,
> > central place?
> > 
> > Jan
> 
> If I recall correctly, the idea was to avoid rules.rst from getting too long
> and too specific about which patterns were deviated, while also having a
> precise record of the MISRA deviations that didn't live in ECLAIR-specific
> files. Maybe the use of the pseudo-keyword emerged after the rule was added to
> rules.rst, since deviations.rst is updated more frequently.

Yes exactly.

I agree with Jan that a single central place is easiest but we cannot
move everything that is in deviations.rst in the note section of the
rules.rst table. Of the two, it would be best to reduce the amount of
notes in rules.rst and move all the deviations listed in rules.rst to
deviations.rst. That way at least the info is present only once,
although they are 2 files.