Advanced
Mailing List Archive

Mailing List Archive

  1. Home >
  2. Xen>
  3. Devel
[XEN PATCH 3/7] xen/sched: address a violation of MISRA C:2012 Rule 16.3
federico.serafini at bugseng
Apr 2, 2024, 12:22 AM
Post #1 of 6 (40 views)
Permalink
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;
--
2.34.1
Re: [XEN PATCH 3/7] xen/sched: address a violation of MISRA C:2012 Rule 16.3 [ In reply to ]
jbeulich at suse
Apr 2, 2024, 11:33 PM
Post #2 of 6 (39 views)
Permalink
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
Re: [XEN PATCH 3/7] xen/sched: address a violation of MISRA C:2012 Rule 16.3 [ In reply to ]
nicola.vetrini at bugseng
Apr 3, 2024, 12:52 AM
Post #3 of 6 (39 views)
Permalink
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.

--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
Re: [XEN PATCH 3/7] xen/sched: address a violation of MISRA C:2012 Rule 16.3 [ In reply to ]
sstabellini at kernel
Apr 4, 2024, 5:18 PM
Post #4 of 6 (34 views)
Permalink
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.
Re: [XEN PATCH 3/7] xen/sched: address a violation of MISRA C:2012 Rule 16.3 [ In reply to ]
jbeulich at suse
Apr 4, 2024, 11:43 PM
Post #5 of 6 (34 views)
Permalink
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
Re: [XEN PATCH 3/7] xen/sched: address a violation of MISRA C:2012 Rule 16.3 [ In reply to ]
sstabellini at kernel
Apr 5, 2024, 12:26 PM
Post #6 of 6 (31 views)
Permalink
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

©2024 GT.net. A Gossamer Threads company.
5th Floor, 455 Granville St., Vancouver, BC V6C 1T1, Canada | Legal