Rename PCIDevice variable to avoid this warning :

  ../hw/ppc/spapr.c: In function ‘spapr_get_fw_dev_path’:
  ../hw/ppc/spapr.c:3217:20: warning: declaration of ‘pcidev’ shadows a previous local [-Wshadow=compatible-local]
   3217 |         PCIDevice *pcidev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
        |                    ^~~~~~
  ../hw/ppc/spapr.c:3147:16: note: shadowed declaration is here
   3147 |     PCIDevice *pcidev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
        |                ^~~~~~

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/spapr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 41ce7de77c14..8090fb0302df 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3214,8 +3214,8 @@ static char *spapr_get_fw_dev_path(FWPathProvider *p, BusState *bus,
 
     if (g_str_equal("pci-bridge", qdev_fw_name(dev))) {
         /* SLOF uses "pci" instead of "pci-bridge" for PCI bridges */
-        PCIDevice *pcidev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
-        return g_strdup_printf("pci@%x", PCI_SLOT(pcidev->devfn));
+        PCIDevice *pdev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
+        return g_strdup_printf("pci@%x", PCI_SLOT(pdev->devfn));
     }
 
     if (pcidev) {
-- 
2.41.0

On 9/18/23 20:28, Cédric Le Goater wrote:
> Rename PCIDevice variable to avoid this warning :
> 
>    ../hw/ppc/spapr.c: In function ‘spapr_get_fw_dev_path’:
>    ../hw/ppc/spapr.c:3217:20: warning: declaration of ‘pcidev’ shadows a previous local [-Wshadow=compatible-local]
>     3217 |         PCIDevice *pcidev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
>          |                    ^~~~~~
>    ../hw/ppc/spapr.c:3147:16: note: shadowed declaration is here
>     3147 |     PCIDevice *pcidev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
>          |                ^~~~~~
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/ppc/spapr.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 41ce7de77c14..8090fb0302df 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3214,8 +3214,8 @@ static char *spapr_get_fw_dev_path(FWPathProvider *p, BusState *bus,
>   
>       if (g_str_equal("pci-bridge", qdev_fw_name(dev))) {
>           /* SLOF uses "pci" instead of "pci-bridge" for PCI bridges */
> -        PCIDevice *pcidev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
> -        return g_strdup_printf("pci@%x", PCI_SLOT(pcidev->devfn));
> +        PCIDevice *pdev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
> +        return g_strdup_printf("pci@%x", PCI_SLOT(pdev->devfn));

Instead of renaming, can't we use pcidev itself without re-declaring ?

>       }
>   
>       if (pcidev) {

On 9/19/23 10:23, Harsh Prateek Bora wrote:
> 
> 
> On 9/18/23 20:28, Cédric Le Goater wrote:
>> Rename PCIDevice variable to avoid this warning :
>>
>>    ../hw/ppc/spapr.c: In function ‘spapr_get_fw_dev_path’:
>>    ../hw/ppc/spapr.c:3217:20: warning: declaration of ‘pcidev’ shadows a previous local [-Wshadow=compatible-local]
>>     3217 |         PCIDevice *pcidev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
>>          |                    ^~~~~~
>>    ../hw/ppc/spapr.c:3147:16: note: shadowed declaration is here
>>     3147 |     PCIDevice *pcidev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
>>          |                ^~~~~~
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   hw/ppc/spapr.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 41ce7de77c14..8090fb0302df 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -3214,8 +3214,8 @@ static char *spapr_get_fw_dev_path(FWPathProvider *p, BusState *bus,
>>       if (g_str_equal("pci-bridge", qdev_fw_name(dev))) {
>>           /* SLOF uses "pci" instead of "pci-bridge" for PCI bridges */
>> -        PCIDevice *pcidev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
>> -        return g_strdup_printf("pci@%x", PCI_SLOT(pcidev->devfn));
>> +        PCIDevice *pdev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
>> +        return g_strdup_printf("pci@%x", PCI_SLOT(pdev->devfn));
> 
> Instead of renaming, can't we use pcidev itself without re-declaring ?

I agree this could be done. But there is this test below :
  
> 
>>       }
>>       if (pcidev) {

which makes use of the same variable. I would rather keep the same
logic in the code or rewrite the routine completely with something
like:

     if (object_dynamic_cast(OBJECT(dev), TYPE_1 {
         /* return this */
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_2 {
         /* or that */
     } else if (object_dynamic_cast(OBJECT(dev), "some string"))
     ....
     } else {
         error_report("...");
     }

     return NULL;


This is beyond the issue that this patch is trying to fix.

Thanks,

C.

On 18/9/23 16:58, Cédric Le Goater wrote:
> Rename PCIDevice variable to avoid this warning :
> 
>    ../hw/ppc/spapr.c: In function ‘spapr_get_fw_dev_path’:
>    ../hw/ppc/spapr.c:3217:20: warning: declaration of ‘pcidev’ shadows a previous local [-Wshadow=compatible-local]
>     3217 |         PCIDevice *pcidev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
>          |                    ^~~~~~
>    ../hw/ppc/spapr.c:3147:16: note: shadowed declaration is here
>     3147 |     PCIDevice *pcidev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
>          |                ^~~~~~
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/ppc/spapr.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>