[PATCH 0/6] hw: Clean up global variables shadowing

[PATCH 0/6] hw: Clean up global variables shadowing

[Philippe Mathieu-Daudé]

Clean up global variables shadowing in hw/ in
order to be able to use -Wshadow with Clang.

Philippe Mathieu-Daudé (6):
  hw/core/cpu: Clean up global variable shadowing
  hw/loader: Clean up global variable shadowing in rom_add_file()
  hw/display/vga: Clean up global variable shadowing
  hw/acpi/pcihp: Clean up global variable shadowing in acpi_pcihp_init()
  hw/pci: Clean up global variable shadowing of address_space_io
    variable
  hw/s390x: Clean up global variable shadowing in
    quiesce_powerdown_req()

 hw/display/vga_int.h      |  2 +-
 include/hw/acpi/pcihp.h   |  2 +-
 include/hw/core/cpu.h     |  8 ++++----
 include/hw/loader.h       |  2 +-
 include/hw/pci/pci.h      |  9 +++------
 cpu-common.c              |  6 +++---
 hw/acpi/pcihp.c           |  5 ++---
 hw/core/loader.c          |  4 ++--
 hw/display/vga.c          |  6 +++---
 hw/pci/pci.c              | 25 +++++++++----------------
 hw/s390x/sclpquiesce.c    |  8 ++++----
 target/s390x/cpu_models.c |  2 +-
 12 files changed, 34 insertions(+), 45 deletions(-)

-- 
2.41.0

[PATCH 1/6] hw/core/cpu: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  hw/core/machine.c:1302:22: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
      const CPUArchId *cpus = possible_cpus->cpus;
                       ^
  hw/core/numa.c:69:17: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
      uint16List *cpus = NULL;
                  ^
  hw/acpi/aml-build.c:2005:20: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
      CPUArchIdList *cpus = ms->possible_cpus;
                     ^
  hw/core/machine-smp.c:77:14: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
      unsigned cpus    = config->has_cpus ? config->cpus : 0;
               ^
  include/hw/core/cpu.h:589:17: note: previous declaration is here
  extern CPUTailQ cpus;
                  ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/core/cpu.h     | 8 ++++----
 cpu-common.c              | 6 +++---
 target/s390x/cpu_models.c | 2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index e02bc5980f..d0dc0a1698 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -586,13 +586,13 @@ static inline CPUArchState *cpu_env(CPUState *cpu)
 }
 
 typedef QTAILQ_HEAD(CPUTailQ, CPUState) CPUTailQ;
-extern CPUTailQ cpus;
+extern CPUTailQ cpus_queue;
 
-#define first_cpu        QTAILQ_FIRST_RCU(&cpus)
+#define first_cpu        QTAILQ_FIRST_RCU(&cpus_queue)
 #define CPU_NEXT(cpu)    QTAILQ_NEXT_RCU(cpu, node)
-#define CPU_FOREACH(cpu) QTAILQ_FOREACH_RCU(cpu, &cpus, node)
+#define CPU_FOREACH(cpu) QTAILQ_FOREACH_RCU(cpu, &cpus_queue, node)
 #define CPU_FOREACH_SAFE(cpu, next_cpu) \
-    QTAILQ_FOREACH_SAFE_RCU(cpu, &cpus, node, next_cpu)
+    QTAILQ_FOREACH_SAFE_RCU(cpu, &cpus_queue, node, next_cpu)
 
 extern __thread CPUState *current_cpu;
 
diff --git a/cpu-common.c b/cpu-common.c
index 45c745ecf6..c81fd72d16 100644
--- a/cpu-common.c
+++ b/cpu-common.c
@@ -73,7 +73,7 @@ static int cpu_get_free_index(void)
     return max_cpu_index;
 }
 
-CPUTailQ cpus = QTAILQ_HEAD_INITIALIZER(cpus);
+CPUTailQ cpus_queue = QTAILQ_HEAD_INITIALIZER(cpus_queue);
 static unsigned int cpu_list_generation_id;
 
 unsigned int cpu_list_generation_id_get(void)
@@ -90,7 +90,7 @@ void cpu_list_add(CPUState *cpu)
     } else {
         assert(!cpu_index_auto_assigned);
     }
-    QTAILQ_INSERT_TAIL_RCU(&cpus, cpu, node);
+    QTAILQ_INSERT_TAIL_RCU(&cpus_queue, cpu, node);
     cpu_list_generation_id++;
 }
 
@@ -102,7 +102,7 @@ void cpu_list_remove(CPUState *cpu)
         return;
     }
 
-    QTAILQ_REMOVE_RCU(&cpus, cpu, node);
+    QTAILQ_REMOVE_RCU(&cpus_queue, cpu, node);
     cpu->cpu_index = UNASSIGNED_CPU_INDEX;
     cpu_list_generation_id++;
 }
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 98f14c09c2..b1e77b3a2b 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -756,7 +756,7 @@ void s390_set_qemu_cpu_model(uint16_t type, uint8_t gen, uint8_t ec_ga,
     const S390CPUDef *def = s390_find_cpu_def(type, gen, ec_ga, NULL);
 
     g_assert(def);
-    g_assert(QTAILQ_EMPTY_RCU(&cpus));
+    g_assert(QTAILQ_EMPTY_RCU(&cpus_queue));
 
     /* build the CPU model */
     s390_qemu_cpu_model.def = def;
-- 
2.41.0

[PATCH 2/6] hw/loader: Clean up global variable shadowing in rom_add_file()

[Philippe Mathieu-Daudé]

Fix:

  hw/core/loader.c:1073:27: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
                       bool option_rom, MemoryRegion *mr,
                            ^
  include/sysemu/sysemu.h:57:22: note: previous declaration is here
  extern QEMUOptionRom option_rom[MAX_OPTION_ROMS];
                       ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/loader.h | 2 +-
 hw/core/loader.c    | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/hw/loader.h b/include/hw/loader.h
index c4c14170ea..8685e27334 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -272,7 +272,7 @@ void pstrcpy_targphys(const char *name,
 
 ssize_t rom_add_file(const char *file, const char *fw_dir,
                      hwaddr addr, int32_t bootindex,
-                     bool option_rom, MemoryRegion *mr, AddressSpace *as);
+                     bool has_option_rom, MemoryRegion *mr, AddressSpace *as);
 MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len,
                            size_t max_len, hwaddr addr,
                            const char *fw_file_name,
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 4dd5a71fb7..7f0cbfb214 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1070,7 +1070,7 @@ static void *rom_set_mr(Rom *rom, Object *owner, const char *name, bool ro)
 
 ssize_t rom_add_file(const char *file, const char *fw_dir,
                      hwaddr addr, int32_t bootindex,
-                     bool option_rom, MemoryRegion *mr,
+                     bool has_option_rom, MemoryRegion *mr,
                      AddressSpace *as)
 {
     MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
@@ -1139,7 +1139,7 @@ ssize_t rom_add_file(const char *file, const char *fw_dir,
                  basename);
         snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name);
 
-        if ((!option_rom || mc->option_rom_has_mr) && mc->rom_file_has_mr) {
+        if ((!has_option_rom || mc->option_rom_has_mr) && mc->rom_file_has_mr) {
             data = rom_set_mr(rom, OBJECT(fw_cfg), devpath, true);
         } else {
             data = rom->data;
-- 
2.41.0

Re: [PATCH 2/6] hw/loader: Clean up global variable shadowing in rom_add_file()

[Ani Sinha]

> On 09-Oct-2023, at 3:17 PM, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> 
> Fix:
> 
>  hw/core/loader.c:1073:27: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>                       bool option_rom, MemoryRegion *mr,
>                            ^
>  include/sysemu/sysemu.h:57:22: note: previous declaration is here
>  extern QEMUOptionRom option_rom[MAX_OPTION_ROMS];
>                       ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> include/hw/loader.h | 2 +-
> hw/core/loader.c    | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/loader.h b/include/hw/loader.h
> index c4c14170ea..8685e27334 100644
> --- a/include/hw/loader.h
> +++ b/include/hw/loader.h
> @@ -272,7 +272,7 @@ void pstrcpy_targphys(const char *name,
> 
> ssize_t rom_add_file(const char *file, const char *fw_dir,
>                      hwaddr addr, int32_t bootindex,
> -                     bool option_rom, MemoryRegion *mr, AddressSpace *as);
> +                     bool has_option_rom, MemoryRegion *mr, AddressSpace *as);

Should this be “no_option_rom” instead looking at this conditional

if ((!option_rom || mc->option_rom_has_mr) && mc->rom_file_has_mr) {
            data = rom_set_mr(rom, OBJECT(fw_cfg), devpath, true);
        } else {
            data = rom->data;
        }



> MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len,
>                            size_t max_len, hwaddr addr,
>                            const char *fw_file_name,
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index 4dd5a71fb7..7f0cbfb214 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -1070,7 +1070,7 @@ static void *rom_set_mr(Rom *rom, Object *owner, const char *name, bool ro)
> 
> ssize_t rom_add_file(const char *file, const char *fw_dir,
>                      hwaddr addr, int32_t bootindex,
> -                     bool option_rom, MemoryRegion *mr,
> +                     bool has_option_rom, MemoryRegion *mr,
>                      AddressSpace *as)
> {
>     MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
> @@ -1139,7 +1139,7 @@ ssize_t rom_add_file(const char *file, const char *fw_dir,
>                  basename);
>         snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name);
> 
> -        if ((!option_rom || mc->option_rom_has_mr) && mc->rom_file_has_mr) {
> +        if ((!has_option_rom || mc->option_rom_has_mr) && mc->rom_file_has_mr) {
>             data = rom_set_mr(rom, OBJECT(fw_cfg), devpath, true);
>         } else {
>             data = rom->data;
> -- 
> 2.41.0
> 

[PATCH 3/6] hw/display/vga: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  hw/display/vga.c:2307:29: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
                MemoryRegion *address_space_io, bool init_vga_ports)
                            ^
  include/exec/address-spaces.h:35:21: note: previous declaration is here
  extern AddressSpace address_space_io;
                      ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/display/vga_int.h | 2 +-
 hw/display/vga.c     | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
index 7cf0d11201..94949d8a0c 100644
--- a/hw/display/vga_int.h
+++ b/hw/display/vga_int.h
@@ -157,7 +157,7 @@ static inline int c6_to_8(int v)
 }
 
 bool vga_common_init(VGACommonState *s, Object *obj, Error **errp);
-void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space,
+void vga_init(VGACommonState *s, Object *obj, MemoryRegion *io,
               MemoryRegion *address_space_io, bool init_vga_ports);
 MemoryRegion *vga_init_io(VGACommonState *s, Object *obj,
                           const MemoryRegionPortio **vga_ports,
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 37557c3442..bb4cd240ec 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -2304,7 +2304,7 @@ MemoryRegion *vga_init_io(VGACommonState *s, Object *obj,
 }
 
 void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space,
-              MemoryRegion *address_space_io, bool init_vga_ports)
+              MemoryRegion *io, bool init_vga_ports)
 {
     MemoryRegion *vga_io_memory;
     const MemoryRegionPortio *vga_ports, *vbe_ports;
@@ -2324,10 +2324,10 @@ void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space,
     if (init_vga_ports) {
         portio_list_init(&s->vga_port_list, obj, vga_ports, s, "vga");
         portio_list_set_flush_coalesced(&s->vga_port_list);
-        portio_list_add(&s->vga_port_list, address_space_io, 0x3b0);
+        portio_list_add(&s->vga_port_list, io, 0x3b0);
     }
     if (vbe_ports) {
         portio_list_init(&s->vbe_port_list, obj, vbe_ports, s, "vbe");
-        portio_list_add(&s->vbe_port_list, address_space_io, 0x1ce);
+        portio_list_add(&s->vbe_port_list, io, 0x1ce);
     }
 }
-- 
2.41.0

Re: [PATCH 3/6] hw/display/vga: Clean up global variable shadowing

[Ani Sinha]

> On 09-Oct-2023, at 3:17 PM, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> 
> Fix:
> 
>  hw/display/vga.c:2307:29: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>                MemoryRegion *address_space_io, bool init_vga_ports)
>                            ^
>  include/exec/address-spaces.h:35:21: note: previous declaration is here
>  extern AddressSpace address_space_io;
>                      ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/display/vga_int.h | 2 +-
> hw/display/vga.c     | 6 +++---
> 2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
> index 7cf0d11201..94949d8a0c 100644
> --- a/hw/display/vga_int.h
> +++ b/hw/display/vga_int.h
> @@ -157,7 +157,7 @@ static inline int c6_to_8(int v)
> }
> 
> bool vga_common_init(VGACommonState *s, Object *obj, Error **errp);
> -void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space,
> +void vga_init(VGACommonState *s, Object *obj, MemoryRegion *io,
>               MemoryRegion *address_space_io, bool init_vga_ports);

Ok here you are replacing *address_space with *io but …

> MemoryRegion *vga_init_io(VGACommonState *s, Object *obj,
>                           const MemoryRegionPortio **vga_ports,
> diff --git a/hw/display/vga.c b/hw/display/vga.c
> index 37557c3442..bb4cd240ec 100644
> --- a/hw/display/vga.c
> +++ b/hw/display/vga.c
> @@ -2304,7 +2304,7 @@ MemoryRegion *vga_init_io(VGACommonState *s, Object *obj,
> }
> 
> void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space,
> -              MemoryRegion *address_space_io, bool init_vga_ports)
> +              MemoryRegion *io, bool init_vga_ports)

Here you replace *address_space_io with *io? This does not look right.

> {
>     MemoryRegion *vga_io_memory;
>     const MemoryRegionPortio *vga_ports, *vbe_ports;
> @@ -2324,10 +2324,10 @@ void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space,
>     if (init_vga_ports) {
>         portio_list_init(&s->vga_port_list, obj, vga_ports, s, "vga");
>         portio_list_set_flush_coalesced(&s->vga_port_list);
> -        portio_list_add(&s->vga_port_list, address_space_io, 0x3b0);
> +        portio_list_add(&s->vga_port_list, io, 0x3b0);
>     }
>     if (vbe_ports) {
>         portio_list_init(&s->vbe_port_list, obj, vbe_ports, s, "vbe");
> -        portio_list_add(&s->vbe_port_list, address_space_io, 0x1ce);
> +        portio_list_add(&s->vbe_port_list, io, 0x1ce);
>     }
> }
> -- 
> 2.41.0
> 

Re: [PATCH 3/6] hw/display/vga: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

On 9/10/23 16:11, Ani Sinha wrote:
> 
> 
>> On 09-Oct-2023, at 3:17 PM, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Fix:
>>
>>   hw/display/vga.c:2307:29: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>>                 MemoryRegion *address_space_io, bool init_vga_ports)
>>                             ^
>>   include/exec/address-spaces.h:35:21: note: previous declaration is here
>>   extern AddressSpace address_space_io;
>>                       ^
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/display/vga_int.h | 2 +-
>> hw/display/vga.c     | 6 +++---
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
>> index 7cf0d11201..94949d8a0c 100644
>> --- a/hw/display/vga_int.h
>> +++ b/hw/display/vga_int.h
>> @@ -157,7 +157,7 @@ static inline int c6_to_8(int v)
>> }
>>
>> bool vga_common_init(VGACommonState *s, Object *obj, Error **errp);
>> -void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space,
>> +void vga_init(VGACommonState *s, Object *obj, MemoryRegion *io,
>>                MemoryRegion *address_space_io, bool init_vga_ports);
> 
> Ok here you are replacing *address_space with *io but …
> 
>> MemoryRegion *vga_init_io(VGACommonState *s, Object *obj,
>>                            const MemoryRegionPortio **vga_ports,
>> diff --git a/hw/display/vga.c b/hw/display/vga.c
>> index 37557c3442..bb4cd240ec 100644
>> --- a/hw/display/vga.c
>> +++ b/hw/display/vga.c
>> @@ -2304,7 +2304,7 @@ MemoryRegion *vga_init_io(VGACommonState *s, Object *obj,
>> }
>>
>> void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space,
>> -              MemoryRegion *address_space_io, bool init_vga_ports)
>> +              MemoryRegion *io, bool init_vga_ports)
> 
> Here you replace *address_space_io with *io? This does not look right.

Yeah this is totally wrong, sorry...

[PATCH 4/6] hw/acpi/pcihp: Clean up global variable shadowing in acpi_pcihp_init()

[Philippe Mathieu-Daudé]

Fix:

  hw/acpi/pcihp.c:499:36: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
                       MemoryRegion *address_space_io,
                                     ^
  include/exec/address-spaces.h:35:21: note: previous declaration is here
  extern AddressSpace address_space_io;
                      ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/acpi/pcihp.h | 2 +-
 hw/acpi/pcihp.c         | 5 ++---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
index ef59810c17..ac21a95913 100644
--- a/include/hw/acpi/pcihp.h
+++ b/include/hw/acpi/pcihp.h
@@ -56,7 +56,7 @@ typedef struct AcpiPciHpState {
 } AcpiPciHpState;
 
 void acpi_pcihp_init(Object *owner, AcpiPciHpState *, PCIBus *root,
-                     MemoryRegion *address_space_io, uint16_t io_base);
+                     MemoryRegion *io, uint16_t io_base);
 
 bool acpi_pcihp_is_hotpluggbale_bus(AcpiPciHpState *s, BusState *bus);
 void acpi_pcihp_device_pre_plug_cb(HotplugHandler *hotplug_dev,
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index cdd6f775a1..4f75c873e2 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -496,8 +496,7 @@ static const MemoryRegionOps acpi_pcihp_io_ops = {
 };
 
 void acpi_pcihp_init(Object *owner, AcpiPciHpState *s, PCIBus *root_bus,
-                     MemoryRegion *address_space_io,
-                     uint16_t io_base)
+                     MemoryRegion *io, uint16_t io_base)
 {
     s->io_len = ACPI_PCIHP_SIZE;
     s->io_base = io_base;
@@ -506,7 +505,7 @@ void acpi_pcihp_init(Object *owner, AcpiPciHpState *s, PCIBus *root_bus,
 
     memory_region_init_io(&s->io, owner, &acpi_pcihp_io_ops, s,
                           "acpi-pci-hotplug", s->io_len);
-    memory_region_add_subregion(address_space_io, s->io_base, &s->io);
+    memory_region_add_subregion(io, s->io_base, &s->io);
 
     object_property_add_uint16_ptr(owner, ACPI_PCIHP_IO_BASE_PROP, &s->io_base,
                                    OBJ_PROP_FLAG_READ);
-- 
2.41.0

Re: [PATCH 4/6] hw/acpi/pcihp: Clean up global variable shadowing in acpi_pcihp_init()

[Ani Sinha]

> On 09-Oct-2023, at 3:17 PM, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> 
> Fix:
> 
>  hw/acpi/pcihp.c:499:36: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>                       MemoryRegion *address_space_io,
>                                     ^
>  include/exec/address-spaces.h:35:21: note: previous declaration is here
>  extern AddressSpace address_space_io;
>                      ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Ani Sinha <anisinha@redhat.com>


> ---
> include/hw/acpi/pcihp.h | 2 +-
> hw/acpi/pcihp.c         | 5 ++---
> 2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
> index ef59810c17..ac21a95913 100644
> --- a/include/hw/acpi/pcihp.h
> +++ b/include/hw/acpi/pcihp.h
> @@ -56,7 +56,7 @@ typedef struct AcpiPciHpState {
> } AcpiPciHpState;
> 
> void acpi_pcihp_init(Object *owner, AcpiPciHpState *, PCIBus *root,
> -                     MemoryRegion *address_space_io, uint16_t io_base);
> +                     MemoryRegion *io, uint16_t io_base);
> 
> bool acpi_pcihp_is_hotpluggbale_bus(AcpiPciHpState *s, BusState *bus);
> void acpi_pcihp_device_pre_plug_cb(HotplugHandler *hotplug_dev,
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index cdd6f775a1..4f75c873e2 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -496,8 +496,7 @@ static const MemoryRegionOps acpi_pcihp_io_ops = {
> };
> 
> void acpi_pcihp_init(Object *owner, AcpiPciHpState *s, PCIBus *root_bus,
> -                     MemoryRegion *address_space_io,
> -                     uint16_t io_base)
> +                     MemoryRegion *io, uint16_t io_base)
> {
>     s->io_len = ACPI_PCIHP_SIZE;
>     s->io_base = io_base;
> @@ -506,7 +505,7 @@ void acpi_pcihp_init(Object *owner, AcpiPciHpState *s, PCIBus *root_bus,
> 
>     memory_region_init_io(&s->io, owner, &acpi_pcihp_io_ops, s,
>                           "acpi-pci-hotplug", s->io_len);
> -    memory_region_add_subregion(address_space_io, s->io_base, &s->io);
> +    memory_region_add_subregion(io, s->io_base, &s->io);
> 
>     object_property_add_uint16_ptr(owner, ACPI_PCIHP_IO_BASE_PROP, &s->io_base,
>                                    OBJ_PROP_FLAG_READ);
> -- 
> 2.41.0
> 

Re: [PATCH 4/6] hw/acpi/pcihp: Clean up global variable shadowing in acpi_pcihp_init()

[Michael S. Tsirkin]

On Mon, Oct 09, 2023 at 11:47:44AM +0200, Philippe Mathieu-Daudé wrote:
> Fix:
> 
>   hw/acpi/pcihp.c:499:36: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>                        MemoryRegion *address_space_io,
>                                      ^
>   include/exec/address-spaces.h:35:21: note: previous declaration is here
>   extern AddressSpace address_space_io;
>                       ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  include/hw/acpi/pcihp.h | 2 +-
>  hw/acpi/pcihp.c         | 5 ++---
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
> index ef59810c17..ac21a95913 100644
> --- a/include/hw/acpi/pcihp.h
> +++ b/include/hw/acpi/pcihp.h
> @@ -56,7 +56,7 @@ typedef struct AcpiPciHpState {
>  } AcpiPciHpState;
>  
>  void acpi_pcihp_init(Object *owner, AcpiPciHpState *, PCIBus *root,
> -                     MemoryRegion *address_space_io, uint16_t io_base);
> +                     MemoryRegion *io, uint16_t io_base);
>  
>  bool acpi_pcihp_is_hotpluggbale_bus(AcpiPciHpState *s, BusState *bus);
>  void acpi_pcihp_device_pre_plug_cb(HotplugHandler *hotplug_dev,
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index cdd6f775a1..4f75c873e2 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -496,8 +496,7 @@ static const MemoryRegionOps acpi_pcihp_io_ops = {
>  };
>  
>  void acpi_pcihp_init(Object *owner, AcpiPciHpState *s, PCIBus *root_bus,
> -                     MemoryRegion *address_space_io,
> -                     uint16_t io_base)
> +                     MemoryRegion *io, uint16_t io_base)
>  {
>      s->io_len = ACPI_PCIHP_SIZE;
>      s->io_base = io_base;
> @@ -506,7 +505,7 @@ void acpi_pcihp_init(Object *owner, AcpiPciHpState *s, PCIBus *root_bus,
>  
>      memory_region_init_io(&s->io, owner, &acpi_pcihp_io_ops, s,
>                            "acpi-pci-hotplug", s->io_len);
> -    memory_region_add_subregion(address_space_io, s->io_base, &s->io);
> +    memory_region_add_subregion(io, s->io_base, &s->io);
>  
>      object_property_add_uint16_ptr(owner, ACPI_PCIHP_IO_BASE_PROP, &s->io_base,
>                                     OBJ_PROP_FLAG_READ);
> -- 
> 2.41.0

[PATCH 5/6] hw/pci: Clean up global variable shadowing of address_space_io variable

[Philippe Mathieu-Daudé]

Fix:

  hw/pci/pci.c:504:54: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
                                         MemoryRegion *address_space_io,
                                                       ^
  hw/pci/pci.c:533:38: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
                         MemoryRegion *address_space_io,
                                       ^
  hw/pci/pci.c:543:40: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
                           MemoryRegion *address_space_io,
                                         ^
  hw/pci/pci.c:590:45: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
                                MemoryRegion *address_space_io,
                                              ^
  include/exec/address-spaces.h:35:21: note: previous declaration is here
  extern AddressSpace address_space_io;
                      ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/pci/pci.h |  9 +++------
 hw/pci/pci.c         | 25 +++++++++----------------
 2 files changed, 12 insertions(+), 22 deletions(-)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index b70a0b95ff..ea5aff118b 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -279,12 +279,10 @@ bool pci_bus_is_express(const PCIBus *bus);
 
 void pci_root_bus_init(PCIBus *bus, size_t bus_size, DeviceState *parent,
                        const char *name,
-                       MemoryRegion *address_space_mem,
-                       MemoryRegion *address_space_io,
+                       MemoryRegion *mem, MemoryRegion *io,
                        uint8_t devfn_min, const char *typename);
 PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
-                         MemoryRegion *address_space_mem,
-                         MemoryRegion *address_space_io,
+                         MemoryRegion *mem, MemoryRegion *io,
                          uint8_t devfn_min, const char *typename);
 void pci_root_bus_cleanup(PCIBus *bus);
 void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq,
@@ -304,8 +302,7 @@ int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
 PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
                               pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
                               void *irq_opaque,
-                              MemoryRegion *address_space_mem,
-                              MemoryRegion *address_space_io,
+                              MemoryRegion *mem, MemoryRegion *io,
                               uint8_t devfn_min, int nirq,
                               const char *typename);
 void pci_unregister_root_bus(PCIBus *bus);
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index b0d21bf43a..7d09e1a39d 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -500,15 +500,14 @@ bool pci_bus_bypass_iommu(PCIBus *bus)
 }
 
 static void pci_root_bus_internal_init(PCIBus *bus, DeviceState *parent,
-                                       MemoryRegion *address_space_mem,
-                                       MemoryRegion *address_space_io,
+                                       MemoryRegion *mem, MemoryRegion *io,
                                        uint8_t devfn_min)
 {
     assert(PCI_FUNC(devfn_min) == 0);
     bus->devfn_min = devfn_min;
     bus->slot_reserved_mask = 0x0;
-    bus->address_space_mem = address_space_mem;
-    bus->address_space_io = address_space_io;
+    bus->address_space_mem = mem;
+    bus->address_space_io = io;
     bus->flags |= PCI_BUS_IS_ROOT;
 
     /* host bridge */
@@ -529,25 +528,21 @@ bool pci_bus_is_express(const PCIBus *bus)
 
 void pci_root_bus_init(PCIBus *bus, size_t bus_size, DeviceState *parent,
                        const char *name,
-                       MemoryRegion *address_space_mem,
-                       MemoryRegion *address_space_io,
+                       MemoryRegion *mem, MemoryRegion *io,
                        uint8_t devfn_min, const char *typename)
 {
     qbus_init(bus, bus_size, typename, parent, name);
-    pci_root_bus_internal_init(bus, parent, address_space_mem,
-                               address_space_io, devfn_min);
+    pci_root_bus_internal_init(bus, parent, mem, io, devfn_min);
 }
 
 PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
-                         MemoryRegion *address_space_mem,
-                         MemoryRegion *address_space_io,
+                         MemoryRegion *mem, MemoryRegion *io,
                          uint8_t devfn_min, const char *typename)
 {
     PCIBus *bus;
 
     bus = PCI_BUS(qbus_new(typename, parent, name));
-    pci_root_bus_internal_init(bus, parent, address_space_mem,
-                               address_space_io, devfn_min);
+    pci_root_bus_internal_init(bus, parent, mem, io, devfn_min);
     return bus;
 }
 
@@ -586,15 +581,13 @@ void pci_bus_irqs_cleanup(PCIBus *bus)
 PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
                               pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
                               void *irq_opaque,
-                              MemoryRegion *address_space_mem,
-                              MemoryRegion *address_space_io,
+                              MemoryRegion *mem, MemoryRegion *io,
                               uint8_t devfn_min, int nirq,
                               const char *typename)
 {
     PCIBus *bus;
 
-    bus = pci_root_bus_new(parent, name, address_space_mem,
-                           address_space_io, devfn_min, typename);
+    bus = pci_root_bus_new(parent, name, mem, io, devfn_min, typename);
     pci_bus_irqs(bus, set_irq, irq_opaque, nirq);
     pci_bus_map_irqs(bus, map_irq);
     return bus;
-- 
2.41.0

Re: [PATCH 5/6] hw/pci: Clean up global variable shadowing of address_space_io variable

[Michael S. Tsirkin]

On Mon, Oct 09, 2023 at 11:47:45AM +0200, Philippe Mathieu-Daudé wrote:
> Fix:
> 
>   hw/pci/pci.c:504:54: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>                                          MemoryRegion *address_space_io,
>                                                        ^
>   hw/pci/pci.c:533:38: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>                          MemoryRegion *address_space_io,
>                                        ^
>   hw/pci/pci.c:543:40: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>                            MemoryRegion *address_space_io,
>                                          ^
>   hw/pci/pci.c:590:45: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>                                 MemoryRegion *address_space_io,
>                                               ^
>   include/exec/address-spaces.h:35:21: note: previous declaration is here
>   extern AddressSpace address_space_io;
>                       ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  include/hw/pci/pci.h |  9 +++------
>  hw/pci/pci.c         | 25 +++++++++----------------
>  2 files changed, 12 insertions(+), 22 deletions(-)
> 
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index b70a0b95ff..ea5aff118b 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -279,12 +279,10 @@ bool pci_bus_is_express(const PCIBus *bus);
>  
>  void pci_root_bus_init(PCIBus *bus, size_t bus_size, DeviceState *parent,
>                         const char *name,
> -                       MemoryRegion *address_space_mem,
> -                       MemoryRegion *address_space_io,
> +                       MemoryRegion *mem, MemoryRegion *io,
>                         uint8_t devfn_min, const char *typename);
>  PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
> -                         MemoryRegion *address_space_mem,
> -                         MemoryRegion *address_space_io,
> +                         MemoryRegion *mem, MemoryRegion *io,
>                           uint8_t devfn_min, const char *typename);
>  void pci_root_bus_cleanup(PCIBus *bus);
>  void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq,
> @@ -304,8 +302,7 @@ int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
>  PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
>                                pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>                                void *irq_opaque,
> -                              MemoryRegion *address_space_mem,
> -                              MemoryRegion *address_space_io,
> +                              MemoryRegion *mem, MemoryRegion *io,
>                                uint8_t devfn_min, int nirq,
>                                const char *typename);
>  void pci_unregister_root_bus(PCIBus *bus);
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index b0d21bf43a..7d09e1a39d 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -500,15 +500,14 @@ bool pci_bus_bypass_iommu(PCIBus *bus)
>  }
>  
>  static void pci_root_bus_internal_init(PCIBus *bus, DeviceState *parent,
> -                                       MemoryRegion *address_space_mem,
> -                                       MemoryRegion *address_space_io,
> +                                       MemoryRegion *mem, MemoryRegion *io,
>                                         uint8_t devfn_min)
>  {
>      assert(PCI_FUNC(devfn_min) == 0);
>      bus->devfn_min = devfn_min;
>      bus->slot_reserved_mask = 0x0;
> -    bus->address_space_mem = address_space_mem;
> -    bus->address_space_io = address_space_io;
> +    bus->address_space_mem = mem;
> +    bus->address_space_io = io;
>      bus->flags |= PCI_BUS_IS_ROOT;
>  
>      /* host bridge */
> @@ -529,25 +528,21 @@ bool pci_bus_is_express(const PCIBus *bus)
>  
>  void pci_root_bus_init(PCIBus *bus, size_t bus_size, DeviceState *parent,
>                         const char *name,
> -                       MemoryRegion *address_space_mem,
> -                       MemoryRegion *address_space_io,
> +                       MemoryRegion *mem, MemoryRegion *io,
>                         uint8_t devfn_min, const char *typename)
>  {
>      qbus_init(bus, bus_size, typename, parent, name);
> -    pci_root_bus_internal_init(bus, parent, address_space_mem,
> -                               address_space_io, devfn_min);
> +    pci_root_bus_internal_init(bus, parent, mem, io, devfn_min);
>  }
>  
>  PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
> -                         MemoryRegion *address_space_mem,
> -                         MemoryRegion *address_space_io,
> +                         MemoryRegion *mem, MemoryRegion *io,
>                           uint8_t devfn_min, const char *typename)
>  {
>      PCIBus *bus;
>  
>      bus = PCI_BUS(qbus_new(typename, parent, name));
> -    pci_root_bus_internal_init(bus, parent, address_space_mem,
> -                               address_space_io, devfn_min);
> +    pci_root_bus_internal_init(bus, parent, mem, io, devfn_min);
>      return bus;
>  }
>  
> @@ -586,15 +581,13 @@ void pci_bus_irqs_cleanup(PCIBus *bus)
>  PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
>                                pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>                                void *irq_opaque,
> -                              MemoryRegion *address_space_mem,
> -                              MemoryRegion *address_space_io,
> +                              MemoryRegion *mem, MemoryRegion *io,
>                                uint8_t devfn_min, int nirq,
>                                const char *typename)
>  {
>      PCIBus *bus;
>  
> -    bus = pci_root_bus_new(parent, name, address_space_mem,
> -                           address_space_io, devfn_min, typename);
> +    bus = pci_root_bus_new(parent, name, mem, io, devfn_min, typename);
>      pci_bus_irqs(bus, set_irq, irq_opaque, nirq);
>      pci_bus_map_irqs(bus, map_irq);
>      return bus;
> -- 
> 2.41.0

[PATCH 6/6] hw/s390x: Clean up global variable shadowing in quiesce_powerdown_req()

[Philippe Mathieu-Daudé]

Fix:

  hw/s390x/sclpquiesce.c:90:22: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
      QuiesceNotifier *qn = container_of(n, QuiesceNotifier, notifier);
                       ^
  hw/s390x/sclpquiesce.c:86:3: note: previous declaration is here
  } qn;
    ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/s390x/sclpquiesce.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/s390x/sclpquiesce.c b/hw/s390x/sclpquiesce.c
index ce07b16884..a641089929 100644
--- a/hw/s390x/sclpquiesce.c
+++ b/hw/s390x/sclpquiesce.c
@@ -78,12 +78,10 @@ static const VMStateDescription vmstate_sclpquiesce = {
      }
 };
 
-typedef struct QuiesceNotifier QuiesceNotifier;
-
-static struct QuiesceNotifier {
+typedef struct QuiesceNotifier {
     Notifier notifier;
     SCLPEvent *event;
-} qn;
+} QuiesceNotifier;
 
 static void quiesce_powerdown_req(Notifier *n, void *opaque)
 {
@@ -97,6 +95,8 @@ static void quiesce_powerdown_req(Notifier *n, void *opaque)
 
 static int quiesce_init(SCLPEvent *event)
 {
+    static QuiesceNotifier qn;
+
     qn.notifier.notify = quiesce_powerdown_req;
     qn.event = event;
 
-- 
2.41.0

Re: [PATCH 6/6] hw/s390x: Clean up global variable shadowing in quiesce_powerdown_req()

[Thomas Huth]

On 09/10/2023 11.47, Philippe Mathieu-Daudé wrote:
> Fix:
> 
>    hw/s390x/sclpquiesce.c:90:22: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>        QuiesceNotifier *qn = container_of(n, QuiesceNotifier, notifier);
>                         ^
>    hw/s390x/sclpquiesce.c:86:3: note: previous declaration is here
>    } qn;
>      ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/s390x/sclpquiesce.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/s390x/sclpquiesce.c b/hw/s390x/sclpquiesce.c
> index ce07b16884..a641089929 100644
> --- a/hw/s390x/sclpquiesce.c
> +++ b/hw/s390x/sclpquiesce.c
> @@ -78,12 +78,10 @@ static const VMStateDescription vmstate_sclpquiesce = {
>        }
>   };
>   
> -typedef struct QuiesceNotifier QuiesceNotifier;
> -
> -static struct QuiesceNotifier {
> +typedef struct QuiesceNotifier {
>       Notifier notifier;
>       SCLPEvent *event;
> -} qn;
> +} QuiesceNotifier;
>   
>   static void quiesce_powerdown_req(Notifier *n, void *opaque)
>   {
> @@ -97,6 +95,8 @@ static void quiesce_powerdown_req(Notifier *n, void *opaque)
>   
>   static int quiesce_init(SCLPEvent *event)
>   {
> +    static QuiesceNotifier qn;
> +
>       qn.notifier.notify = quiesce_powerdown_req;
>       qn.event = event;

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 6/6] hw/s390x: Clean up global variable shadowing in quiesce_powerdown_req()

[David Hildenbrand]

On 09.10.23 11:47, Philippe Mathieu-Daudé wrote:
> Fix:
> 
>    hw/s390x/sclpquiesce.c:90:22: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>        QuiesceNotifier *qn = container_of(n, QuiesceNotifier, notifier);
>                         ^
>    hw/s390x/sclpquiesce.c:86:3: note: previous declaration is here
>    } qn;
>      ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/s390x/sclpquiesce.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/s390x/sclpquiesce.c b/hw/s390x/sclpquiesce.c
> index ce07b16884..a641089929 100644
> --- a/hw/s390x/sclpquiesce.c
> +++ b/hw/s390x/sclpquiesce.c
> @@ -78,12 +78,10 @@ static const VMStateDescription vmstate_sclpquiesce = {
>        }
>   };
>   
> -typedef struct QuiesceNotifier QuiesceNotifier;
> -
> -static struct QuiesceNotifier {
> +typedef struct QuiesceNotifier {
>       Notifier notifier;
>       SCLPEvent *event;
> -} qn;
> +} QuiesceNotifier;
>   
>   static void quiesce_powerdown_req(Notifier *n, void *opaque)
>   {
> @@ -97,6 +95,8 @@ static void quiesce_powerdown_req(Notifier *n, void *opaque)
>   
>   static int quiesce_init(SCLPEvent *event)
>   {
> +    static QuiesceNotifier qn;
> +
>       qn.notifier.notify = quiesce_powerdown_req;
>       qn.event = event;
>   

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH 6/6] hw/s390x: Clean up global variable shadowing in quiesce_powerdown_req()

[Eric Farman]

On Mon, 2023-10-09 at 11:47 +0200, Philippe Mathieu-Daudé wrote:
> Fix:
> 
>   hw/s390x/sclpquiesce.c:90:22: error: declaration shadows a variable
> in the global scope [-Werror,-Wshadow]
>       QuiesceNotifier *qn = container_of(n, QuiesceNotifier,
> notifier);
>                        ^
>   hw/s390x/sclpquiesce.c:86:3: note: previous declaration is here
>   } qn;
>     ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/s390x/sclpquiesce.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Eric Farman <farman@linux.ibm.com>