On Sat, 23 Sep 2023, at 03:58, Philippe Mathieu-Daudé wrote:
> On 22/9/23 17:59, Cédric Le Goater wrote:
>> to fix warning :
>>
>> ../hw/i3c/aspeed_i3c.c: In function ‘aspeed_i3c_realize’:
>> ../hw/i3c/aspeed_i3c.c:1959:17: warning: declaration of ‘dev’ shadows a
>> parameter [-Wshadow=local]
>> 1959 | Object *dev = OBJECT(&s->devices[i]);
>> | ^~~
>> ../hw/i3c/aspeed_i3c.c:1942:45: note: shadowed declaration is here
>> 1942 | static void aspeed_i3c_realize(DeviceState *dev, Error **errp)
>> | ~~~~~~~~~~~~~^~~
>>
>> Signed-off-by: Cédric Le Goater <c...@kaod.org>
>> ---
>> hw/misc/aspeed_i3c.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/misc/aspeed_i3c.c b/hw/misc/aspeed_i3c.c
>> index f54f5da522b3..d1ff61767167 100644
>> --- a/hw/misc/aspeed_i3c.c
>> +++ b/hw/misc/aspeed_i3c.c
>> @@ -296,13 +296,13 @@ static void aspeed_i3c_realize(DeviceState *dev, Error
>> **errp)
>
> Alternatively:
>
> -- >8 --
>
> -static void aspeed_i3c_realize(DeviceState *dev, Error **errp)
> +static void aspeed_i3c_realize(DeviceState *ctrl, Error **errp)
> {
> int i;
> - AspeedI3CState *s = ASPEED_I3C(dev);
> - SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> + AspeedI3CState *s = ASPEED_I3C(ctrl);
> + SysBusDevice *sbd = SYS_BUS_DEVICE(ctrl);
Hmm, I feel like `dev` isn't an unreasonable or unusual name for the formal
parameter? Switching to `ctrl` isn't my immediate preference.
>
> ---
>
>> memory_region_add_subregion(&s->iomem_container, 0x0, &s->iomem);
>>
>> for (i = 0; i < ASPEED_I3C_NR_DEVICES; ++i) {
>> - Object *dev = OBJECT(&s->devices[i]);
>> + Object *i3c_dev = OBJECT(&s->devices[i]);
Maybe `s/i3c_dev/subdev`? I dunno. That bikeshed isn't gonna get painted on its
own.
Reviewed-by: Andrew Jeffery <and...@aj.id.au>
>>
>> - if (!object_property_set_uint(dev, "device-id", i, errp)) {
>> + if (!object_property_set_uint(i3c_dev, "device-id", i, errp)) {
>> return;
>> }
>>
>> - if (!sysbus_realize(SYS_BUS_DEVICE(dev), errp)) {
>> + if (!sysbus_realize(SYS_BUS_DEVICE(i3c_dev), errp)) {
>> return;
>> }
>>
>
> Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
