Re: [XEN PATCH 2/7] x86/mm: address MISRA C:2012 Rule 2.1

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 11 Dec 2023, Nicola Vetrini wrote:
> The "return 0" after the swich statement in 'xen/arch/x86/mm.c'
> is unreachable because all switch clauses end with returns.
> However, some of them can be substituted with "break"s to allow
> the "return 0" outside the switch to be reachable.
> 
> No functional changes.

This is correct but makes the code inconsistent. I would either remove
the return 0; at the end of arch_memory_op, or do the following:

- initialize rc to 0 at the beginning: int rc = 0;
- all switch clauses break instead of return;
- at the end: return rc;


> Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> ---
>  xen/arch/x86/mm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 0a66db10b959..8b31426a5348 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -4753,7 +4753,7 @@ long arch_memory_op(unsigned long cmd, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>          }
>  
>          spin_unlock(&d->arch.e820_lock);
> -        return 0;
> +        break;
>      }
>  
>      case XENMEM_machine_memory_map:
> @@ -4818,7 +4818,7 @@ long arch_memory_op(unsigned long cmd, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>          if ( __copy_to_guest(arg, &ctxt.map, 1) )
>              return -EFAULT;
>  
> -        return 0;
> +        break;
>      }
>  
>      case XENMEM_machphys_mapping:
> @@ -4834,7 +4834,7 @@ long arch_memory_op(unsigned long cmd, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>          if ( copy_to_guest(arg, &mapping, 1) )
>              return -EFAULT;
>  
> -        return 0;
> +        break;
>      }
>  
>  #ifdef CONFIG_HVM
> -- 
> 2.34.1
> 
>