1. Patchew
  2. QEMU
  3. [PATCH v4 0/3] redundant code: Fix warnings reported by Clang static code analyzer
  4. View patch

[PATCH v4 3/3] timer/exynos4210_mct: Remove redundant statement in exynos4210_mct_write()

Chen Qun posted 3 patches 5 years, 10 months ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Igor Mitsyanko <i.mitsyanko@gmail.com>, Peter Maydell <peter.maydell@linaro.org>, Andrzej Zaborowski <balrogg@gmail.com>, Fam Zheng <fam@euphon.net>
There is a newer version of this series
[PATCH v4 3/3] timer/exynos4210_mct: Remove redundant statement in exynos4210_mct_write()
Posted by Chen Qun 5 years, 10 months ago 
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
Re: [PATCH v4 3/3] timer/exynos4210_mct: Remove redundant statement in exynos4210_mct_write()
Posted by Laurent Vivier 5 years, 10 months ago 
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;
>
RE: [PATCH v4 3/3] timer/exynos4210_mct: Remove redundant statement in exynos4210_mct_write()
Posted by Chenqun (kuhn) 5 years, 10 months ago 
>-----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;
>>
Re: [PATCH v4 3/3] timer/exynos4210_mct: Remove redundant statement in exynos4210_mct_write()
Posted by Laurent Vivier 5 years, 10 months ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.