| Message ID | 20231004120019.93101-14-philmd@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | (few more) Steps towards enabling -Wshadow | expand |
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>
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?
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/
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; + if (common_semi_sys_exit_extended(cs, nr)) { /* * The A64 version of SYS_EXIT takes a parameter block, @@ -752,6 +754,7 @@ void do_common_semihosting(CPUState *cs) } gdb_exit(ret); exit(ret); + } case TARGET_SYS_ELAPSED: elapsed = get_clock() - clock_start;
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(-)