* [XEN PATCH 0/7] xen: address violations of MISRA C:2012 Rule 16.3
@ 2024-04-02 7:22 Federico Serafini
2024-04-02 7:22 ` [XEN PATCH 1/7] xen/domctl: address a violation " Federico Serafini
` (6 more replies)
0 siblings, 7 replies; 21+ messages in thread
From: Federico Serafini @ 2024-04-02 7:22 UTC (permalink / raw
To: xen-devel
Cc: consulting, Federico Serafini, Andrew Cooper, George Dunlap,
Jan Beulich, Julien Grall, Stefano Stabellini, Dario Faggioli,
Tamas K Lengyel, Alexandru Isaila, Petre Pircalabu
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [XEN PATCH 1/7] xen/domctl: address a violation of MISRA C:2012 Rule 16.3
2024-04-02 7:22 [XEN PATCH 0/7] xen: address violations of MISRA C:2012 Rule 16.3 Federico Serafini
@ 2024-04-02 7:22 ` Federico Serafini
2024-04-03 6:23 ` Jan Beulich
2024-04-02 7:22 ` [XEN PATCH 2/7] console: " Federico Serafini
` (5 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Federico Serafini @ 2024-04-02 7:22 UTC (permalink / raw
To: xen-devel
Cc: consulting, Federico Serafini, Andrew Cooper, George Dunlap,
Jan Beulich, Julien Grall, Stefano Stabellini
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [XEN PATCH 2/7] console: address a violation of MISRA C:2012 Rule 16.3
2024-04-02 7:22 [XEN PATCH 0/7] xen: address violations of MISRA C:2012 Rule 16.3 Federico Serafini
2024-04-02 7:22 ` [XEN PATCH 1/7] xen/domctl: address a violation " Federico Serafini
@ 2024-04-02 7:22 ` Federico Serafini
2024-04-03 6:24 ` Jan Beulich
2024-04-02 7:22 ` [XEN PATCH 3/7] xen/sched: " Federico Serafini
` (4 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Federico Serafini @ 2024-04-02 7:22 UTC (permalink / raw
To: xen-devel
Cc: consulting, Federico Serafini, Andrew Cooper, George Dunlap,
Jan Beulich, Julien Grall, Stefano Stabellini
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [XEN PATCH 3/7] xen/sched: address a violation of MISRA C:2012 Rule 16.3
2024-04-02 7:22 [XEN PATCH 0/7] xen: address violations of MISRA C:2012 Rule 16.3 Federico Serafini
2024-04-02 7:22 ` [XEN PATCH 1/7] xen/domctl: address a violation " Federico Serafini
2024-04-02 7:22 ` [XEN PATCH 2/7] console: " Federico Serafini
@ 2024-04-02 7:22 ` Federico Serafini
2024-04-03 6:33 ` Jan Beulich
2024-04-02 7:22 ` [XEN PATCH 4/7] xen/evtchn: " Federico Serafini
` (3 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Federico Serafini @ 2024-04-02 7:22 UTC (permalink / raw
To: xen-devel; +Cc: consulting, Federico Serafini, George Dunlap, Dario Faggioli
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [XEN PATCH 4/7] xen/evtchn: address a violation of MISRA C:2012 Rule 16.3
2024-04-02 7:22 [XEN PATCH 0/7] xen: address violations of MISRA C:2012 Rule 16.3 Federico Serafini
` (2 preceding siblings ...)
2024-04-02 7:22 ` [XEN PATCH 3/7] xen/sched: " Federico Serafini
@ 2024-04-02 7:22 ` Federico Serafini
2024-04-03 6:53 ` Jan Beulich
2024-04-02 7:22 ` [XEN PATCH 5/7] xen/sched: " Federico Serafini
` (2 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Federico Serafini @ 2024-04-02 7:22 UTC (permalink / raw
To: xen-devel
Cc: consulting, Federico Serafini, Andrew Cooper, George Dunlap,
Jan Beulich, Julien Grall, Stefano Stabellini
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [XEN PATCH 5/7] xen/sched: address a violation of MISRA C:2012 Rule 16.3
2024-04-02 7:22 [XEN PATCH 0/7] xen: address violations of MISRA C:2012 Rule 16.3 Federico Serafini
` (3 preceding siblings ...)
2024-04-02 7:22 ` [XEN PATCH 4/7] xen/evtchn: " Federico Serafini
@ 2024-04-02 7:22 ` Federico Serafini
2024-04-03 6:55 ` Jan Beulich
2024-04-02 7:22 ` [XEN PATCH 6/7] xen/vm-event: " Federico Serafini
2024-04-02 7:22 ` [XEN PATCH 7/7] vsprintf: address violations " Federico Serafini
6 siblings, 1 reply; 21+ messages in thread
From: Federico Serafini @ 2024-04-02 7:22 UTC (permalink / raw
To: xen-devel; +Cc: consulting, Federico Serafini, George Dunlap, Dario Faggioli
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [XEN PATCH 6/7] xen/vm-event: address a violation of MISRA C:2012 Rule 16.3
2024-04-02 7:22 [XEN PATCH 0/7] xen: address violations of MISRA C:2012 Rule 16.3 Federico Serafini
` (4 preceding siblings ...)
2024-04-02 7:22 ` [XEN PATCH 5/7] xen/sched: " Federico Serafini
@ 2024-04-02 7:22 ` Federico Serafini
2024-04-02 23:31 ` Tamas K Lengyel
2024-04-02 7:22 ` [XEN PATCH 7/7] vsprintf: address violations " Federico Serafini
6 siblings, 1 reply; 21+ messages in thread
From: Federico Serafini @ 2024-04-02 7:22 UTC (permalink / raw
To: xen-devel
Cc: consulting, Federico Serafini, Tamas K Lengyel, Alexandru Isaila,
Petre Pircalabu
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [XEN PATCH 7/7] vsprintf: address violations of MISRA C:2012 Rule 16.3
2024-04-02 7:22 [XEN PATCH 0/7] xen: address violations of MISRA C:2012 Rule 16.3 Federico Serafini
` (5 preceding siblings ...)
2024-04-02 7:22 ` [XEN PATCH 6/7] xen/vm-event: " Federico Serafini
@ 2024-04-02 7:22 ` Federico Serafini
2024-04-03 7:06 ` Jan Beulich
6 siblings, 1 reply; 21+ messages in thread
From: Federico Serafini @ 2024-04-02 7:22 UTC (permalink / raw
To: xen-devel
Cc: consulting, Federico Serafini, Andrew Cooper, George Dunlap,
Jan Beulich, Julien Grall, Stefano Stabellini
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [XEN PATCH 6/7] xen/vm-event: address a violation of MISRA C:2012 Rule 16.3
2024-04-02 7:22 ` [XEN PATCH 6/7] xen/vm-event: " Federico Serafini
@ 2024-04-02 23:31 ` Tamas K Lengyel
0 siblings, 0 replies; 21+ messages in thread
From: Tamas K Lengyel @ 2024-04-02 23:31 UTC (permalink / raw
To: Federico Serafini
Cc: xen-devel, consulting, Alexandru Isaila, Petre Pircalabu
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>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [XEN PATCH 1/7] xen/domctl: address a violation of MISRA C:2012 Rule 16.3
2024-04-02 7:22 ` [XEN PATCH 1/7] xen/domctl: address a violation " Federico Serafini
@ 2024-04-03 6:23 ` Jan Beulich
0 siblings, 0 replies; 21+ messages in thread
From: Jan Beulich @ 2024-04-03 6:23 UTC (permalink / raw
To: Federico Serafini
Cc: consulting, Andrew Cooper, George Dunlap, Julien Grall,
Stefano Stabellini, xen-devel
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>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [XEN PATCH 2/7] console: address a violation of MISRA C:2012 Rule 16.3
2024-04-02 7:22 ` [XEN PATCH 2/7] console: " Federico Serafini
@ 2024-04-03 6:24 ` Jan Beulich
0 siblings, 0 replies; 21+ messages in thread
From: Jan Beulich @ 2024-04-03 6:24 UTC (permalink / raw
To: Federico Serafini
Cc: consulting, Andrew Cooper, George Dunlap, Julien Grall,
Stefano Stabellini, xen-devel
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>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [XEN PATCH 3/7] xen/sched: address a violation of MISRA C:2012 Rule 16.3
2024-04-02 7:22 ` [XEN PATCH 3/7] xen/sched: " Federico Serafini
@ 2024-04-03 6:33 ` Jan Beulich
2024-04-03 7:52 ` Nicola Vetrini
0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2024-04-03 6:33 UTC (permalink / raw
To: Federico Serafini, Stefano Stabellini
Cc: consulting, George Dunlap, xen-devel, Dario Faggioli
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [XEN PATCH 4/7] xen/evtchn: address a violation of MISRA C:2012 Rule 16.3
2024-04-02 7:22 ` [XEN PATCH 4/7] xen/evtchn: " Federico Serafini
@ 2024-04-03 6:53 ` Jan Beulich
0 siblings, 0 replies; 21+ messages in thread
From: Jan Beulich @ 2024-04-03 6:53 UTC (permalink / raw
To: Federico Serafini
Cc: consulting, Andrew Cooper, George Dunlap, Julien Grall,
Stefano Stabellini, xen-devel
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>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [XEN PATCH 5/7] xen/sched: address a violation of MISRA C:2012 Rule 16.3
2024-04-02 7:22 ` [XEN PATCH 5/7] xen/sched: " Federico Serafini
@ 2024-04-03 6:55 ` Jan Beulich
2024-04-03 7:36 ` Federico Serafini
0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2024-04-03 6:55 UTC (permalink / raw
To: Federico Serafini; +Cc: consulting, George Dunlap, Dario Faggioli, xen-devel
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [XEN PATCH 7/7] vsprintf: address violations of MISRA C:2012 Rule 16.3
2024-04-02 7:22 ` [XEN PATCH 7/7] vsprintf: address violations " Federico Serafini
@ 2024-04-03 7:06 ` Jan Beulich
2024-04-03 7:38 ` Federico Serafini
0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2024-04-03 7:06 UTC (permalink / raw
To: Federico Serafini
Cc: consulting, Andrew Cooper, George Dunlap, Julien Grall,
Stefano Stabellini, xen-devel
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [XEN PATCH 5/7] xen/sched: address a violation of MISRA C:2012 Rule 16.3
2024-04-03 6:55 ` Jan Beulich
@ 2024-04-03 7:36 ` Federico Serafini
0 siblings, 0 replies; 21+ messages in thread
From: Federico Serafini @ 2024-04-03 7:36 UTC (permalink / raw
To: Jan Beulich; +Cc: consulting, George Dunlap, Dario Faggioli, xen-devel
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)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [XEN PATCH 7/7] vsprintf: address violations of MISRA C:2012 Rule 16.3
2024-04-03 7:06 ` Jan Beulich
@ 2024-04-03 7:38 ` Federico Serafini
0 siblings, 0 replies; 21+ messages in thread
From: Federico Serafini @ 2024-04-03 7:38 UTC (permalink / raw
To: Jan Beulich
Cc: consulting, Andrew Cooper, George Dunlap, Julien Grall,
Stefano Stabellini, xen-devel
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)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [XEN PATCH 3/7] xen/sched: address a violation of MISRA C:2012 Rule 16.3
2024-04-03 6:33 ` Jan Beulich
@ 2024-04-03 7:52 ` Nicola Vetrini
2024-04-05 0:18 ` Stefano Stabellini
0 siblings, 1 reply; 21+ messages in thread
From: Nicola Vetrini @ 2024-04-03 7:52 UTC (permalink / raw
To: Jan Beulich
Cc: Federico Serafini, Stefano Stabellini, consulting, George Dunlap,
xen-devel, Dario Faggioli
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)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [XEN PATCH 3/7] xen/sched: address a violation of MISRA C:2012 Rule 16.3
2024-04-03 7:52 ` Nicola Vetrini
@ 2024-04-05 0:18 ` Stefano Stabellini
2024-04-05 6:43 ` Jan Beulich
0 siblings, 1 reply; 21+ messages in thread
From: Stefano Stabellini @ 2024-04-05 0:18 UTC (permalink / raw
To: Nicola Vetrini
Cc: Jan Beulich, Federico Serafini, Stefano Stabellini, consulting,
George Dunlap, xen-devel, Dario Faggioli
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [XEN PATCH 3/7] xen/sched: address a violation of MISRA C:2012 Rule 16.3
2024-04-05 0:18 ` Stefano Stabellini
@ 2024-04-05 6:43 ` Jan Beulich
2024-04-05 19:26 ` Stefano Stabellini
0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2024-04-05 6:43 UTC (permalink / raw
To: Stefano Stabellini, Nicola Vetrini
Cc: Federico Serafini, Stefano Stabellini, consulting, George Dunlap,
xen-devel, Dario Faggioli
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [XEN PATCH 3/7] xen/sched: address a violation of MISRA C:2012 Rule 16.3
2024-04-05 6:43 ` Jan Beulich
@ 2024-04-05 19:26 ` Stefano Stabellini
0 siblings, 0 replies; 21+ messages in thread
From: Stefano Stabellini @ 2024-04-05 19:26 UTC (permalink / raw
To: Jan Beulich
Cc: Stefano Stabellini, Nicola Vetrini, Federico Serafini,
Stefano Stabellini, consulting, George Dunlap, xen-devel,
Dario Faggioli
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
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2024-04-05 19:26 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-02 7:22 [XEN PATCH 0/7] xen: address violations of MISRA C:2012 Rule 16.3 Federico Serafini
2024-04-02 7:22 ` [XEN PATCH 1/7] xen/domctl: address a violation " Federico Serafini
2024-04-03 6:23 ` Jan Beulich
2024-04-02 7:22 ` [XEN PATCH 2/7] console: " Federico Serafini
2024-04-03 6:24 ` Jan Beulich
2024-04-02 7:22 ` [XEN PATCH 3/7] xen/sched: " Federico Serafini
2024-04-03 6:33 ` Jan Beulich
2024-04-03 7:52 ` Nicola Vetrini
2024-04-05 0:18 ` Stefano Stabellini
2024-04-05 6:43 ` Jan Beulich
2024-04-05 19:26 ` Stefano Stabellini
2024-04-02 7:22 ` [XEN PATCH 4/7] xen/evtchn: " Federico Serafini
2024-04-03 6:53 ` Jan Beulich
2024-04-02 7:22 ` [XEN PATCH 5/7] xen/sched: " Federico Serafini
2024-04-03 6:55 ` Jan Beulich
2024-04-03 7:36 ` Federico Serafini
2024-04-02 7:22 ` [XEN PATCH 6/7] xen/vm-event: " Federico Serafini
2024-04-02 23:31 ` Tamas K Lengyel
2024-04-02 7:22 ` [XEN PATCH 7/7] vsprintf: address violations " Federico Serafini
2024-04-03 7:06 ` Jan Beulich
2024-04-03 7:38 ` Federico Serafini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.