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

[Markus Armbruster]

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?