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 <clg@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 <clg@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-philmd@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.