In vpl011_mmio_read switch block, all cases have a return. So the
outside one can be removed.

Signed-off-by: Jiamei Xie <jiamei.xie@arm.com>
---
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:
     *r = 0;
     return 1;
-- 
2.25.1

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@arm.com>
> ---
> 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