| Message ID | 20221201080400.1842558-3-jiamei.xie@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm: refine vpl011 | expand |
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,
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;
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(-)