Re: [PATCH] x86/mm: use "break" in arch_memory_op()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.12.2023 01:22, Stefano Stabellini wrote:
> On Tue, 19 Dec 2023, Jan Beulich wrote:
>> The final return statement is unreachable and hence disliked by Misra
>> C:2012 (rule 2.1). Convert those case-specific (main) return statements
>> which already use "rc", or in one case when it can be used without
>> further adding of code, to break.
>>
>> No functional change intended.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> This is an alternative proposal to
>> https://lists.xen.org/archives/html/xen-devel/2023-12/msg01537.html.
>> Yet another option would be to simply pull the default case out of the
>> switch() statement.
>>
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -4724,7 +4724,7 @@ long arch_memory_op(unsigned long cmd, X
>>          spin_unlock(&d->arch.e820_lock);
>>  
>>          rcu_unlock_domain(d);
>> -        return rc;
>> +        break;
>>      }
>>  
>>      case XENMEM_memory_map:
>> @@ -4818,7 +4818,7 @@ long arch_memory_op(unsigned long cmd, X
>>          if ( __copy_to_guest(arg, &ctxt.map, 1) )
>>              return -EFAULT;
>>  
>> -        return 0;
>> +        break;
>>      }
> 
> There are also two other return 0; under case XENMEM_memory_map and
> XENMEM_machphys_mapping. I would be consistent and either leave this
> return 0 alone, or change all the return 0.

Yes, that would have been another possible pattern to follow. Due to
the multiple possible approaches I had specifically outlined (in the
description) the pattern I decided to follow.

> Either way, this patch is correct, so:
> 
> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>

Thanks.

Jan