Re: [PATCH 2/4] aspeed: Clean up local variable shadowing

[Cédric Le Goater]

On 9/23/23 08:34, Markus Armbruster wrote:

Cédric Le Goater <c...@kaod.org> writes:

Remove superfluous local 'irq' variables and use the one define at the
top of the routine. This fixes warnings in aspeed_soc_ast2600_realize()
such as :

   ../hw/arm/aspeed_ast2600.c: In function ‘aspeed_soc_ast2600_realize’:
   ../hw/arm/aspeed_ast2600.c:420:18: warning: declaration of ‘irq’ shadows a 
previous local [-Wshadow=compatible-local]
     420 |         qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
         |                  ^~~
   ../hw/arm/aspeed_ast2600.c:312:14: note: shadowed declaration is here
     312 |     qemu_irq irq;
         |              ^~~

Signed-off-by: Cédric Le Goater <c...@kaod.org>
---
  hw/arm/aspeed_ast2600.c | 10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index a8b3a8065a11..e122e1c32d42 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -388,7 +388,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
      aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->timerctrl), 0,
                      sc->memmap[ASPEED_DEV_TIMER1]);
      for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) {
-        qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
+        irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
          sysbus_connect_irq(SYS_BUS_DEVICE(&s->timerctrl), i, irq);
      }

@@ -413,8 +413,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)

      }
      aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->i2c), 0, 
sc->memmap[ASPEED_DEV_I2C]);
      for (i = 0; i < ASPEED_I2C_GET_CLASS(&s->i2c)->num_busses; i++) {
-        qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore),
-                                        sc->irqmap[ASPEED_DEV_I2C] + i);
+        irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore),
+                               sc->irqmap[ASPEED_DEV_I2C] + i);
          /* The AST2600 I2C controller has one IRQ per bus. */
          sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c.busses[i]), 0, irq);
      }
@@ -611,8 +611,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
      }
      aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->i3c), 0, 
sc->memmap[ASPEED_DEV_I3C]);
      for (i = 0; i < ASPEED_I3C_NR_DEVICES; i++) {
-        qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore),
-                                        sc->irqmap[ASPEED_DEV_I3C] + i);
+        irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore),
+                               sc->irqmap[ASPEED_DEV_I3C] + i);
          /* The AST2600 I3C controller has one IRQ per bus. */
          sysbus_connect_irq(SYS_BUS_DEVICE(&s->i3c.devices[i]), 0, irq);
      }

Clashes with Philippe's

     [PATCH v2 10/22] hw/arm/aspeed: Clean up local variable shadowing
     Message-ID: <20230904161235.84651-11-phi...@linaro.org>

The difference is a matter of taste: one @irq in function scope vs. four
in nested scopes.  I'd prefer the former, i.e. this patch, but
maintainers' preference matter more than mine.  If you want me to merge
together with other shadowing patches, tell me your preference, if any.

You are the one gathering patches. Please choose. I don't mind.

Thanks,

C.