Re: [PATCH v3 13/16] semihosting/arm-compat: Clean up local variable shadowing

[Philippe Mathieu-Daudé]

On 6/10/23 11:14, Markus Armbruster wrote:
> Alex Bennée <alex.bennee@linaro.org> writes:
>
> > Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> >
> > > Fix:
> > >
> > >    semihosting/arm-compat-semi.c: In function ‘do_common_semihosting’:
> > >    semihosting/arm-compat-semi.c:379:13: warning: declaration of ‘ret’ shadows 
> > > a previous local [-Wshadow=local]
> > >      379 |         int ret, err = 0;
> > >          |             ^~~
> > >    semihosting/arm-compat-semi.c:370:14: note: shadowed declaration is here
> > >      370 |     uint32_t ret;
> > >          |              ^~~
> > >    semihosting/arm-compat-semi.c:682:27: warning: declaration of ‘ret’
> > > shadows a previous local [-Wshadow=local]
> > >      682 |                 abi_ulong ret;
> > >          |                           ^~~
> > >    semihosting/arm-compat-semi.c:370:9: note: shadowed declaration is here
> > >      370 |     int ret;
> > >          |         ^~~
> > >
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > ---
> > >   semihosting/arm-compat-semi.c | 5 ++++-
> > >   1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
> > > index 564fe17f75..0033a1e018 100644
> > > --- a/semihosting/arm-compat-semi.c
> > > +++ b/semihosting/arm-compat-semi.c
> > > @@ -367,7 +367,6 @@ void do_common_semihosting(CPUState *cs)
> > >       target_ulong ul_ret;
> > >       char * s;
> > >       int nr;
> > > -    uint32_t ret;
> > >       int64_t elapsed;
> > >   
> > >       nr = common_semi_arg(cs, 0) & 0xffffffffU;
> > > @@ -725,6 +724,9 @@ void do_common_semihosting(CPUState *cs)
> > >   
> > >       case TARGET_SYS_EXIT:
> > >       case TARGET_SYS_EXIT_EXTENDED:
> > > +    {
> > > +        uint32_t ret;
> > > +
> >
> > I suspect this could just as well be an int with an explicit cast for ret = arg1
> > because the consumers are all expecting int anyway.
> >
> > Otherwise:
> >
> > Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>
> Philippe, would you like to follow up on Alex's suspicion, or would you
> like me to queue the patch as is?

Please queue as is. The signed conversion is done here (well
documented):
https://lore.kernel.org/qemu-devel/20231005062610.57351-1-philmd@linaro.org/