| Message ID | b1103bc13d5ce04159417592705b4fe6a6db748b.1702283415.git.nicola.vetrini@bugseng.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | address violations of MISRA C:2012 Rule 2.1 | expand |
On Mon, 11 Dec 2023, Nicola Vetrini wrote: > The break statement is redundant, hence it can be removed. > > Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
On 11.12.2023 11:30, Nicola Vetrini wrote: > The break statement is redundant, hence it can be removed. Except ... > --- a/xen/arch/x86/platform_hypercall.c > +++ b/xen/arch/x86/platform_hypercall.c > @@ -723,7 +723,6 @@ ret_t do_platform_op( > > ret = continue_hypercall_on_cpu( > 0, cpu_down_helper, (void *)(unsigned long)cpu); > - break; > } > break; ... it wants to be the other break that is removed, imo. Andrew, Roger, what do you think? There are many such (again: imo) oddly placed break-s in that switch() ... In some cases there are also inner scopes without there being new local variables in there. IOW imo throughout this switch() - pointless inner scopes want dropping, - all "main" break-s want to have the same indentation. Jan
On Tue, 12 Dec 2023, Jan Beulich wrote: > On 11.12.2023 11:30, Nicola Vetrini wrote: > > The break statement is redundant, hence it can be removed. > > Except ... > > > --- a/xen/arch/x86/platform_hypercall.c > > +++ b/xen/arch/x86/platform_hypercall.c > > @@ -723,7 +723,6 @@ ret_t do_platform_op( > > > > ret = continue_hypercall_on_cpu( > > 0, cpu_down_helper, (void *)(unsigned long)cpu); > > - break; > > } > > break; > > ... it wants to be the other break that is removed, imo. I was also about to comment about which of the two breaks to remove... I didn't know if there are x86 specific conventions so I didn't say anything about it. But I also think it is more natural to keep the other break.
On 2023-12-12 11:13, Jan Beulich wrote: > On 11.12.2023 11:30, Nicola Vetrini wrote: >> The break statement is redundant, hence it can be removed. > > Except ... > >> --- a/xen/arch/x86/platform_hypercall.c >> +++ b/xen/arch/x86/platform_hypercall.c >> @@ -723,7 +723,6 @@ ret_t do_platform_op( >> >> ret = continue_hypercall_on_cpu( >> 0, cpu_down_helper, (void *)(unsigned long)cpu); >> - break; >> } >> break; > > ... it wants to be the other break that is removed, imo. Andrew, Roger, > what do you think? There are many such (again: imo) oddly placed > break-s > in that switch() ... In some cases there are also inner scopes without > there being new local variables in there. IOW imo throughout this > switch() > - pointless inner scopes want dropping, > - all "main" break-s want to have the same indentation. > > Jan Ok. I'm not particularly keen on doing a full style cleanup; I can drop the other break, for the sake of resolving the MISRA violation, so that the cleanup can happen anytime.
diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c index 4dde71db275c..7556c6e6cd0c 100644 --- a/xen/arch/x86/platform_hypercall.c +++ b/xen/arch/x86/platform_hypercall.c @@ -723,7 +723,6 @@ ret_t do_platform_op( ret = continue_hypercall_on_cpu( 0, cpu_down_helper, (void *)(unsigned long)cpu); - break; } break;
The break statement is redundant, hence it can be removed. Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> --- xen/arch/x86/platform_hypercall.c | 1 - 1 file changed, 1 deletion(-)