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".
> 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;
>
> nr = common_semi_arg(cs, 0) & 0xffffffffU;
> @@ -376,7 +376,7 @@ void do_common_semihosting(CPUState *cs)
> switch (nr) {
> case TARGET_SYS_OPEN:
> {
> - int ret, err = 0;
> + int err = 0;
> int hostfd;
>
> GET_ARG(0);
> @@ -679,14 +679,11 @@ void do_common_semihosting(CPUState *cs)
> * allocate it using sbrk.
> */
> if (!ts->heap_limit) {
> - abi_ulong ret;
> -
> ts->heap_base = do_brk(0);
> limit = ts->heap_base + COMMON_SEMI_HEAP_SIZE;
> /* Try a big heap, and reduce the size if that fails. */
> for (;;) {
> - ret = do_brk(limit);
> - if (ret >= limit) {
> + if (do_brk(limit) >= limit) {
> break;
> }
> limit = (ts->heap_base >> 1) + (limit >> 1);
> -
thanks
-- PMM
