[XEN PATCH 0/7] xen: address violations of MISRA C:2012 Rule 16.3

[XEN PATCH 0/7] xen: address violations of MISRA C:2012 Rule 16.3

[Federico Serafini]

This patch series addresses some violations of Rule 16.3 by adding the missing
break statements and pseudo-keyword fallthrough.

Federico Serafini (7):
  xen/domctl: address a violation of MISRA C:2012 Rule 16.3
  console: address a violation of MISRA C:2012 Rule 16.3
  xen/sched: address a violation of MISRA C:2012 Rule 16.3
  xen/evtchn: address a violation of MISRA C:2012 Rule 16.3
  xen/sched: address a violation of MISRA C:2012 Rule 16.3
  xen/vm-event: address a violation of MISRA C:2012 Rule 16.3
  vsprintf: address violations of MISRA C:2012 Rule 16.3

 xen/common/domctl.c        | 1 +
 xen/common/event_channel.c | 1 +
 xen/common/sched/core.c    | 1 +
 xen/common/sched/credit2.c | 2 +-
 xen/common/vm_event.c      | 1 +
 xen/common/vsprintf.c      | 6 +++++-
 xen/drivers/char/console.c | 2 ++
 7 files changed, 12 insertions(+), 2 deletions(-)

-- 
2.34.1

[XEN PATCH 1/7] xen/domctl: address a violation of MISRA C:2012 Rule 16.3

[Federico Serafini]

Add break statement to address a violation of 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/domctl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index d94a9dae91..f2e0e36a17 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -316,6 +316,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
         d = rcu_lock_domain_by_id(op->domain);
         if ( !d )
             return -ESRCH;
+        break;
     }
 
     ret = xsm_domctl(XSM_OTHER, d, op->cmd);
-- 
2.34.1

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

[Jan Beulich]

On 02.04.2024 09:22, Federico Serafini wrote:
> Add break statement to address a violation of 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>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

[XEN PATCH 2/7] console: address a violation of MISRA C:2012 Rule 16.3

[Federico Serafini]

Add break statement to address a violation of 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/drivers/char/console.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index ccd5f8cc14..e185f80efe 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -568,6 +568,8 @@ static void __serial_rx(char c)
 
         if ( d != NULL )
             rcu_unlock_domain(d);
+
+        break;
     }
 #endif
     }
-- 
2.34.1

Re: [XEN PATCH 2/7] console: address a violation of MISRA C:2012 Rule 16.3

[Jan Beulich]

On 02.04.2024 09:22, Federico Serafini wrote:
> Add break statement to address a violation of 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>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

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

[Federico Serafini]

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

[Jan Beulich]

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

[Nicola Vetrini]

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

[Stefano Stabellini]

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

[Jan Beulich]

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

[Stefano Stabellini]

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

[XEN PATCH 4/7] xen/evtchn: address a violation of MISRA C:2012 Rule 16.3

[Federico Serafini]

Add break statement to address a violation of 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/event_channel.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index 20f586cf5e..aceee0695f 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -853,6 +853,7 @@ int evtchn_send(struct domain *ld, unsigned int lport)
         break;
     default:
         ret = -EINVAL;
+        break;
     }
 
 out:
-- 
2.34.1

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

[Jan Beulich]

On 02.04.2024 09:22, Federico Serafini wrote:
> Add break statement to address a violation of 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>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

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

[Federico Serafini]

Add break statement to address a violation of 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/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
index 3e48da1cdd..0cb33831d2 100644
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -2007,6 +2007,7 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 
     default:
         ret = -ENOSYS;
+        break;
     }
 
     return ret;
-- 
2.34.1

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

[Jan Beulich]

On 02.04.2024 09:22, Federico Serafini wrote:
> Add break statement to address a violation of 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>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

However, this is the 2nd patch in this series with exactly the same title.
Going just from titles, one option would be to fold both. The other option
would be to change the title prefix of patch 3 to "xen/credit2:".

Jan

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

[Federico Serafini]

On 03/04/24 08:55, Jan Beulich wrote:
> On 02.04.2024 09:22, Federico Serafini wrote:
>> Add break statement to address a violation of 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>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> However, this is the 2nd patch in this series with exactly the same title.
> Going just from titles, one option would be to fold both. The other option
> would be to change the title prefix of patch 3 to "xen/credit2:".

I'll go for the first option.

-- 
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)

[XEN PATCH 6/7] xen/vm-event: address a violation of MISRA C:2012 Rule 16.3

[Federico Serafini]

Add break statement to address a violation of 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/vm_event.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index ecf49c38a9..fbf1aa0848 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -770,6 +770,7 @@ int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec)
 
     default:
         rc = -ENOSYS;
+        break;
     }
 
     return rc;
-- 
2.34.1

Re: [XEN PATCH 6/7] xen/vm-event: address a violation of MISRA C:2012 Rule 16.3

[Tamas K Lengyel]

On Tue, Apr 2, 2024 at 3:24 AM Federico Serafini
<federico.serafini@bugseng.com> wrote:
>
> Add break statement to address a violation of 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>

Acked-by: Tamas K Lengyel <tamas@tklengyel.com>

[XEN PATCH 7/7] vsprintf: address violations of MISRA C:2012 Rule 16.3

[Federico Serafini]

MISRA C:2012 Rule 16.3 states: "An unconditional `break' statement
shall terminate every switch-clause".

Add break statement to address violations of the rule or add
pseudo-keyword fallthrough to meet the requirements to deviate it.

No functional change.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
---
 xen/common/vsprintf.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/xen/common/vsprintf.c b/xen/common/vsprintf.c
index c49631c0a4..612751c90f 100644
--- a/xen/common/vsprintf.c
+++ b/xen/common/vsprintf.c
@@ -377,7 +377,7 @@ static char *pointer(char *str, const char *end, const char **fmt_ptr,
             str = number(str, end, hex_buffer[i], 16, 2, -1, ZEROPAD);
 
             if ( ++i == field_width )
-                return str;
+                break;
 
             if ( sep )
             {
@@ -386,6 +386,8 @@ static char *pointer(char *str, const char *end, const char **fmt_ptr,
                 ++str;
             }
         }
+
+        return str;
     }
 
     case 'p': /* PCI SBDF. */
@@ -619,6 +621,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
 
         case 'X':
             flags |= LARGE;
+            fallthrough;
         case 'x':
             base = 16;
             break;
@@ -626,6 +629,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
         case 'd':
         case 'i':
             flags |= SIGN;
+            fallthrough;
         case 'u':
             break;
 
-- 
2.34.1

Re: [XEN PATCH 7/7] vsprintf: address violations of MISRA C:2012 Rule 16.3

[Jan Beulich]

On 02.04.2024 09:22, Federico Serafini wrote:
> MISRA C:2012 Rule 16.3 states: "An unconditional `break' statement
> shall terminate every switch-clause".
> 
> Add break statement to address violations of the rule or add
> pseudo-keyword fallthrough to meet the requirements to deviate it.

While the latter half of the sentence properly describes the latter
two of the hunks, the former half doesn't match the former two hunks
at all:

> --- a/xen/common/vsprintf.c
> +++ b/xen/common/vsprintf.c
> @@ -377,7 +377,7 @@ static char *pointer(char *str, const char *end, const char **fmt_ptr,
>              str = number(str, end, hex_buffer[i], 16, 2, -1, ZEROPAD);
>  
>              if ( ++i == field_width )
> -                return str;
> +                break;

This "break" is inside for(), not switch().

> @@ -386,6 +386,8 @@ static char *pointer(char *str, const char *end, const char **fmt_ptr,
>                  ++str;
>              }
>          }
> +
> +        return str;
>      }

And this "return" is what now "delimits" case 'h' of the switch(). The
original situation therefore was that the for() could not be exited by
other than the "return" inside. The supposedly missing "break" in that
arrangement would have been "unreachable code", i.e. violate another
rule. Hence the (undescribed) further re-arrangement.

Jan

Re: [XEN PATCH 7/7] vsprintf: address violations of MISRA C:2012 Rule 16.3

[Federico Serafini]

On 03/04/24 09:06, Jan Beulich wrote:
> On 02.04.2024 09:22, Federico Serafini wrote:
>> MISRA C:2012 Rule 16.3 states: "An unconditional `break' statement
>> shall terminate every switch-clause".
>>
>> Add break statement to address violations of the rule or add
>> pseudo-keyword fallthrough to meet the requirements to deviate it.
> 
> While the latter half of the sentence properly describes the latter
> two of the hunks, the former half doesn't match the former two hunks
> at all:
> 
>> --- a/xen/common/vsprintf.c
>> +++ b/xen/common/vsprintf.c
>> @@ -377,7 +377,7 @@ static char *pointer(char *str, const char *end, const char **fmt_ptr,
>>               str = number(str, end, hex_buffer[i], 16, 2, -1, ZEROPAD);
>>   
>>               if ( ++i == field_width )
>> -                return str;
>> +                break;
> 
> This "break" is inside for(), not switch().
> 
>> @@ -386,6 +386,8 @@ static char *pointer(char *str, const char *end, const char **fmt_ptr,
>>                   ++str;
>>               }
>>           }
>> +
>> +        return str;
>>       }
> 
> And this "return" is what now "delimits" case 'h' of the switch(). The
> original situation therefore was that the for() could not be exited by
> other than the "return" inside. The supposedly missing "break" in that
> arrangement would have been "unreachable code", i.e. violate another
> rule. Hence the (undescribed) further re-arrangement.

I'll improve the description.

-- 
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)