All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/3] redundant code: Fix warnings reported by Clang static code analyzer
@ 2020-03-24  8:22 Chen Qun
  2020-03-24  8:22 ` [PATCH v4 1/3] scsi/esp-pci: Remove redundant statement in esp_pci_io_write() Chen Qun
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Chen Qun @ 2020-03-24  8:22 UTC (permalink / raw)
  To: qemu-devel, qemu-trivial; +Cc: Chen Qun, philmd, zhang.zhanghailiang, laurent

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="y", Size: 1222 bytes --]

v1->v2:
- Patch1: Add John Snow review comment.
- Patch9: Move the 'dst_type' declaration to while() statement.
- Patch12: Add Philippe Mathieu-Daudé review comment.
- Patch13: Move the 'set' declaration to the for() statement.

v2->v3:
- Patch1: Add Kevin Wolf review comment.
- Patch2: Keep the 'flags' then use it(Base on Kevin's comments).
- Patch3: Add Kevin Wolf review comment.
- Patch9: Add Francisco Iglesias and Alistair Francis review comment.
- Patch10: Juan Quintela has added it to the queue and delete it.
- Patch12->Patch11: Add Philippe Mathieu-Daudé review comment.
- Patch13->Patch12: Add Philippe Mathieu-Daudé review comment.

v3->v4:
- Deleted the patches that have been merged in the v3.
- Modify "scsi/esp-pci" Patch, use g_assert with variable size.

Chen Qun (3):
  scsi/esp-pci: Remove redundant statement in esp_pci_io_write()
  display/blizzard: Remove redundant statement in
    blizzard_draw_line16_32()
  timer/exynos4210_mct: Remove redundant statement in
    exynos4210_mct_write()

 hw/display/blizzard.c     | 1 -
 hw/scsi/esp-pci.c         | 1 +
 hw/timer/exynos4210_mct.c | 4 ----
 3 files changed, 1 insertion(+), 5 deletions(-)

-- 
2.23.0




^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 1/3] scsi/esp-pci: Remove redundant statement in esp_pci_io_write()
  2020-03-24  8:22 [PATCH v4 0/3] redundant code: Fix warnings reported by Clang static code analyzer Chen Qun
@ 2020-03-24  8:22 ` Chen Qun
  2020-03-24  8:27   ` Laurent Vivier
  2020-03-24  8:22 ` [PATCH v4 2/3] display/blizzard: Remove redundant statement in blizzard_draw_line16_32() Chen Qun
  2020-03-24  8:22 ` [PATCH v4 3/3] timer/exynos4210_mct: Remove redundant statement in exynos4210_mct_write() Chen Qun
  2 siblings, 1 reply; 11+ messages in thread
From: Chen Qun @ 2020-03-24  8:22 UTC (permalink / raw)
  To: qemu-devel, qemu-trivial
  Cc: zhang.zhanghailiang, laurent, Paolo Bonzini, Euler Robot,
	Chen Qun, philmd

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>
v1->v2:
keep ' size = 4'  and  add 'g_assert(size >= 4)' after if() statement.
(Base on Laurent's comments)
---
 hw/scsi/esp-pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
index d5a1f9e017..497a8d5901 100644
--- a/hw/scsi/esp-pci.c
+++ b/hw/scsi/esp-pci.c
@@ -197,6 +197,7 @@ static void esp_pci_io_write(void *opaque, hwaddr addr,
         addr &= ~3;
         size = 4;
     }
+    g_assert(size >= 4);
 
     if (addr < 0x40) {
         /* SCSI core reg */
-- 
2.23.0




^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 2/3] display/blizzard: Remove redundant statement in blizzard_draw_line16_32()
  2020-03-24  8:22 [PATCH v4 0/3] redundant code: Fix warnings reported by Clang static code analyzer Chen Qun
  2020-03-24  8:22 ` [PATCH v4 1/3] scsi/esp-pci: Remove redundant statement in esp_pci_io_write() Chen Qun
@ 2020-03-24  8:22 ` Chen Qun
  2020-03-24  8:38   ` Laurent Vivier
  2020-03-24  8:22 ` [PATCH v4 3/3] timer/exynos4210_mct: Remove redundant statement in exynos4210_mct_write() Chen Qun
  2 siblings, 1 reply; 11+ messages in thread
From: Chen Qun @ 2020-03-24  8:22 UTC (permalink / raw)
  To: qemu-devel, qemu-trivial
  Cc: Peter Maydell, zhang.zhanghailiang, philmd, laurent, Euler Robot,
	Chen Qun

Clang static code analyzer show warning:
  hw/display/blizzard.c:940:9: warning: Value stored to 'data' is never read
        data >>= 5;
        ^        ~
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
---
Cc: Andrzej Zaborowski <balrogg@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
---
 hw/display/blizzard.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/display/blizzard.c b/hw/display/blizzard.c
index 359e399c2a..62517bdf75 100644
--- a/hw/display/blizzard.c
+++ b/hw/display/blizzard.c
@@ -937,7 +937,6 @@ static void blizzard_draw_line16_32(uint32_t *dest,
         g = (data & 0x3f) << 2;
         data >>= 6;
         r = (data & 0x1f) << 3;
-        data >>= 5;
         *dest++ = rgb_to_pixel32(r, g, b);
     }
 }
-- 
2.23.0




^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 3/3] timer/exynos4210_mct: Remove redundant statement in exynos4210_mct_write()
  2020-03-24  8:22 [PATCH v4 0/3] redundant code: Fix warnings reported by Clang static code analyzer Chen Qun
  2020-03-24  8:22 ` [PATCH v4 1/3] scsi/esp-pci: Remove redundant statement in esp_pci_io_write() Chen Qun
  2020-03-24  8:22 ` [PATCH v4 2/3] display/blizzard: Remove redundant statement in blizzard_draw_line16_32() Chen Qun
@ 2020-03-24  8:22 ` Chen Qun
  2020-03-24 10:59   ` Laurent Vivier
  2 siblings, 1 reply; 11+ messages in thread
From: Chen Qun @ 2020-03-24  8:22 UTC (permalink / raw)
  To: qemu-devel, qemu-trivial
  Cc: Peter Maydell, zhang.zhanghailiang, Igor Mitsyanko, laurent,
	Euler Robot, Chen Qun, philmd

Clang static code analyzer show warning:
hw/timer/exynos4210_mct.c:1370:9: warning: Value stored to 'index' is never read
        index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
        ^       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
hw/timer/exynos4210_mct.c:1399:9: warning: Value stored to 'index' is never read
        index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
        ^       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
hw/timer/exynos4210_mct.c:1441:9: warning: Value stored to 'index' is never read
        index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
        ^       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
---
Cc: Igor Mitsyanko <i.mitsyanko@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
---
 hw/timer/exynos4210_mct.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/hw/timer/exynos4210_mct.c b/hw/timer/exynos4210_mct.c
index 944120aea5..570cf7075b 100644
--- a/hw/timer/exynos4210_mct.c
+++ b/hw/timer/exynos4210_mct.c
@@ -1367,7 +1367,6 @@ static void exynos4210_mct_write(void *opaque, hwaddr offset,
 
     case L0_TCNTB: case L1_TCNTB:
         lt_i = GET_L_TIMER_IDX(offset);
-        index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
 
         /*
          * TCNTB is updated to internal register only after CNT expired.
@@ -1396,7 +1395,6 @@ static void exynos4210_mct_write(void *opaque, hwaddr offset,
 
     case L0_ICNTB: case L1_ICNTB:
         lt_i = GET_L_TIMER_IDX(offset);
-        index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
 
         s->l_timer[lt_i].reg.wstat |= L_WSTAT_ICNTB_WRITE;
         s->l_timer[lt_i].reg.cnt[L_REG_CNT_ICNTB] = value &
@@ -1438,8 +1436,6 @@ static void exynos4210_mct_write(void *opaque, hwaddr offset,
 
     case L0_FRCNTB: case L1_FRCNTB:
         lt_i = GET_L_TIMER_IDX(offset);
-        index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
-
         DPRINTF("local timer[%d] FRCNTB write %llx\n", lt_i, value);
 
         s->l_timer[lt_i].reg.wstat |= L_WSTAT_FRCCNTB_WRITE;
-- 
2.23.0




^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/3] scsi/esp-pci: Remove redundant statement in esp_pci_io_write()
  2020-03-24  8:22 ` [PATCH v4 1/3] scsi/esp-pci: Remove redundant statement in esp_pci_io_write() Chen Qun
@ 2020-03-24  8:27   ` Laurent Vivier
  0 siblings, 0 replies; 11+ messages in thread
From: Laurent Vivier @ 2020-03-24  8:27 UTC (permalink / raw)
  To: Chen Qun, qemu-devel, qemu-trivial
  Cc: Paolo Bonzini, philmd, zhang.zhanghailiang, Euler Robot

Le 24/03/2020 à 09:22, 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>
> v1->v2:
> keep ' size = 4'  and  add 'g_assert(size >= 4)' after if() statement.
> (Base on Laurent's comments)
> ---
>  hw/scsi/esp-pci.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
> index d5a1f9e017..497a8d5901 100644
> --- a/hw/scsi/esp-pci.c
> +++ b/hw/scsi/esp-pci.c
> @@ -197,6 +197,7 @@ static void esp_pci_io_write(void *opaque, hwaddr addr,
>          addr &= ~3;
>          size = 4;
>      }
> +    g_assert(size >= 4);
>  
>      if (addr < 0x40) {
>          /* SCSI core reg */
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/3] display/blizzard: Remove redundant statement in blizzard_draw_line16_32()
  2020-03-24  8:22 ` [PATCH v4 2/3] display/blizzard: Remove redundant statement in blizzard_draw_line16_32() Chen Qun
@ 2020-03-24  8:38   ` Laurent Vivier
  2020-03-24  8:40     ` Laurent Vivier
  0 siblings, 1 reply; 11+ messages in thread
From: Laurent Vivier @ 2020-03-24  8:38 UTC (permalink / raw)
  To: Chen Qun, qemu-devel, qemu-trivial
  Cc: Peter Maydell, philmd, zhang.zhanghailiang, Euler Robot

Le 24/03/2020 à 09:22, Chen Qun a écrit :
> Clang static code analyzer show warning:
>   hw/display/blizzard.c:940:9: warning: Value stored to 'data' is never read
>         data >>= 5;
>         ^        ~
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
> ---
> Cc: Andrzej Zaborowski <balrogg@gmail.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/display/blizzard.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/hw/display/blizzard.c b/hw/display/blizzard.c
> index 359e399c2a..62517bdf75 100644
> --- a/hw/display/blizzard.c
> +++ b/hw/display/blizzard.c
> @@ -937,7 +937,6 @@ static void blizzard_draw_line16_32(uint32_t *dest,
>          g = (data & 0x3f) << 2;
>          data >>= 6;
>          r = (data & 0x1f) << 3;
> -        data >>= 5;
>          *dest++ = rgb_to_pixel32(r, g, b);
>      }
>  }
> 

Perhaps it would be clearer to use extract32() to compute r, g and b?

Thanks,
Laurent


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/3] display/blizzard: Remove redundant statement in blizzard_draw_line16_32()
  2020-03-24  8:38   ` Laurent Vivier
@ 2020-03-24  8:40     ` Laurent Vivier
  2020-03-24 11:49       ` Chenqun (kuhn)
  0 siblings, 1 reply; 11+ messages in thread
From: Laurent Vivier @ 2020-03-24  8:40 UTC (permalink / raw)
  To: Chen Qun, qemu-devel, qemu-trivial
  Cc: Peter Maydell, philmd, zhang.zhanghailiang, Euler Robot

Le 24/03/2020 à 09:38, Laurent Vivier a écrit :
> Le 24/03/2020 à 09:22, Chen Qun a écrit :
>> Clang static code analyzer show warning:
>>   hw/display/blizzard.c:940:9: warning: Value stored to 'data' is never read
>>         data >>= 5;
>>         ^        ~
>> Reported-by: Euler Robot <euler.robot@huawei.com>
>> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
>> ---
>> Cc: Andrzej Zaborowski <balrogg@gmail.com>
>> Cc: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  hw/display/blizzard.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/hw/display/blizzard.c b/hw/display/blizzard.c
>> index 359e399c2a..62517bdf75 100644
>> --- a/hw/display/blizzard.c
>> +++ b/hw/display/blizzard.c
>> @@ -937,7 +937,6 @@ static void blizzard_draw_line16_32(uint32_t *dest,
>>          g = (data & 0x3f) << 2;
>>          data >>= 6;
>>          r = (data & 0x1f) << 3;
>> -        data >>= 5;
>>          *dest++ = rgb_to_pixel32(r, g, b);
>>      }
>>  }
>>
> 
> Perhaps it would be clearer to use extract32() to compute r, g and b?

in fact extract16() as data is uint16_t...

Thanks,
Laurent


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 3/3] timer/exynos4210_mct: Remove redundant statement in exynos4210_mct_write()
  2020-03-24  8:22 ` [PATCH v4 3/3] timer/exynos4210_mct: Remove redundant statement in exynos4210_mct_write() Chen Qun
@ 2020-03-24 10:59   ` Laurent Vivier
  2020-03-24 12:22     ` Chenqun (kuhn)
  0 siblings, 1 reply; 11+ messages in thread
From: Laurent Vivier @ 2020-03-24 10:59 UTC (permalink / raw)
  To: Chen Qun, qemu-devel, qemu-trivial
  Cc: Igor Mitsyanko, Peter Maydell, philmd, zhang.zhanghailiang, Euler Robot

Le 24/03/2020 à 09:22, Chen Qun a écrit :
> Clang static code analyzer show warning:
> hw/timer/exynos4210_mct.c:1370:9: warning: Value stored to 'index' is never read
>         index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
>         ^       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> hw/timer/exynos4210_mct.c:1399:9: warning: Value stored to 'index' is never read
>         index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
>         ^       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> hw/timer/exynos4210_mct.c:1441:9: warning: Value stored to 'index' is never read
>         index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
>         ^       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

It would be interesting to understand why we need the index for these
registers in exynos4210_mct_read() and not in exynos4210_mct_write().

Thanks,
Laurent

> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
> ---
> Cc: Igor Mitsyanko <i.mitsyanko@gmail.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/timer/exynos4210_mct.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/hw/timer/exynos4210_mct.c b/hw/timer/exynos4210_mct.c
> index 944120aea5..570cf7075b 100644
> --- a/hw/timer/exynos4210_mct.c
> +++ b/hw/timer/exynos4210_mct.c
> @@ -1367,7 +1367,6 @@ static void exynos4210_mct_write(void *opaque, hwaddr offset,
>  
>      case L0_TCNTB: case L1_TCNTB:
>          lt_i = GET_L_TIMER_IDX(offset);
> -        index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
>  
>          /*
>           * TCNTB is updated to internal register only after CNT expired.
> @@ -1396,7 +1395,6 @@ static void exynos4210_mct_write(void *opaque, hwaddr offset,
>  
>      case L0_ICNTB: case L1_ICNTB:
>          lt_i = GET_L_TIMER_IDX(offset);
> -        index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
>  
>          s->l_timer[lt_i].reg.wstat |= L_WSTAT_ICNTB_WRITE;
>          s->l_timer[lt_i].reg.cnt[L_REG_CNT_ICNTB] = value &
> @@ -1438,8 +1436,6 @@ static void exynos4210_mct_write(void *opaque, hwaddr offset,
>  
>      case L0_FRCNTB: case L1_FRCNTB:
>          lt_i = GET_L_TIMER_IDX(offset);
> -        index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
> -
>          DPRINTF("local timer[%d] FRCNTB write %llx\n", lt_i, value);
>  
>          s->l_timer[lt_i].reg.wstat |= L_WSTAT_FRCCNTB_WRITE;
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH v4 2/3] display/blizzard: Remove redundant statement in blizzard_draw_line16_32()
  2020-03-24  8:40     ` Laurent Vivier
@ 2020-03-24 11:49       ` Chenqun (kuhn)
  0 siblings, 0 replies; 11+ messages in thread
From: Chenqun (kuhn) @ 2020-03-24 11:49 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel, qemu-trivial
  Cc: Peter Maydell, philmd, Zhanghailiang, Euler Robot

>-----Original Message-----
>From: Laurent Vivier [mailto:laurent@vivier.eu]
>Sent: Tuesday, March 24, 2020 4:40 PM
>To: Chenqun (kuhn) <kuhn.chenqun@huawei.com>; qemu-devel@nongnu.org;
>qemu-trivial@nongnu.org
>Cc: Zhanghailiang <zhang.zhanghailiang@huawei.com>; philmd@redhat.com;
>Euler Robot <euler.robot@huawei.com>; Andrzej Zaborowski
><balrogg@gmail.com>; Peter Maydell <peter.maydell@linaro.org>
>Subject: Re: [PATCH v4 2/3] display/blizzard: Remove redundant statement in
>blizzard_draw_line16_32()
>
>Le 24/03/2020 à 09:38, Laurent Vivier a écrit :
>> Le 24/03/2020 à 09:22, Chen Qun a écrit :
>>> Clang static code analyzer show warning:
>>>   hw/display/blizzard.c:940:9: warning: Value stored to 'data' is never read
>>>         data >>= 5;
>>>         ^        ~
>>> Reported-by: Euler Robot <euler.robot@huawei.com>
>>> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
>>> ---
>>> Cc: Andrzej Zaborowski <balrogg@gmail.com>
>>> Cc: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>>  hw/display/blizzard.c | 1 -
>>>  1 file changed, 1 deletion(-)
>>>
>>> diff --git a/hw/display/blizzard.c b/hw/display/blizzard.c index
>>> 359e399c2a..62517bdf75 100644
>>> --- a/hw/display/blizzard.c
>>> +++ b/hw/display/blizzard.c
>>> @@ -937,7 +937,6 @@ static void blizzard_draw_line16_32(uint32_t *dest,
>>>          g = (data & 0x3f) << 2;
>>>          data >>= 6;
>>>          r = (data & 0x1f) << 3;
>>> -        data >>= 5;
>>>          *dest++ = rgb_to_pixel32(r, g, b);
>>>      }
>>>  }
>>>
>>
>> Perhaps it would be clearer to use extract32() to compute r, g and b?
>
>in fact extract16() as data is uint16_t...

Good Point, I will update it next verison.

Thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH v4 3/3] timer/exynos4210_mct: Remove redundant statement in exynos4210_mct_write()
  2020-03-24 10:59   ` Laurent Vivier
@ 2020-03-24 12:22     ` Chenqun (kuhn)
  2020-03-24 13:21       ` Laurent Vivier
  0 siblings, 1 reply; 11+ messages in thread
From: Chenqun (kuhn) @ 2020-03-24 12:22 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel, qemu-trivial
  Cc: Igor Mitsyanko, Peter Maydell, philmd, Zhanghailiang, Euler Robot

>-----Original Message-----
>From: Laurent Vivier [mailto:laurent@vivier.eu]
>Sent: Tuesday, March 24, 2020 6:59 PM
>To: Chenqun (kuhn) <kuhn.chenqun@huawei.com>; qemu-devel@nongnu.org;
>qemu-trivial@nongnu.org
>Cc: Zhanghailiang <zhang.zhanghailiang@huawei.com>; philmd@redhat.com;
>Euler Robot <euler.robot@huawei.com>; Igor Mitsyanko
><i.mitsyanko@gmail.com>; Peter Maydell <peter.maydell@linaro.org>
>Subject: Re: [PATCH v4 3/3] timer/exynos4210_mct: Remove redundant
>statement in exynos4210_mct_write()
>
>Le 24/03/2020 à 09:22, Chen Qun a écrit :
>> Clang static code analyzer show warning:
>> hw/timer/exynos4210_mct.c:1370:9: warning: Value stored to 'index' is never
>read
>>         index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
>>         ^       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> hw/timer/exynos4210_mct.c:1399:9: warning: Value stored to 'index' is never
>read
>>         index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
>>         ^       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> hw/timer/exynos4210_mct.c:1441:9: warning: Value stored to 'index' is never
>read
>>         index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
>>         ^       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>It would be interesting to understand why we need the index for these registers
>in exynos4210_mct_read() and not in exynos4210_mct_write().
>
I think the index can also be used in exynos4210_mct_write(), but the original author used a more obvious reg name instead of it.

The obvious reg name:
case L0_TCNTB: case L1_TCNTB:
           reg name is :  L_REG_CNT_TCNTB 

case L0_ICNTB: case L1_ICNTB:
           reg name is :   L_REG_CNT_ICNTB

case L0_FRCNTB: case L1_FRCNTB:
           reg name is :   L_REG_CNT_FRCCNTB

Thanks.
>
>> Reported-by: Euler Robot <euler.robot@huawei.com>
>> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
>> ---
>> Cc: Igor Mitsyanko <i.mitsyanko@gmail.com>
>> Cc: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  hw/timer/exynos4210_mct.c | 4 ----
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/hw/timer/exynos4210_mct.c b/hw/timer/exynos4210_mct.c
>> index 944120aea5..570cf7075b 100644
>> --- a/hw/timer/exynos4210_mct.c
>> +++ b/hw/timer/exynos4210_mct.c
>> @@ -1367,7 +1367,6 @@ static void exynos4210_mct_write(void *opaque,
>> hwaddr offset,
>>
>>      case L0_TCNTB: case L1_TCNTB:
>>          lt_i = GET_L_TIMER_IDX(offset);
>> -        index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
>>
>>          /*
>>           * TCNTB is updated to internal register only after CNT expired.
>> @@ -1396,7 +1395,6 @@ static void exynos4210_mct_write(void *opaque,
>> hwaddr offset,
>>
>>      case L0_ICNTB: case L1_ICNTB:
>>          lt_i = GET_L_TIMER_IDX(offset);
>> -        index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
>>
>>          s->l_timer[lt_i].reg.wstat |= L_WSTAT_ICNTB_WRITE;
>>          s->l_timer[lt_i].reg.cnt[L_REG_CNT_ICNTB] = value & @@
>> -1438,8 +1436,6 @@ static void exynos4210_mct_write(void *opaque,
>> hwaddr offset,
>>
>>      case L0_FRCNTB: case L1_FRCNTB:
>>          lt_i = GET_L_TIMER_IDX(offset);
>> -        index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
>> -
>>          DPRINTF("local timer[%d] FRCNTB write %llx\n", lt_i, value);
>>
>>          s->l_timer[lt_i].reg.wstat |= L_WSTAT_FRCCNTB_WRITE;
>>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 3/3] timer/exynos4210_mct: Remove redundant statement in exynos4210_mct_write()
  2020-03-24 12:22     ` Chenqun (kuhn)
@ 2020-03-24 13:21       ` Laurent Vivier
  0 siblings, 0 replies; 11+ messages in thread
From: Laurent Vivier @ 2020-03-24 13:21 UTC (permalink / raw)
  To: Chenqun (kuhn), qemu-devel, qemu-trivial
  Cc: Igor Mitsyanko, Peter Maydell, philmd, Zhanghailiang, Euler Robot

Le 24/03/2020 à 13:22, Chenqun (kuhn) a écrit :
>> -----Original Message-----
>> From: Laurent Vivier [mailto:laurent@vivier.eu]
>> Sent: Tuesday, March 24, 2020 6:59 PM
>> To: Chenqun (kuhn) <kuhn.chenqun@huawei.com>; qemu-devel@nongnu.org;
>> qemu-trivial@nongnu.org
>> Cc: Zhanghailiang <zhang.zhanghailiang@huawei.com>; philmd@redhat.com;
>> Euler Robot <euler.robot@huawei.com>; Igor Mitsyanko
>> <i.mitsyanko@gmail.com>; Peter Maydell <peter.maydell@linaro.org>
>> Subject: Re: [PATCH v4 3/3] timer/exynos4210_mct: Remove redundant
>> statement in exynos4210_mct_write()
>>
>> Le 24/03/2020 à 09:22, Chen Qun a écrit :
>>> Clang static code analyzer show warning:
>>> hw/timer/exynos4210_mct.c:1370:9: warning: Value stored to 'index' is never
>> read
>>>         index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
>>>         ^       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> hw/timer/exynos4210_mct.c:1399:9: warning: Value stored to 'index' is never
>> read
>>>         index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
>>>         ^       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> hw/timer/exynos4210_mct.c:1441:9: warning: Value stored to 'index' is never
>> read
>>>         index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
>>>         ^       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> It would be interesting to understand why we need the index for these registers
>> in exynos4210_mct_read() and not in exynos4210_mct_write().
>>
> I think the index can also be used in exynos4210_mct_write(), but the original author used a more obvious reg name instead of it.
> 
> The obvious reg name:
> case L0_TCNTB: case L1_TCNTB:
>            reg name is :  L_REG_CNT_TCNTB 
> 
> case L0_ICNTB: case L1_ICNTB:
>            reg name is :   L_REG_CNT_ICNTB
> 
> case L0_FRCNTB: case L1_FRCNTB:
>            reg name is :   L_REG_CNT_FRCCNTB
>

Yes, you're right.

Computed index gives the value used with s->l_timer[lt_i].reg.cnt[].

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Thanks,
Laurent



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-03-24 13:22 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-24  8:22 [PATCH v4 0/3] redundant code: Fix warnings reported by Clang static code analyzer Chen Qun
2020-03-24  8:22 ` [PATCH v4 1/3] scsi/esp-pci: Remove redundant statement in esp_pci_io_write() Chen Qun
2020-03-24  8:27   ` Laurent Vivier
2020-03-24  8:22 ` [PATCH v4 2/3] display/blizzard: Remove redundant statement in blizzard_draw_line16_32() Chen Qun
2020-03-24  8:38   ` Laurent Vivier
2020-03-24  8:40     ` Laurent Vivier
2020-03-24 11:49       ` Chenqun (kuhn)
2020-03-24  8:22 ` [PATCH v4 3/3] timer/exynos4210_mct: Remove redundant statement in exynos4210_mct_write() Chen Qun
2020-03-24 10:59   ` Laurent Vivier
2020-03-24 12:22     ` Chenqun (kuhn)
2020-03-24 13:21       ` Laurent Vivier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.