| Message ID | 20230922155924.1172019-3-clg@kaod.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | aspeed: Clean up local variable shadowing | expand |
On 22/9/23 17:59, Cédric Le Goater wrote: > 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(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
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.
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.
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); }
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(-)