Hi Peter, Alex, On 8/9/23 14:36, Peter Maydell wrote:
On Mon, 4 Sept 2023 at 17:15, Philippe Mathieu-Daudé <phi...@linaro.org> wrote: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é <phi...@linaro.org> --- semihosting/arm-compat-semi.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)If I'm reading the code correctly, the top level 'ret' variable is only used by the SYS_EXIT case currently. So rather than changing the type of it I think it would be better to remove it and have a variable at tighter scope for SYS_EXIT. I think that's easier to read than this kind of "single variable used for multiple purposes at different places within a long function".
Yes you are right. Now looking at it, we currently use a uint32_t type, but per https://github.com/ARM-software/abi-aa/blob/main/semihosting/semihosting.rst#652entry-64-bit: In particular, if field 1 is ADP_Stopped_ApplicationExit then field 2 is an exit status code, as passed to the C standard library exit() function. A simulator receiving this request must notify a connected debugger, if present, and then exit with the specified status. exit() is declared as: LIBRARY Standard C Library (libc, -lc) SYNOPSIS void exit(int status); So it expects a signed status code, but we convert it to unsigned... Alex, shouldn't we use a 'int' type here instead of 'uint32_t'?
diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c index 564fe17f75..85852a15b8 100644 --- a/semihosting/arm-compat-semi.c +++ b/semihosting/arm-compat-semi.c @@ -367,7 +367,7 @@ void do_common_semihosting(CPUState *cs) target_ulong ul_ret; char * s; int nr; - uint32_t ret; + int ret; int64_t elapsed;
