* [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.