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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 5 Apr 2024, Jan Beulich wrote:
> On 05.04.2024 02:18, Stefano Stabellini wrote:
> > 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.
> 
> Could every rules.rst section having a deviations.rst counterpart then perhaps
> have a standardized referral to there?

+1