[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 18/22] semihosting/arm-compat: Clean up local variable sha
From: |
Peter Maydell |
Subject: |
Re: [PATCH v2 18/22] semihosting/arm-compat: Clean up local variable shadowing |
Date: |
Fri, 8 Sep 2023 13:36:15 +0100 |
On Mon, 4 Sept 2023 at 17:15, Philippe Mathieu-Daudé <philmd@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é <philmd@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
- [PATCH v2 14/22] hw/nios2: Clean up local variable shadowing, (continued)
- [PATCH v2 14/22] hw/nios2: Clean up local variable shadowing, Philippe Mathieu-Daudé, 2023/09/04
- [PATCH v2 15/22] net/eth: Clean up local variable shadowing, Philippe Mathieu-Daudé, 2023/09/04
- [PATCH v2 16/22] crypto/cipher-gnutls.c: Clean up local variable shadowing, Philippe Mathieu-Daudé, 2023/09/04
- [PATCH v2 17/22] util/vhost-user-server: Clean up local variable shadowing, Philippe Mathieu-Daudé, 2023/09/04
- [PATCH v2 18/22] semihosting/arm-compat: Clean up local variable shadowing, Philippe Mathieu-Daudé, 2023/09/04
- Re: [PATCH v2 18/22] semihosting/arm-compat: Clean up local variable shadowing,
Peter Maydell <=
- [PATCH v2 19/22] linux-user/strace: Clean up local variable shadowing, Philippe Mathieu-Daudé, 2023/09/04
- [PATCH v2 20/22] sysemu/device_tree: Clean up local variable shadowing, Philippe Mathieu-Daudé, 2023/09/04
- [RFC PATCH v2 22/22] softmmu/physmem: Clean up local variable shadowing, Philippe Mathieu-Daudé, 2023/09/04
- [PATCH v2 21/22] softmmu/memory: Clean up local variable shadowing, Philippe Mathieu-Daudé, 2023/09/04
- Re: [PATCH v2 00/22] (few more) Steps towards enabling -Wshadow, Markus Armbruster, 2023/09/29