Re: [PATCH 3/4] aspeed/i3c: Rename variable shadowing a local

[Andrew Jeffery]

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 
>> ---
>>   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 

>>   
>> -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é 

Re: [PATCH 3/4] aspeed/i3c: Rename variable shadowing a local

[Philippe Mathieu-Daudé]

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 
---
  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);

---


  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]);
  
-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é 

[PATCH 3/4] aspeed/i3c: Rename variable shadowing a local

[Cédric Le Goater]

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 
---
 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)
 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]);
 
-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;
 }
 
-- 
2.41.0