Re: [PATCH v4 2/2] xen/arm: vpl011: drop extra return in vpl011_mmio_read

[Julien Grall <julien@xxxxxxx>]

Hi,

On 01/12/2022 09:04, Jiamei Xie wrote:
> In vpl011_mmio_read switch block, all cases have a return. So the
> outside one can be removed.
That's correct today. However, if tomorrow we add a new case and forgot 
to add the return, then ...
> Signed-off-by: Jiamei Xie <jiamei.xie@xxxxxxx>
> ---
> v3 -> v4
> - Don't consolidate check registers access
> - Don't remove label read_as_zero
> ---
>   xen/arch/arm/vpl011.c | 2 --
>   1 file changed, 2 deletions(-)
>
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> index f4a5621fab..35de50760c 100644
> --- a/xen/arch/arm/vpl011.c
> +++ b/xen/arch/arm/vpl011.c
> @@ -417,8 +417,6 @@ static int vpl011_mmio_read(struct vcpu *v,
>           goto read_as_zero;
>       }
>   
> -    return 1;
> - >   read_as_zero:
... we would end up to clobber the register. This is far from idea. So 
was this change made because the compiler complained?
If not, then I would prefer if we keep "return 1" and maybe add 
ASSERT_UNREACHABLE() to catch case where the return is not added.
Cheers,

--
Julien Grall