[v3] redundant code: Fix warnings reported by Clang static code analyzer

[PATCH v3 04/12] scsi/esp-pci: Remove redundant statement in esp_pci_io_write()

Posted by Chen Qun 5 years, 6 months ago

Clang static code analyzer show warning:
  hw/scsi/esp-pci.c:198:9: warning: Value stored to 'size' is never read
        size = 4;
        ^      ~

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc:Fam Zheng <fam@euphon.net>
---
 hw/scsi/esp-pci.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
index d5a1f9e017..2e6cc07d4e 100644
--- a/hw/scsi/esp-pci.c
+++ b/hw/scsi/esp-pci.c
@@ -195,7 +195,6 @@ static void esp_pci_io_write(void *opaque, hwaddr addr,
         val <<= shift;
         val |= current & ~(mask << shift);
         addr &= ~3;
-        size = 4;
     }
 
     if (addr < 0x40) {
-- 
2.23.0

Re: [PATCH v3 04/12] scsi/esp-pci: Remove redundant statement in esp_pci_io_write()

Posted by Laurent Vivier 5 years, 6 months ago

Le 02/03/2020 à 14:07, Chen Qun a écrit :
> Clang static code analyzer show warning:
>   hw/scsi/esp-pci.c:198:9: warning: Value stored to 'size' is never read
>         size = 4;
>         ^      ~
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
> ---
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc:Fam Zheng <fam@euphon.net>
> ---
>  hw/scsi/esp-pci.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
> index d5a1f9e017..2e6cc07d4e 100644
> --- a/hw/scsi/esp-pci.c
> +++ b/hw/scsi/esp-pci.c
> @@ -195,7 +195,6 @@ static void esp_pci_io_write(void *opaque, hwaddr addr,
>          val <<= shift;
>          val |= current & ~(mask << shift);
>          addr &= ~3;
> -        size = 4;
>      }

perhaps a "g_assert(size >= 4)" instead would be cleaner to mute the
warning?

I think it's a good point to update the size if in the future the code
below is modified to use size.

Thanks,
Laurent

RE: [PATCH v3 04/12] scsi/esp-pci: Remove redundant statement in esp_pci_io_write()

Posted by Chenqun (kuhn) 5 years, 6 months ago

>-----Original Message-----
>From: Laurent Vivier [mailto:laurent@vivier.eu]
>Sent: Monday, March 9, 2020 8:22 PM
>To: Chenqun (kuhn) <kuhn.chenqun@huawei.com>; qemu-
>devel@nongnu.org; qemu-trivial@nongnu.org
>Cc: peter.maydell@linaro.org; Euler Robot <euler.robot@huawei.com>;
>Zhanghailiang <zhang.zhanghailiang@huawei.com>; Paolo Bonzini
><pbonzini@redhat.com>
>Subject: Re: [PATCH v3 04/12] scsi/esp-pci: Remove redundant statement in
>esp_pci_io_write()
>
>Le 02/03/2020 à 14:07, Chen Qun a écrit :
>> Clang static code analyzer show warning:
>>   hw/scsi/esp-pci.c:198:9: warning: Value stored to 'size' is never read
>>         size = 4;
>>         ^      ~
>>
>> Reported-by: Euler Robot <euler.robot@huawei.com>
>> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
>> ---
>> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc:Fam Zheng
><fam@euphon.net>
>> ---
>>  hw/scsi/esp-pci.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c index
>> d5a1f9e017..2e6cc07d4e 100644
>> --- a/hw/scsi/esp-pci.c
>> +++ b/hw/scsi/esp-pci.c
>> @@ -195,7 +195,6 @@ static void esp_pci_io_write(void *opaque, hwaddr
>addr,
>>          val <<= shift;
>>          val |= current & ~(mask << shift);
>>          addr &= ~3;
>> -        size = 4;
>>      }
>
>perhaps a "g_assert(size >= 4)" instead would be cleaner to mute the warning?
>
Yes, add 'g_assert(size >= 4)' can mute the warning.

>
>I think it's a good point to update the size if in the future the code below is
>modified to use size.
>
Hmm, maybe it is true.

So, let's  keep ' size = 4'  and  add 'g_assert(size >= 4)' after if() statement , shall we?

Thanks,
Chen Qun
>
>Thanks,
>Laurent
>

Re: [PATCH v3 04/12] scsi/esp-pci: Remove redundant statement in esp_pci_io_write()

Posted by Laurent Vivier 5 years, 6 months ago

Le 10/03/2020 à 12:52, Chenqun (kuhn) a écrit :
>> -----Original Message-----
>> From: Laurent Vivier [mailto:laurent@vivier.eu]
>> Sent: Monday, March 9, 2020 8:22 PM
>> To: Chenqun (kuhn) <kuhn.chenqun@huawei.com>; qemu-
>> devel@nongnu.org; qemu-trivial@nongnu.org
>> Cc: peter.maydell@linaro.org; Euler Robot <euler.robot@huawei.com>;
>> Zhanghailiang <zhang.zhanghailiang@huawei.com>; Paolo Bonzini
>> <pbonzini@redhat.com>
>> Subject: Re: [PATCH v3 04/12] scsi/esp-pci: Remove redundant statement in
>> esp_pci_io_write()
>>
>> Le 02/03/2020 à 14:07, Chen Qun a écrit :
>>> Clang static code analyzer show warning:
>>>   hw/scsi/esp-pci.c:198:9: warning: Value stored to 'size' is never read
>>>         size = 4;
>>>         ^      ~
>>>
>>> Reported-by: Euler Robot <euler.robot@huawei.com>
>>> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
>>> ---
>>> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc:Fam Zheng
>> <fam@euphon.net>
>>> ---
>>>  hw/scsi/esp-pci.c | 1 -
>>>  1 file changed, 1 deletion(-)
>>>
>>> diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c index
>>> d5a1f9e017..2e6cc07d4e 100644
>>> --- a/hw/scsi/esp-pci.c
>>> +++ b/hw/scsi/esp-pci.c
>>> @@ -195,7 +195,6 @@ static void esp_pci_io_write(void *opaque, hwaddr
>> addr,
>>>          val <<= shift;
>>>          val |= current & ~(mask << shift);
>>>          addr &= ~3;
>>> -        size = 4;
>>>      }
>>
>> perhaps a "g_assert(size >= 4)" instead would be cleaner to mute the warning?
>>
> Yes, add 'g_assert(size >= 4)' can mute the warning.
> 
>>
>> I think it's a good point to update the size if in the future the code below is
>> modified to use size.
>>
> Hmm, maybe it is true.
> 
> So, let's  keep ' size = 4'  and  add 'g_assert(size >= 4)' after if() statement , shall we?

Yes, it's what I would prefer. But it's a question of taste...

Paolo? Fam?

Thanks,
Laurent