| Message ID | 8f91430e37594831dd8d92ab630477be88417b49.1712042178.git.federico.serafini@bugseng.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: address violations of MISRA C:2012 Rule 16.3 | expand |
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@bugseng.com> > --- > 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
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@bugseng.com> >> --- >> 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.
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@bugseng.com> > > > --- > > > 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.
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@bugseng.com> >>>> --- >>>> 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? Jan
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@bugseng.com> > >>>> --- > >>>> 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
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;
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@bugseng.com> --- xen/common/sched/credit2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)