[PATCH-for-5.0 v2 00/11] misc: Trivial static code analyzer fixes

[PATCH-for-5.0 v2 00/11] misc: Trivial static code analyzer fixes

[Philippe Mathieu-Daudé]

Fix trivial warnings reported by the Clang static code analyzer.

Since v1:
- Addressed Markus/Zoltan/Aleksandar review comments

Philippe Mathieu-Daudé (11):
  block: Avoid dead assignment
  blockdev: Remove dead assignment
  hw/i2c/pm_smbus: Remove dead assignment
  hw/input/adb-kbd: Remove dead assignment
  hw/ide/sii3112: Remove dead assignment
  hw/isa/i82378: Remove dead assignment
  hw/gpio/aspeed_gpio: Remove dead assignment
  hw/timer/exynos4210_mct: Remove dead assignments
  hw/timer/stm32f2xx_timer: Remove dead assignment
  hw/timer/pxa2xx_timer: Add assertion to silent static analyzer warning
  hw/scsi/esp-pci: Remove dead assignment

 block.c                    | 2 +-
 blockdev.c                 | 2 +-
 hw/gpio/aspeed_gpio.c      | 2 +-
 hw/i2c/pm_smbus.c          | 1 -
 hw/ide/sii3112.c           | 5 +++--
 hw/input/adb-kbd.c         | 6 +-----
 hw/isa/i82378.c            | 8 ++++----
 hw/scsi/esp-pci.c          | 1 -
 hw/timer/exynos4210_mct.c  | 3 ---
 hw/timer/pxa2xx_timer.c    | 1 +
 hw/timer/stm32f2xx_timer.c | 1 -
 11 files changed, 12 insertions(+), 20 deletions(-)

-- 
2.21.1

[PATCH-for-5.0 v2 01/11] block: Avoid dead assignment

[Philippe Mathieu-Daudé]

Fix warning reported by Clang static code analyzer:

  block.c:3167:5: warning: Value stored to 'ret' is never read
      ret = bdrv_fill_options(&options, filename, &flags, &local_err);
      ^     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Fixes: 462f5bcf6
Reported-by: Clang Static Analyzer
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: Keep 'ret' assigned and check it (Markus)
---
 block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block.c b/block.c
index a2542c977b..a6f069d8bd 100644
--- a/block.c
+++ b/block.c
@@ -3165,7 +3165,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
     }
 
     ret = bdrv_fill_options(&options, filename, &flags, &local_err);
-    if (local_err) {
+    if (ret < 0) {
         goto fail;
     }
 
-- 
2.21.1

[PATCH-for-5.0 v2 02/11] blockdev: Remove dead assignment

[Philippe Mathieu-Daudé]

Fix warning reported by Clang static code analyzer:

    CC      blockdev.o
  blockdev.c:2744:5: warning: Value stored to 'ret' is never read
      ret = blk_truncate(blk, size, false, PREALLOC_MODE_OFF, errp);
      ^     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

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

diff --git a/blockdev.c b/blockdev.c
index fa8630cb41..6effd5afaa 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2741,7 +2741,7 @@ void qmp_block_resize(bool has_device, const char *device,
     }
 
     bdrv_drained_begin(bs);
-    ret = blk_truncate(blk, size, false, PREALLOC_MODE_OFF, errp);
+    blk_truncate(blk, size, false, PREALLOC_MODE_OFF, errp);
     bdrv_drained_end(bs);
 
 out:
-- 
2.21.1

[PATCH-for-5.0 v2 03/11] hw/i2c/pm_smbus: Remove dead assignment

[Philippe Mathieu-Daudé]

Fix warning reported by Clang static code analyzer:

    CC      hw/i2c/pm_smbus.o
  hw/i2c/pm_smbus.c:187:17: warning: Value stored to 'ret' is never read
                  ret = 0;
                  ^     ~

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

diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c
index 36994ff585..4728540c37 100644
--- a/hw/i2c/pm_smbus.c
+++ b/hw/i2c/pm_smbus.c
@@ -184,7 +184,6 @@ static void smb_transaction(PMSMBus *s)
                 s->smb_stat |= STS_HOST_BUSY | STS_BYTE_DONE;
                 s->smb_data[0] = s->smb_blkdata;
                 s->smb_index = 0;
-                ret = 0;
             }
             goto out;
         }
-- 
2.21.1

[PATCH-for-5.0 v2 04/11] hw/input/adb-kbd: Remove dead assignment

[Philippe Mathieu-Daudé]

Since commit 5a1f49718 the 'olen' variable is not really
used. Remove it to fix a warning reported by Clang static
code analyzer:

    CC      hw/input/adb-kbd.o
  hw/input/adb-kbd.c:200:5: warning: Value stored to 'olen' is never read
      olen = 0;
      ^      ~

Fixes: 5a1f49718 (adb: add support for QKeyCode)
Reported-by: Clang Static Analyzer
Suggested-by: BALATON Zoltan <balaton@eik.bme.hu>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: Remove 'olen' (Zoltan)
---
 hw/input/adb-kbd.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/hw/input/adb-kbd.c b/hw/input/adb-kbd.c
index 0ba8207589..a6d5c9b7c9 100644
--- a/hw/input/adb-kbd.c
+++ b/hw/input/adb-kbd.c
@@ -195,9 +195,7 @@ static int adb_kbd_poll(ADBDevice *d, uint8_t *obuf)
 {
     KBDState *s = ADB_KEYBOARD(d);
     int keycode;
-    int olen;
 
-    olen = 0;
     if (s->count == 0) {
         return 0;
     }
@@ -216,7 +214,6 @@ static int adb_kbd_poll(ADBDevice *d, uint8_t *obuf)
     if (keycode == 0x7f) {
         obuf[0] = 0x7f;
         obuf[1] = 0x7f;
-        olen = 2;
     } else {
         obuf[0] = keycode;
         /* NOTE: the power key key-up is the two byte sequence 0xff 0xff;
@@ -224,10 +221,9 @@ static int adb_kbd_poll(ADBDevice *d, uint8_t *obuf)
          * byte, but choose not to bother.
          */
         obuf[1] = 0xff;
-        olen = 2;
     }
 
-    return olen;
+    return 2;
 }
 
 static int adb_kbd_request(ADBDevice *d, uint8_t *obuf,
-- 
2.21.1

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

[Philippe Mathieu-Daudé]

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;
          ^     ~

Fixes: a9dd6604
Reported-by: Clang Static Analyzer
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: Fix the correct function (Aleksandar review)
---
 hw/ide/sii3112.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
index 06605d7af2..b2ff6dd6d9 100644
--- a/hw/ide/sii3112.c
+++ b/hw/ide/sii3112.c
@@ -42,7 +42,7 @@ static uint64_t sii3112_reg_read(void *opaque, hwaddr addr,
                                 unsigned int size)
 {
     SiI3112PCIState *d = opaque;
-    uint64_t val = 0;
+    uint64_t val;
 
     switch (addr) {
     case 0x00:
@@ -126,6 +126,7 @@ static uint64_t sii3112_reg_read(void *opaque, hwaddr addr,
         break;
     default:
         val = 0;
+        break;
     }
     trace_sii3112_read(size, addr, val);
     return val;
@@ -201,7 +202,7 @@ static void sii3112_reg_write(void *opaque, hwaddr addr,
         d->regs[1].sien = (val >> 16) & 0x3eed;
         break;
     default:
-        val = 0;
+        break;
     }
 }
 
-- 
2.21.1

[PATCH-for-5.0 v2 06/11] hw/isa/i82378: Remove dead assignment

[Philippe Mathieu-Daudé]

Rename the unique variable assigned as 'pit' which better
represents what it holds, to fix a warning reported by the
Clang static code analyzer:

    CC      hw/isa/i82378.o
  hw/isa/i82378.c:108:5: warning: Value stored to 'isa' is never read
      isa = isa_create_simple(isabus, "i82374");
      ^     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/isa/i82378.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
index dcb6b479ea..d9e6c7fa00 100644
--- a/hw/isa/i82378.c
+++ b/hw/isa/i82378.c
@@ -67,7 +67,7 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
     I82378State *s = I82378(dev);
     uint8_t *pci_conf;
     ISABus *isabus;
-    ISADevice *isa;
+    ISADevice *pit;
 
     pci_conf = pci->config;
     pci_set_word(pci_conf + PCI_COMMAND,
@@ -99,13 +99,13 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
     isa_bus_irqs(isabus, s->i8259);
 
     /* 1 82C54 (pit) */
-    isa = i8254_pit_init(isabus, 0x40, 0, NULL);
+    pit = i8254_pit_init(isabus, 0x40, 0, NULL);
 
     /* speaker */
-    pcspk_init(isabus, isa);
+    pcspk_init(isabus, pit);
 
     /* 2 82C37 (dma) */
-    isa = isa_create_simple(isabus, "i82374");
+    isa_create_simple(isabus, "i82374");
 }
 
 static void i82378_init(Object *obj)
-- 
2.21.1

[PATCH-for-5.0 v2 07/11] hw/gpio/aspeed_gpio: Remove dead assignment

[Philippe Mathieu-Daudé]

Fix warning reported by Clang static code analyzer:

  hw/gpio/aspeed_gpio.c:717:18: warning: Value stored to 'g_idx' during its initialization is never read
      int set_idx, g_idx = *group_idx;
                   ^~~~~   ~~~~~~~~~~

Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: Do not declare g_idx in for() (Zoltan)
---
 hw/gpio/aspeed_gpio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index 41e11ea9b0..bd19db31f4 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -714,7 +714,7 @@ static void aspeed_gpio_write(void *opaque, hwaddr offset, uint64_t data,
 static int get_set_idx(AspeedGPIOState *s, const char *group, int *group_idx)
 {
     AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
-    int set_idx, g_idx = *group_idx;
+    int set_idx, g_idx;
 
     for (set_idx = 0; set_idx < agc->nr_gpio_sets; set_idx++) {
         const GPIOSetProperties *set_props = &agc->props[set_idx];
-- 
2.21.1

[PATCH-for-5.0 v2 08/11] hw/timer/exynos4210_mct: Remove dead assignments

[Philippe Mathieu-Daudé]

Fix warnings reported by Clang static code analyzer:

  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: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/timer/exynos4210_mct.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/timer/exynos4210_mct.c b/hw/timer/exynos4210_mct.c
index 944120aea5..c0a25e71ec 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,7 +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);
 
-- 
2.21.1

[PATCH-for-5.0 v2 09/11] hw/timer/stm32f2xx_timer: Remove dead assignment

[Philippe Mathieu-Daudé]

Fix warning reported by Clang static code analyzer:

    CC      hw/timer/stm32f2xx_timer.o
  hw/timer/stm32f2xx_timer.c:225:9: warning: Value stored to 'value' is never read
          value = timer_val;
          ^       ~~~~~~~~~

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

diff --git a/hw/timer/stm32f2xx_timer.c b/hw/timer/stm32f2xx_timer.c
index 06ec8a02c2..ba8694dcd3 100644
--- a/hw/timer/stm32f2xx_timer.c
+++ b/hw/timer/stm32f2xx_timer.c
@@ -222,7 +222,6 @@ static void stm32f2xx_timer_write(void *opaque, hwaddr offset,
     case TIM_PSC:
         timer_val = stm32f2xx_ns_to_ticks(s, now) - s->tick_offset;
         s->tim_psc = value & 0xFFFF;
-        value = timer_val;
         break;
     case TIM_CNT:
         timer_val = value;
-- 
2.21.1

[PATCH-for-5.0 v2 10/11] hw/timer/pxa2xx_timer: Add assertion to silent static analyzer warning

[Philippe Mathieu-Daudé]

pxa2xx_timer_tick4() takes an opaque pointer, then calls
pxa2xx_timer_update4(), so the static analyzer can not
verify that the 'n < 8':

  425 static void pxa2xx_timer_tick4(void *opaque)
  426 {
  427     PXA2xxTimer4 *t = (PXA2xxTimer4 *) opaque;
  428     PXA2xxTimerInfo *i = (PXA2xxTimerInfo *) t->tm.info;
  429
  430     pxa2xx_timer_tick(&t->tm);
  433     if (t->control & (1 << 6))
  434         pxa2xx_timer_update4(i, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), t->tm.num - 4);

  135 static void pxa2xx_timer_update4(void *opaque, uint64_t now_qemu, int n)
  136 {
  137     PXA2xxTimerInfo *s = (PXA2xxTimerInfo *) opaque;
  140     static const int counters[8] = { 0, 0, 0, 0, 4, 4, 6, 6 };
  142
  143     if (s->tm4[n].control & (1 << 7))
  144         counter = n;
  145     else
  146         counter = counters[n];

Add an assert() to give the static analyzer a hint, this fixes a
warning reported by Clang static code analyzer:

    CC      hw/timer/pxa2xx_timer.o
  hw/timer/pxa2xx_timer.c:146:17: warning: Assigned value is garbage or undefined
          counter = counters[n];
                  ^ ~~~~~~~~~~~

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

diff --git a/hw/timer/pxa2xx_timer.c b/hw/timer/pxa2xx_timer.c
index cd172cc1e9..944c165889 100644
--- a/hw/timer/pxa2xx_timer.c
+++ b/hw/timer/pxa2xx_timer.c
@@ -140,6 +140,7 @@ static void pxa2xx_timer_update4(void *opaque, uint64_t now_qemu, int n)
     static const int counters[8] = { 0, 0, 0, 0, 4, 4, 6, 6 };
     int counter;
 
+    assert(n < ARRAY_SIZE(counters));
     if (s->tm4[n].control & (1 << 7))
         counter = n;
     else
-- 
2.21.1

[PATCH-for-5.0 v2 11/11] hw/scsi/esp-pci: Remove dead assignment

[Philippe Mathieu-Daudé]

Fix warning reported by Clang static code analyzer:

    CC      hw/scsi/esp-pci.o
  hw/scsi/esp-pci.c:198:9: warning: Value stored to 'size' is never read
          size = 4;
          ^      ~

Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 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.21.1

Re: [PATCH-for-5.0 v2 00/11] misc: Trivial static code analyzer fixes

[Philippe Mathieu-Daudé]

On 3/21/20 3:40 PM, Philippe Mathieu-Daudé wrote:
> Fix trivial warnings reported by the Clang static code analyzer.

I forgot to add the official Clang static code analyzer documentation is 
on https://clang-analyzer.llvm.org/ and on Fedora I simply used it as:

$ sudo dnf install clang-analyzer
$ ../configure
$ scan-build make

> 
> Since v1:
> - Addressed Markus/Zoltan/Aleksandar review comments
> 
> Philippe Mathieu-Daudé (11):
>    block: Avoid dead assignment
>    blockdev: Remove dead assignment
>    hw/i2c/pm_smbus: Remove dead assignment
>    hw/input/adb-kbd: Remove dead assignment
>    hw/ide/sii3112: Remove dead assignment
>    hw/isa/i82378: Remove dead assignment
>    hw/gpio/aspeed_gpio: Remove dead assignment
>    hw/timer/exynos4210_mct: Remove dead assignments
>    hw/timer/stm32f2xx_timer: Remove dead assignment
>    hw/timer/pxa2xx_timer: Add assertion to silent static analyzer warning
>    hw/scsi/esp-pci: Remove dead assignment
> 
>   block.c                    | 2 +-
>   blockdev.c                 | 2 +-
>   hw/gpio/aspeed_gpio.c      | 2 +-
>   hw/i2c/pm_smbus.c          | 1 -
>   hw/ide/sii3112.c           | 5 +++--
>   hw/input/adb-kbd.c         | 6 +-----
>   hw/isa/i82378.c            | 8 ++++----
>   hw/scsi/esp-pci.c          | 1 -
>   hw/timer/exynos4210_mct.c  | 3 ---
>   hw/timer/pxa2xx_timer.c    | 1 +
>   hw/timer/stm32f2xx_timer.c | 1 -
>   11 files changed, 12 insertions(+), 20 deletions(-)
> 

Re: [PATCH-for-5.0 v2 04/11] hw/input/adb-kbd: Remove dead assignment

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1980 bytes --]

On Sat, Mar 21, 2020 at 03:41:03PM +0100, Philippe Mathieu-Daudé wrote:
> Since commit 5a1f49718 the 'olen' variable is not really
> used. Remove it to fix a warning reported by Clang static
> code analyzer:
> 
>     CC      hw/input/adb-kbd.o
>   hw/input/adb-kbd.c:200:5: warning: Value stored to 'olen' is never read
>       olen = 0;
>       ^      ~
> 
> Fixes: 5a1f49718 (adb: add support for QKeyCode)
> Reported-by: Clang Static Analyzer
> Suggested-by: BALATON Zoltan <balaton@eik.bme.hu>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> v2: Remove 'olen' (Zoltan)
> ---
>  hw/input/adb-kbd.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/hw/input/adb-kbd.c b/hw/input/adb-kbd.c
> index 0ba8207589..a6d5c9b7c9 100644
> --- a/hw/input/adb-kbd.c
> +++ b/hw/input/adb-kbd.c
> @@ -195,9 +195,7 @@ static int adb_kbd_poll(ADBDevice *d, uint8_t *obuf)
>  {
>      KBDState *s = ADB_KEYBOARD(d);
>      int keycode;
> -    int olen;
>  
> -    olen = 0;
>      if (s->count == 0) {
>          return 0;
>      }
> @@ -216,7 +214,6 @@ static int adb_kbd_poll(ADBDevice *d, uint8_t *obuf)
>      if (keycode == 0x7f) {
>          obuf[0] = 0x7f;
>          obuf[1] = 0x7f;
> -        olen = 2;
>      } else {
>          obuf[0] = keycode;
>          /* NOTE: the power key key-up is the two byte sequence 0xff 0xff;
> @@ -224,10 +221,9 @@ static int adb_kbd_poll(ADBDevice *d, uint8_t *obuf)
>           * byte, but choose not to bother.
>           */
>          obuf[1] = 0xff;
> -        olen = 2;
>      }
>  
> -    return olen;
> +    return 2;
>  }
>  
>  static int adb_kbd_request(ADBDevice *d, uint8_t *obuf,

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH-for-5.0 v2 07/11] hw/gpio/aspeed_gpio: Remove dead assignment

[Cédric Le Goater]

On 3/21/20 3:41 PM, Philippe Mathieu-Daudé wrote:
> Fix warning reported by Clang static code analyzer:
> 
>   hw/gpio/aspeed_gpio.c:717:18: warning: Value stored to 'g_idx' during its initialization is never read
>       int set_idx, g_idx = *group_idx;
>                    ^~~~~   ~~~~~~~~~~
> 
> Reported-by: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

> ---
> v2: Do not declare g_idx in for() (Zoltan)
> ---
>  hw/gpio/aspeed_gpio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
> index 41e11ea9b0..bd19db31f4 100644
> --- a/hw/gpio/aspeed_gpio.c
> +++ b/hw/gpio/aspeed_gpio.c
> @@ -714,7 +714,7 @@ static void aspeed_gpio_write(void *opaque, hwaddr offset, uint64_t data,
>  static int get_set_idx(AspeedGPIOState *s, const char *group, int *group_idx)
>  {
>      AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
> -    int set_idx, g_idx = *group_idx;
> +    int set_idx, g_idx;
>  
>      for (set_idx = 0; set_idx < agc->nr_gpio_sets; set_idx++) {
>          const GPIOSetProperties *set_props = &agc->props[set_idx];
> 

Re: [PATCH-for-5.0 v2 00/11] misc: Trivial static code analyzer fixes

[Laurent Vivier]

Le 21/03/2020 à 15:40, Philippe Mathieu-Daudé a écrit :
> Fix trivial warnings reported by the Clang static code analyzer.
> 
> Since v1:
> - Addressed Markus/Zoltan/Aleksandar review comments
> 
> Philippe Mathieu-Daudé (11):
>   block: Avoid dead assignment
>   blockdev: Remove dead assignment
>   hw/i2c/pm_smbus: Remove dead assignment
>   hw/input/adb-kbd: Remove dead assignment
>   hw/ide/sii3112: Remove dead assignment
>   hw/isa/i82378: Remove dead assignment
>   hw/gpio/aspeed_gpio: Remove dead assignment
>   hw/timer/exynos4210_mct: Remove dead assignments
>   hw/timer/stm32f2xx_timer: Remove dead assignment
>   hw/timer/pxa2xx_timer: Add assertion to silent static analyzer warning
>   hw/scsi/esp-pci: Remove dead assignment
> 
>  block.c                    | 2 +-
>  blockdev.c                 | 2 +-
>  hw/gpio/aspeed_gpio.c      | 2 +-
>  hw/i2c/pm_smbus.c          | 1 -
>  hw/ide/sii3112.c           | 5 +++--
>  hw/input/adb-kbd.c         | 6 +-----
>  hw/isa/i82378.c            | 8 ++++----
>  hw/scsi/esp-pci.c          | 1 -
>  hw/timer/exynos4210_mct.c  | 3 ---
>  hw/timer/pxa2xx_timer.c    | 1 +
>  hw/timer/stm32f2xx_timer.c | 1 -
>  11 files changed, 12 insertions(+), 20 deletions(-)
> 

I think your series covers cases already covered by:

[PATCH v3 00/12] redundant code: Fix warnings reported by Clang static
code analyzer
https://patchew.org/QEMU/20200302130715.29440-1-kuhn.ch

Thanks,
Laurent

Re: [PATCH-for-5.0 v2 11/11] hw/scsi/esp-pci: Remove dead assignment

[Laurent Vivier]

Le 21/03/2020 à 15:41, Philippe Mathieu-Daudé a écrit :
> Fix warning reported by Clang static code analyzer:
> 
>     CC      hw/scsi/esp-pci.o
>   hw/scsi/esp-pci.c:198:9: warning: Value stored to 'size' is never read
>           size = 4;
>           ^      ~
> 
> Reported-by: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  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) {
> 

This one has already been reported.

I would prefer an assert(), see my comment:
https://patchew.org/QEMU/20200302130715.29440-1-kuhn.chenqun@huawei.com/20200302130715.29440-6-kuhn.chenqun@huawei.com/

Thanks,
Laurent

Re: [PATCH-for-5.0 v2 00/11] misc: Trivial static code analyzer fixes

[Philippe Mathieu-Daudé]

On 3/23/20 3:32 PM, Laurent Vivier wrote:
> Le 21/03/2020 à 15:40, Philippe Mathieu-Daudé a écrit :
>> Fix trivial warnings reported by the Clang static code analyzer.
>>
>> Since v1:
>> - Addressed Markus/Zoltan/Aleksandar review comments
>>
>> Philippe Mathieu-Daudé (11):
>>    block: Avoid dead assignment
>>    blockdev: Remove dead assignment
>>    hw/i2c/pm_smbus: Remove dead assignment
>>    hw/input/adb-kbd: Remove dead assignment
>>    hw/ide/sii3112: Remove dead assignment
>>    hw/isa/i82378: Remove dead assignment
>>    hw/gpio/aspeed_gpio: Remove dead assignment
>>    hw/timer/exynos4210_mct: Remove dead assignments
>>    hw/timer/stm32f2xx_timer: Remove dead assignment
>>    hw/timer/pxa2xx_timer: Add assertion to silent static analyzer warning
>>    hw/scsi/esp-pci: Remove dead assignment
>>
>>   block.c                    | 2 +-
>>   blockdev.c                 | 2 +-
>>   hw/gpio/aspeed_gpio.c      | 2 +-
>>   hw/i2c/pm_smbus.c          | 1 -
>>   hw/ide/sii3112.c           | 5 +++--
>>   hw/input/adb-kbd.c         | 6 +-----
>>   hw/isa/i82378.c            | 8 ++++----
>>   hw/scsi/esp-pci.c          | 1 -
>>   hw/timer/exynos4210_mct.c  | 3 ---
>>   hw/timer/pxa2xx_timer.c    | 1 +
>>   hw/timer/stm32f2xx_timer.c | 1 -
>>   11 files changed, 12 insertions(+), 20 deletions(-)
>>
> 
> I think your series covers cases already covered by:
> 
> [PATCH v3 00/12] redundant code: Fix warnings reported by Clang static
> code analyzer
> https://patchew.org/QEMU/20200302130715.29440-1-kuhn.ch

Unfortunately [for me...] I don't have v3 in my INBOX... *sigh*
This was 3 weeks ago. *sigh*.

I can see the series in the archives:
https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg00219.html
But I can't find the outcome, was it queued in the trivial tree?
Any idea when this will be merged in the master tree?

What a waste of time...

> 
> Thanks,
> Laurent
> 

Re: [PATCH-for-5.0 v2 00/11] misc: Trivial static code analyzer fixes

[Laurent Vivier]

Le 23/03/2020 à 15:45, Philippe Mathieu-Daudé a écrit :
> On 3/23/20 3:32 PM, Laurent Vivier wrote:
>> Le 21/03/2020 à 15:40, Philippe Mathieu-Daudé a écrit :
>>> Fix trivial warnings reported by the Clang static code analyzer.
>>>
>>> Since v1:
>>> - Addressed Markus/Zoltan/Aleksandar review comments
>>>
>>> Philippe Mathieu-Daudé (11):
>>>    block: Avoid dead assignment
>>>    blockdev: Remove dead assignment
>>>    hw/i2c/pm_smbus: Remove dead assignment
>>>    hw/input/adb-kbd: Remove dead assignment
>>>    hw/ide/sii3112: Remove dead assignment
>>>    hw/isa/i82378: Remove dead assignment
>>>    hw/gpio/aspeed_gpio: Remove dead assignment
>>>    hw/timer/exynos4210_mct: Remove dead assignments
>>>    hw/timer/stm32f2xx_timer: Remove dead assignment
>>>    hw/timer/pxa2xx_timer: Add assertion to silent static analyzer
>>> warning
>>>    hw/scsi/esp-pci: Remove dead assignment
>>>
>>>   block.c                    | 2 +-
>>>   blockdev.c                 | 2 +-
>>>   hw/gpio/aspeed_gpio.c      | 2 +-
>>>   hw/i2c/pm_smbus.c          | 1 -
>>>   hw/ide/sii3112.c           | 5 +++--
>>>   hw/input/adb-kbd.c         | 6 +-----
>>>   hw/isa/i82378.c            | 8 ++++----
>>>   hw/scsi/esp-pci.c          | 1 -
>>>   hw/timer/exynos4210_mct.c  | 3 ---
>>>   hw/timer/pxa2xx_timer.c    | 1 +
>>>   hw/timer/stm32f2xx_timer.c | 1 -
>>>   11 files changed, 12 insertions(+), 20 deletions(-)
>>>
>>
>> I think your series covers cases already covered by:
>>
>> [PATCH v3 00/12] redundant code: Fix warnings reported by Clang static
>> code analyzer
>> https://patchew.org/QEMU/20200302130715.29440-1-kuhn.ch
> 
> Unfortunately [for me...] I don't have v3 in my INBOX... *sigh*
> This was 3 weeks ago. *sigh*.
> 
> I can see the series in the archives:
> https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg00219.html
> But I can't find the outcome, was it queued in the trivial tree?
> Any idea when this will be merged in the master tree?

Some patches are already merged via trivial (1, 2 (should go by SCSI
queue) 3, 5, 6, 7, 9, 11 (by USB queue), 12).

But others needed R-b tags or new version. I didn't check which of your
patches are already covered by this series.

I'm sorry to not have checked your series earlier...

Thanks,
Laurent

Re: [PATCH-for-5.0 v2 00/11] misc: Trivial static code analyzer fixes

[Philippe Mathieu-Daudé]

On 3/23/20 3:55 PM, Laurent Vivier wrote:
> Le 23/03/2020 à 15:45, Philippe Mathieu-Daudé a écrit :
>> On 3/23/20 3:32 PM, Laurent Vivier wrote:
>>> Le 21/03/2020 à 15:40, Philippe Mathieu-Daudé a écrit :
>>>> Fix trivial warnings reported by the Clang static code analyzer.
>>>>
>>>> Since v1:
>>>> - Addressed Markus/Zoltan/Aleksandar review comments
>>>>
>>>> Philippe Mathieu-Daudé (11):
>>>>     block: Avoid dead assignment
>>>>     blockdev: Remove dead assignment
>>>>     hw/i2c/pm_smbus: Remove dead assignment
>>>>     hw/input/adb-kbd: Remove dead assignment
>>>>     hw/ide/sii3112: Remove dead assignment
>>>>     hw/isa/i82378: Remove dead assignment
>>>>     hw/gpio/aspeed_gpio: Remove dead assignment
>>>>     hw/timer/exynos4210_mct: Remove dead assignments
>>>>     hw/timer/stm32f2xx_timer: Remove dead assignment
>>>>     hw/timer/pxa2xx_timer: Add assertion to silent static analyzer
>>>> warning
>>>>     hw/scsi/esp-pci: Remove dead assignment
>>>>
>>>>    block.c                    | 2 +-
>>>>    blockdev.c                 | 2 +-
>>>>    hw/gpio/aspeed_gpio.c      | 2 +-
>>>>    hw/i2c/pm_smbus.c          | 1 -
>>>>    hw/ide/sii3112.c           | 5 +++--
>>>>    hw/input/adb-kbd.c         | 6 +-----
>>>>    hw/isa/i82378.c            | 8 ++++----
>>>>    hw/scsi/esp-pci.c          | 1 -
>>>>    hw/timer/exynos4210_mct.c  | 3 ---
>>>>    hw/timer/pxa2xx_timer.c    | 1 +
>>>>    hw/timer/stm32f2xx_timer.c | 1 -
>>>>    11 files changed, 12 insertions(+), 20 deletions(-)
>>>>
>>>
>>> I think your series covers cases already covered by:
>>>
>>> [PATCH v3 00/12] redundant code: Fix warnings reported by Clang static
>>> code analyzer
>>> https://patchew.org/QEMU/20200302130715.29440-1-kuhn.ch
>>
>> Unfortunately [for me...] I don't have v3 in my INBOX... *sigh*
>> This was 3 weeks ago. *sigh*.
>>
>> I can see the series in the archives:
>> https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg00219.html
>> But I can't find the outcome, was it queued in the trivial tree?
>> Any idea when this will be merged in the master tree?
> 
> Some patches are already merged via trivial (1, 2 (should go by SCSI
> queue) 3, 5, 6, 7, 9, 11 (by USB queue), 12).
> 
> But others needed R-b tags or new version. I didn't check which of your
> patches are already covered by this series.
> 
> I'm sorry to not have checked your series earlier...

Don't be sorry, the problem is my INBOX that is unreliable.

I was using NNTP last month until I heard it was working properly again, 
which is not the case apparently. I'll try to find them on NNTP and 
review them.

> 
> Thanks,
> Laurent
> 

Re: [PATCH-for-5.0 v2 01/11] block: Avoid dead assignment

[Alistair Francis]

On Sat, Mar 21, 2020 at 7:42 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Fix warning reported by Clang static code analyzer:
>
>   block.c:3167:5: warning: Value stored to 'ret' is never read
>       ret = bdrv_fill_options(&options, filename, &flags, &local_err);
>       ^     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Fixes: 462f5bcf6
> Reported-by: Clang Static Analyzer
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> v2: Keep 'ret' assigned and check it (Markus)
> ---
>  block.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block.c b/block.c
> index a2542c977b..a6f069d8bd 100644
> --- a/block.c
> +++ b/block.c
> @@ -3165,7 +3165,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
>      }
>
>      ret = bdrv_fill_options(&options, filename, &flags, &local_err);
> -    if (local_err) {
> +    if (ret < 0) {
>          goto fail;
>      }
>
> --
> 2.21.1
>
>

Re: [PATCH-for-5.0 v2 03/11] hw/i2c/pm_smbus: Remove dead assignment

[Alistair Francis]

On Sat, Mar 21, 2020 at 7:45 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Fix warning reported by Clang static code analyzer:
>
>     CC      hw/i2c/pm_smbus.o
>   hw/i2c/pm_smbus.c:187:17: warning: Value stored to 'ret' is never read
>                   ret = 0;
>                   ^     ~
>
> Reported-by: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/i2c/pm_smbus.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c
> index 36994ff585..4728540c37 100644
> --- a/hw/i2c/pm_smbus.c
> +++ b/hw/i2c/pm_smbus.c
> @@ -184,7 +184,6 @@ static void smb_transaction(PMSMBus *s)
>                  s->smb_stat |= STS_HOST_BUSY | STS_BYTE_DONE;
>                  s->smb_data[0] = s->smb_blkdata;
>                  s->smb_index = 0;
> -                ret = 0;
>              }
>              goto out;
>          }
> --
> 2.21.1
>
>

Re: [PATCH-for-5.0 v2 06/11] hw/isa/i82378: Remove dead assignment

[Alistair Francis]

On Sat, Mar 21, 2020 at 7:46 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Rename the unique variable assigned as 'pit' which better
> represents what it holds, to fix a warning reported by the
> Clang static code analyzer:
>
>     CC      hw/isa/i82378.o
>   hw/isa/i82378.c:108:5: warning: Value stored to 'isa' is never read
>       isa = isa_create_simple(isabus, "i82374");
>       ^     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Reported-by: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/isa/i82378.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
> index dcb6b479ea..d9e6c7fa00 100644
> --- a/hw/isa/i82378.c
> +++ b/hw/isa/i82378.c
> @@ -67,7 +67,7 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
>      I82378State *s = I82378(dev);
>      uint8_t *pci_conf;
>      ISABus *isabus;
> -    ISADevice *isa;
> +    ISADevice *pit;
>
>      pci_conf = pci->config;
>      pci_set_word(pci_conf + PCI_COMMAND,
> @@ -99,13 +99,13 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
>      isa_bus_irqs(isabus, s->i8259);
>
>      /* 1 82C54 (pit) */
> -    isa = i8254_pit_init(isabus, 0x40, 0, NULL);
> +    pit = i8254_pit_init(isabus, 0x40, 0, NULL);
>
>      /* speaker */
> -    pcspk_init(isabus, isa);
> +    pcspk_init(isabus, pit);
>
>      /* 2 82C37 (dma) */
> -    isa = isa_create_simple(isabus, "i82374");
> +    isa_create_simple(isabus, "i82374");
>  }
>
>  static void i82378_init(Object *obj)
> --
> 2.21.1
>
>

Re: [PATCH-for-5.0 v2 08/11] hw/timer/exynos4210_mct: Remove dead assignments

[Alistair Francis]

On Sat, Mar 21, 2020 at 7:50 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Fix warnings reported by Clang static code analyzer:
>
>   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: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/timer/exynos4210_mct.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/hw/timer/exynos4210_mct.c b/hw/timer/exynos4210_mct.c
> index 944120aea5..c0a25e71ec 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,7 +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);
>
> --
> 2.21.1
>
>

Re: [PATCH-for-5.0 v2 09/11] hw/timer/stm32f2xx_timer: Remove dead assignment

[Alistair Francis]

On Sat, Mar 21, 2020 at 7:52 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Fix warning reported by Clang static code analyzer:
>
>     CC      hw/timer/stm32f2xx_timer.o
>   hw/timer/stm32f2xx_timer.c:225:9: warning: Value stored to 'value' is never read
>           value = timer_val;
>           ^       ~~~~~~~~~
>
> Reported-by: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/timer/stm32f2xx_timer.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/hw/timer/stm32f2xx_timer.c b/hw/timer/stm32f2xx_timer.c
> index 06ec8a02c2..ba8694dcd3 100644
> --- a/hw/timer/stm32f2xx_timer.c
> +++ b/hw/timer/stm32f2xx_timer.c
> @@ -222,7 +222,6 @@ static void stm32f2xx_timer_write(void *opaque, hwaddr offset,
>      case TIM_PSC:
>          timer_val = stm32f2xx_ns_to_ticks(s, now) - s->tick_offset;
>          s->tim_psc = value & 0xFFFF;
> -        value = timer_val;
>          break;
>      case TIM_CNT:
>          timer_val = value;
> --
> 2.21.1
>
>

Re: [PATCH-for-5.0 v2 10/11] hw/timer/pxa2xx_timer: Add assertion to silent static analyzer warning

[Alistair Francis]

On Sat, Mar 21, 2020 at 7:50 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> pxa2xx_timer_tick4() takes an opaque pointer, then calls
> pxa2xx_timer_update4(), so the static analyzer can not
> verify that the 'n < 8':
>
>   425 static void pxa2xx_timer_tick4(void *opaque)
>   426 {
>   427     PXA2xxTimer4 *t = (PXA2xxTimer4 *) opaque;
>   428     PXA2xxTimerInfo *i = (PXA2xxTimerInfo *) t->tm.info;
>   429
>   430     pxa2xx_timer_tick(&t->tm);
>   433     if (t->control & (1 << 6))
>   434         pxa2xx_timer_update4(i, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), t->tm.num - 4);
>
>   135 static void pxa2xx_timer_update4(void *opaque, uint64_t now_qemu, int n)
>   136 {
>   137     PXA2xxTimerInfo *s = (PXA2xxTimerInfo *) opaque;
>   140     static const int counters[8] = { 0, 0, 0, 0, 4, 4, 6, 6 };
>   142
>   143     if (s->tm4[n].control & (1 << 7))
>   144         counter = n;
>   145     else
>   146         counter = counters[n];
>
> Add an assert() to give the static analyzer a hint, this fixes a
> warning reported by Clang static code analyzer:
>
>     CC      hw/timer/pxa2xx_timer.o
>   hw/timer/pxa2xx_timer.c:146:17: warning: Assigned value is garbage or undefined
>           counter = counters[n];
>                   ^ ~~~~~~~~~~~
>
> Reported-by: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/timer/pxa2xx_timer.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/timer/pxa2xx_timer.c b/hw/timer/pxa2xx_timer.c
> index cd172cc1e9..944c165889 100644
> --- a/hw/timer/pxa2xx_timer.c
> +++ b/hw/timer/pxa2xx_timer.c
> @@ -140,6 +140,7 @@ static void pxa2xx_timer_update4(void *opaque, uint64_t now_qemu, int n)
>      static const int counters[8] = { 0, 0, 0, 0, 4, 4, 6, 6 };
>      int counter;
>
> +    assert(n < ARRAY_SIZE(counters));
>      if (s->tm4[n].control & (1 << 7))
>          counter = n;
>      else
> --
> 2.21.1
>
>

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

[John Snow]

On 3/21/20 10:41 AM, 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;
>           ^     ~
> 
> Fixes: a9dd6604
> Reported-by: Clang Static Analyzer
> Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2: Fix the correct function (Aleksandar review)
> ---
>  hw/ide/sii3112.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
> index 06605d7af2..b2ff6dd6d9 100644
> --- a/hw/ide/sii3112.c
> +++ b/hw/ide/sii3112.c
> @@ -42,7 +42,7 @@ static uint64_t sii3112_reg_read(void *opaque, hwaddr addr,
>                                  unsigned int size)
>  {
>      SiI3112PCIState *d = opaque;
> -    uint64_t val = 0;
> +    uint64_t val;
>  
>      switch (addr) {
>      case 0x00:
> @@ -126,6 +126,7 @@ static uint64_t sii3112_reg_read(void *opaque, hwaddr addr,
>          break;
>      default:
>          val = 0;
> +        break;
>      }
>      trace_sii3112_read(size, addr, val);
>      return val;
> @@ -201,7 +202,7 @@ static void sii3112_reg_write(void *opaque, hwaddr addr,
>          d->regs[1].sien = (val >> 16) & 0x3eed;
>          break;
>      default:
> -        val = 0;
> +        break;
>      }
>  }
>  
> 

Acked-by: John Snow <jsnow@redhat.com>

RE: [PATCH-for-5.0 v2 00/11] misc: Trivial static code analyzer fixes

[Chenqun (kuhn)]

>-----Original Message-----
>From: Qemu-devel [mailto:qemu-devel-
>bounces+kuhn.chenqun=huawei.com@nongnu.org] On Behalf Of Laurent Vivier
>Sent: Monday, March 23, 2020 10:56 PM
>To: Philippe Mathieu-Daudé <philmd@redhat.com>; qemu-devel@nongnu.org
>Cc: Fam Zheng <fam@euphon.net>; Peter Maydell <peter.maydell@linaro.org>;
>Michael S. Tsirkin <mst@redhat.com>; Mark Cave-Ayland <mark.cave-
>ayland@ilande.co.uk>; qemu-block@nongnu.org; qemu-trivial@nongnu.org;
>Markus Armbruster <armbru@redhat.com>; Hervé Poussineau
><hpoussin@reactos.org>; Joel Stanley <joel@jms.id.au>; Michael Tokarev
><mjt@tls.msk.ru>; Alistair Francis <alistair@alistair23.me>; qemu-
>arm@nongnu.org; Cédric Le Goater <clg@kaod.org>; John Snow
><jsnow@redhat.com>; David Gibson <david@gibson.dropbear.id.au>; Kevin Wolf
><kwolf@redhat.com>; Andrew Jeffery <andrew@aj.id.au>; Max Reitz
><mreitz@redhat.com>; Igor Mitsyanko <i.mitsyanko@gmail.com>; qemu-
>ppc@nongnu.org; Paolo Bonzini <pbonzini@redhat.com>
>Subject: Re: [PATCH-for-5.0 v2 00/11] misc: Trivial static code analyzer fixes
>
>Le 23/03/2020 à 15:45, Philippe Mathieu-Daudé a écrit :
>> On 3/23/20 3:32 PM, Laurent Vivier wrote:
>>> Le 21/03/2020 à 15:40, Philippe Mathieu-Daudé a écrit :
>>>> Fix trivial warnings reported by the Clang static code analyzer.
>>>>
>>>> Since v1:
>>>> - Addressed Markus/Zoltan/Aleksandar review comments
>>>>
>>>> Philippe Mathieu-Daudé (11):
>>>>    block: Avoid dead assignment
>>>>    blockdev: Remove dead assignment
>>>>    hw/i2c/pm_smbus: Remove dead assignment
>>>>    hw/input/adb-kbd: Remove dead assignment
>>>>    hw/ide/sii3112: Remove dead assignment
>>>>    hw/isa/i82378: Remove dead assignment
>>>>    hw/gpio/aspeed_gpio: Remove dead assignment
>>>>    hw/timer/exynos4210_mct: Remove dead assignments

Same as this:
https://patchwork.kernel.org/patch/11415527/

>>>>    hw/timer/stm32f2xx_timer: Remove dead assignment
>>>>    hw/timer/pxa2xx_timer: Add assertion to silent static analyzer
>>>> warning
>>>>    hw/scsi/esp-pci: Remove dead assignment

Same as this:
https://patchwork.kernel.org/patch/11415529/

>>>>
>>>>   block.c                    | 2 +-
>>>>   blockdev.c                 | 2 +-
>>>>   hw/gpio/aspeed_gpio.c      | 2 +-
>>>>   hw/i2c/pm_smbus.c          | 1 -
>>>>   hw/ide/sii3112.c           | 5 +++--
>>>>   hw/input/adb-kbd.c         | 6 +-----
>>>>   hw/isa/i82378.c            | 8 ++++----
>>>>   hw/scsi/esp-pci.c          | 1 -
>>>>   hw/timer/exynos4210_mct.c  | 3 ---
>>>>   hw/timer/pxa2xx_timer.c    | 1 +
>>>>   hw/timer/stm32f2xx_timer.c | 1 -
>>>>   11 files changed, 12 insertions(+), 20 deletions(-)
>>>>
>>>
>>> I think your series covers cases already covered by:
>>>
>>> [PATCH v3 00/12] redundant code: Fix warnings reported by Clang
>>> static code analyzer
>>> https://patchew.org/QEMU/20200302130715.29440-1-kuhn.ch
>>
>> Unfortunately [for me...] I don't have v3 in my INBOX... *sigh* This
>> was 3 weeks ago. *sigh*.
>>
>> I can see the series in the archives:
>> https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg00219.html
>> But I can't find the outcome, was it queued in the trivial tree?
>> Any idea when this will be merged in the master tree?
>
>Some patches are already merged via trivial (1, 2 (should go by SCSI
>queue) 3, 5, 6, 7, 9, 11 (by USB queue), 12).
>
>But others needed R-b tags or new version. I didn't check which of your patches
>are already covered by this series.
>
Hi, Laurent and Philippe

      Yes, there are currently three patches( 4、8 、10) that are not merged.  
And, only patch4 and patch10  have the same points as this series.

I should update it again earlier.

Thanks.