1. Patchew
  2. QEMU
  3. [PATCH-for-5.0 00/11] misc: Trivial static code analyzer fixes
  4. View patch

[PATCH-for-5.0 05/11] hw/ide/sii3112: Remove dead assignment

Philippe Mathieu-Daudé posted 11 patches 5 years, 10 months ago
Maintainers: Andrew Jeffery <andrew@aj.id.au>, David Gibson <david@gibson.dropbear.id.au>, Alistair Francis <alistair@alistair23.me>, "Cédric Le Goater" <clg@kaod.org>, Andrzej Zaborowski <balrogg@gmail.com>, Fam Zheng <fam@euphon.net>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, John Snow <jsnow@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Joel Stanley <joel@jms.id.au>, Paolo Bonzini <pbonzini@redhat.com>, Igor Mitsyanko <i.mitsyanko@gmail.com>, Markus Armbruster <armbru@redhat.com>, BALATON Zoltan <balaton@eik.bme.hu>, Peter Maydell <peter.maydell@linaro.org>, "Hervé Poussineau" <hpoussin@reactos.org>, Max Reitz <mreitz@redhat.com>
There is a newer version of this series
[PATCH-for-5.0 05/11] hw/ide/sii3112: Remove dead assignment
Posted by Philippe Mathieu-Daudé 5 years, 10 months ago 
Fix warning reported by Clang static code analyzer:

    CC      hw/ide/sii3112.o
  hw/ide/sii3112.c:204:9: warning: Value stored to 'val' is never read
          val = 0;
          ^     ~

Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/ide/sii3112.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
index 06605d7af2..36f1905ddb 100644
--- a/hw/ide/sii3112.c
+++ b/hw/ide/sii3112.c
@@ -125,7 +125,7 @@ static uint64_t sii3112_reg_read(void *opaque, hwaddr addr,
         val = (uint32_t)d->regs[1].sien << 16;
         break;
     default:
-        val = 0;
+        break;
     }
     trace_sii3112_read(size, addr, val);
     return val;
-- 
2.21.1
Re: [PATCH-for-5.0 05/11] hw/ide/sii3112: Remove dead assignment
Posted by Aleksandar Markovic 5 years, 10 months ago 
On Saturday, March 21, 2020, Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:

> Fix warning reported by Clang static code analyzer:
>
>     CC      hw/ide/sii3112.o
>   hw/ide/sii3112.c:204:9: warning: Value stored to 'val' is never read
>           val = 0;
>           ^     ~
>
> Reported-by: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/ide/sii3112.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
> index 06605d7af2..36f1905ddb 100644
> --- a/hw/ide/sii3112.c
> +++ b/hw/ide/sii3112.c
> @@ -125,7 +125,7 @@ static uint64_t sii3112_reg_read(void *opaque, hwaddr
> addr,
>          val = (uint32_t)d->regs[1].sien << 16;
>          break;
>      default:
> -        val = 0;
> +        break;


There is another function in the same file, having a similar switch
statement. There is no warning for that second tunction, since "val" is its
parameter, not a local varioble, like is the case here. This means that the
proposed change introduces inconsistency between two functions, therefore
it is better to remove the initialization of "val" to 0, than to remove
this line in "default" section.

Regards,
Aleksandar



>      }
>      trace_sii3112_read(size, addr, val);
>      return val;
> --
> 2.21.1
>
>
>
Re: [PATCH-for-5.0 05/11] hw/ide/sii3112: Remove dead assignment
Posted by BALATON Zoltan 5 years, 10 months ago 
On Sat, 21 Mar 2020, Aleksandar Markovic wrote:
> On Saturday, March 21, 2020, Philippe Mathieu-Daudé <philmd@redhat.com>
> wrote:
>
>> Fix warning reported by Clang static code analyzer:
>>
>>     CC      hw/ide/sii3112.o
>>   hw/ide/sii3112.c:204:9: warning: Value stored to 'val' is never read
>>           val = 0;
>>           ^     ~
>>
>> Reported-by: Clang Static Analyzer
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  hw/ide/sii3112.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
>> index 06605d7af2..36f1905ddb 100644
>> --- a/hw/ide/sii3112.c
>> +++ b/hw/ide/sii3112.c
>> @@ -125,7 +125,7 @@ static uint64_t sii3112_reg_read(void *opaque, hwaddr
>> addr,
>>          val = (uint32_t)d->regs[1].sien << 16;
>>          break;
>>      default:
>> -        val = 0;
>> +        break;
>
>
> There is another function in the same file, having a similar switch
> statement. There is no warning for that second tunction, since "val" is its
> parameter, not a local varioble, like is the case here. This means that the
> proposed change introduces inconsistency between two functions, therefore
> it is better to remove the initialization of "val" to 0, than to remove
> this line in "default" section.

Oh, actually I think the warning was for that statement not for the one 
patched as it makes more sense there where val is assigned in void 
sii3112_reg_write() where it's indeed not used so maybe that was meant to 
be patched instead?

Regards,
BALATON Zoltan
Re: [PATCH-for-5.0 05/11] hw/ide/sii3112: Remove dead assignment
Posted by Philippe Mathieu-Daudé 5 years, 10 months ago 
On 3/21/20 3:12 PM, BALATON Zoltan wrote:
> On Sat, 21 Mar 2020, Aleksandar Markovic wrote:
>> On Saturday, March 21, 2020, Philippe Mathieu-Daudé <philmd@redhat.com>
>> wrote:
>>
>>> Fix warning reported by Clang static code analyzer:
>>>
>>>     CC      hw/ide/sii3112.o
>>>   hw/ide/sii3112.c:204:9: warning: Value stored to 'val' is never read
>>>           val = 0;
>>>           ^     ~
>>>
>>> Reported-by: Clang Static Analyzer
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>>  hw/ide/sii3112.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
>>> index 06605d7af2..36f1905ddb 100644
>>> --- a/hw/ide/sii3112.c
>>> +++ b/hw/ide/sii3112.c
>>> @@ -125,7 +125,7 @@ static uint64_t sii3112_reg_read(void *opaque, 
>>> hwaddr
>>> addr,
>>>          val = (uint32_t)d->regs[1].sien << 16;
>>>          break;
>>>      default:
>>> -        val = 0;
>>> +        break;
>>
>>
>> There is another function in the same file, having a similar switch
>> statement. There is no warning for that second tunction, since "val" 
>> is its
>> parameter, not a local varioble, like is the case here. This means 
>> that the
>> proposed change introduces inconsistency between two functions, therefore
>> it is better to remove the initialization of "val" to 0, than to remove
>> this line in "default" section.
> 
> Oh, actually I think the warning was for that statement not for the one 
> patched as it makes more sense there where val is assigned in void 
> sii3112_reg_write() where it's indeed not used so maybe that was meant 
> to be patched instead?

Ah, the warning is for hw/ide/sii3112.c:204 but I incorrectly patched 
hw/ide/sii3112.c:128 =)

> 
> Regards,
> BALATON Zoltan
Re: [PATCH-for-5.0 05/11] hw/ide/sii3112: Remove dead assignment
Posted by BALATON Zoltan 5 years, 10 months ago 
On Sat, 21 Mar 2020, Philippe Mathieu-Daudé wrote:
> On 3/21/20 3:12 PM, BALATON Zoltan wrote:
>> On Sat, 21 Mar 2020, Aleksandar Markovic wrote:
>>> On Saturday, March 21, 2020, Philippe Mathieu-Daudé <philmd@redhat.com>
>>> wrote:
>>> 
>>>> Fix warning reported by Clang static code analyzer:
>>>> 
>>>>     CC      hw/ide/sii3112.o
>>>>   hw/ide/sii3112.c:204:9: warning: Value stored to 'val' is never read
>>>>           val = 0;
>>>>           ^     ~
>>>> 
>>>> Reported-by: Clang Static Analyzer
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> ---
>>>>  hw/ide/sii3112.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>> 
>>>> diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
>>>> index 06605d7af2..36f1905ddb 100644
>>>> --- a/hw/ide/sii3112.c
>>>> +++ b/hw/ide/sii3112.c
>>>> @@ -125,7 +125,7 @@ static uint64_t sii3112_reg_read(void *opaque, hwaddr
>>>> addr,
>>>>          val = (uint32_t)d->regs[1].sien << 16;
>>>>          break;
>>>>      default:
>>>> -        val = 0;
>>>> +        break;
>>> 
>>> 
>>> There is another function in the same file, having a similar switch
>>> statement. There is no warning for that second tunction, since "val" is 
>>> its
>>> parameter, not a local varioble, like is the case here. This means that 
>>> the
>>> proposed change introduces inconsistency between two functions, therefore
>>> it is better to remove the initialization of "val" to 0, than to remove
>>> this line in "default" section.
>> 
>> Oh, actually I think the warning was for that statement not for the one 
>> patched as it makes more sense there where val is assigned in void 
>> sii3112_reg_write() where it's indeed not used so maybe that was meant to 
>> be patched instead?
>
> Ah, the warning is for hw/ide/sii3112.c:204 but I incorrectly patched 
> hw/ide/sii3112.c:128 =)

Now you may patch both to replace val = 0 with break to keep symmetry. My 
Reviewed-by stands (even if apparently not much use). Thanks for 
Aleksandar for spotting it.

Regards,
BALATON Zoltan
Re: [PATCH-for-5.0 05/11] hw/ide/sii3112: Remove dead assignment
Posted by Philippe Mathieu-Daudé 5 years, 10 months ago 
On 3/21/20 2:39 PM, Aleksandar Markovic wrote:
> 
> 
> On Saturday, March 21, 2020, Philippe Mathieu-Daudé <philmd@redhat.com 
> <mailto:philmd@redhat.com>> wrote:
> 
>     Fix warning reported by Clang static code analyzer:
> 
>          CC      hw/ide/sii3112.o
>        hw/ide/sii3112.c:204:9: warning: Value stored to 'val' is never read
>                val = 0;
>                ^     ~
> 
>     Reported-by: Clang Static Analyzer
>     Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com
>     <mailto:philmd@redhat.com>>
>     ---
>       hw/ide/sii3112.c | 2 +-
>       1 file changed, 1 insertion(+), 1 deletion(-)
> 
>     diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
>     index 06605d7af2..36f1905ddb 100644
>     --- a/hw/ide/sii3112.c
>     +++ b/hw/ide/sii3112.c
>     @@ -125,7 +125,7 @@ static uint64_t sii3112_reg_read(void *opaque,
>     hwaddr addr,
>               val = (uint32_t)d->regs[1].sien << 16;
>               break;
>           default:
>     -        val = 0;
>     +        break;
> 
> 
> There is another function in the same file, having a similar switch 
> statement. There is no warning for that second tunction, since "val" is 
> its parameter, not a local varioble, like is the case here. This means 
> that the proposed change introduces inconsistency between two functions, 
> therefore it is better to remove the initialization of "val" to 0, than 
> to remove this line in "default" section.

No clue why there is no warnings emitted for sii3112_reg_write()...

Do you mind sending a patch?

> 
> Regards,
> Aleksandar
> 
>           }
>           trace_sii3112_read(size, addr, val);
>           return val;
>     -- 
>     2.21.1
> 
>
Re: [PATCH-for-5.0 05/11] hw/ide/sii3112: Remove dead assignment
Posted by Philippe Mathieu-Daudé 5 years, 10 months ago 
On 3/21/20 3:14 PM, Philippe Mathieu-Daudé wrote:
> On 3/21/20 2:39 PM, Aleksandar Markovic wrote:
>>
>>
>> On Saturday, March 21, 2020, Philippe Mathieu-Daudé <philmd@redhat.com 
>> <mailto:philmd@redhat.com>> wrote:
>>
>>     Fix warning reported by Clang static code analyzer:
>>
>>          CC      hw/ide/sii3112.o
>>        hw/ide/sii3112.c:204:9: warning: Value stored to 'val' is never 
>> read
>>                val = 0;
>>                ^     ~
>>
>>     Reported-by: Clang Static Analyzer
>>     Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com
>>     <mailto:philmd@redhat.com>>
>>     ---
>>       hw/ide/sii3112.c | 2 +-
>>       1 file changed, 1 insertion(+), 1 deletion(-)
>>
>>     diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
>>     index 06605d7af2..36f1905ddb 100644
>>     --- a/hw/ide/sii3112.c
>>     +++ b/hw/ide/sii3112.c
>>     @@ -125,7 +125,7 @@ static uint64_t sii3112_reg_read(void *opaque,
>>     hwaddr addr,
>>               val = (uint32_t)d->regs[1].sien << 16;
>>               break;
>>           default:
>>     -        val = 0;
>>     +        break;
>>
>>
>> There is another function in the same file, having a similar switch 
>> statement. There is no warning for that second tunction, since "val" 
>> is its parameter, not a local varioble, like is the case here. This 
>> means that the proposed change introduces inconsistency between two 
>> functions, therefore it is better to remove the initialization of 
>> "val" to 0, than to remove this line in "default" section.
> 
> No clue why there is no warnings emitted for sii3112_reg_write()...
> 
> Do you mind sending a patch?

Don't worry I'll follow up.

> 
>>
>> Regards,
>> Aleksandar
>>
>>           }
>>           trace_sii3112_read(size, addr, val);
>>           return val;
>>     --     2.21.1
>>
>>
Re: [PATCH-for-5.0 05/11] hw/ide/sii3112: Remove dead assignment
Posted by BALATON Zoltan 5 years, 10 months ago 
On Sat, 21 Mar 2020, Philippe Mathieu-Daudé wrote:
> Fix warning reported by Clang static code analyzer:
>
>    CC      hw/ide/sii3112.o
>  hw/ide/sii3112.c:204:9: warning: Value stored to 'val' is never read
>          val = 0;
>          ^     ~
>
> Reported-by: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/ide/sii3112.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
> index 06605d7af2..36f1905ddb 100644
> --- a/hw/ide/sii3112.c
> +++ b/hw/ide/sii3112.c
> @@ -125,7 +125,7 @@ static uint64_t sii3112_reg_read(void *opaque, hwaddr addr,
>         val = (uint32_t)d->regs[1].sien << 16;
>         break;
>     default:
> -        val = 0;
> +        break;
>     }
>     trace_sii3112_read(size, addr, val);
>     return val;

Value is clearly used in trace and return so don't really get why the 
compiler complains here. Looks like wrong warning to me. It's true however 
that since val is init to 0 at the beginning this assignment is not 
strictily needed and this should work as well, so

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

Regards,
BALATON Zoltan

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.