| Message ID | 20230918145850.241074-6-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
| Series | ppc: Clean up local variable shadowing | expand |
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>
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.
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) {
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(-)