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(-)

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;
 
-- 
2.34.1

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 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.

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

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 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>