[PATCH] hw/acpi: changes towards enabling -Wshadow=local

[PATCH] hw/acpi: changes towards enabling -Wshadow=local

[Ani Sinha]

Code changes in acpi that addresses all compiler complaints coming from enabling
-Wshadow flags. Enabling -Wshadow catches cases of local variables shadowing
other local variables or parameters. These makes the code confusing and/or adds
bugs that are difficult to catch.

The code is tested to build with and without the flag turned on.

CC: Markus Armbruster <armbru@redhat.com>
CC: Philippe Mathieu-Daude <philmd@linaro.org>
CC: mst@redhat.com
CC: imammedo@redhat.com
Message-Id: <87r0mqlf9x.fsf@pond.sub.org>
Signed-off-by: Ani Sinha <anisinha@redhat.com>
---
 hw/acpi/cpu_hotplug.c | 25 +++++++++++++------------
 hw/i386/acpi-build.c  | 24 ++++++++++++------------
 hw/smbios/smbios.c    | 37 +++++++++++++++++++------------------
 3 files changed, 44 insertions(+), 42 deletions(-)

diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index ff14c3f410..634bbecb31 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -265,26 +265,27 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
 
     /* build Processor object for each processor */
     for (i = 0; i < apic_ids->len; i++) {
-        int apic_id = apic_ids->cpus[i].arch_id;
+        int cpu_apic_id = apic_ids->cpus[i].arch_id;
 
-        assert(apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
+        assert(cpu_apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
 
-        dev = aml_processor(i, 0, 0, "CP%.02X", apic_id);
+        dev = aml_processor(i, 0, 0, "CP%.02X", cpu_apic_id);
 
         method = aml_method("_MAT", 0, AML_NOTSERIALIZED);
         aml_append(method,
-            aml_return(aml_call2(CPU_MAT_METHOD, aml_int(apic_id), aml_int(i))
+            aml_return(aml_call2(CPU_MAT_METHOD,
+                                 aml_int(cpu_apic_id), aml_int(i))
         ));
         aml_append(dev, method);
 
         method = aml_method("_STA", 0, AML_NOTSERIALIZED);
         aml_append(method,
-            aml_return(aml_call1(CPU_STATUS_METHOD, aml_int(apic_id))));
+            aml_return(aml_call1(CPU_STATUS_METHOD, aml_int(cpu_apic_id))));
         aml_append(dev, method);
 
         method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
         aml_append(method,
-            aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(apic_id),
+            aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(cpu_apic_id),
                 aml_arg(0)))
         );
         aml_append(dev, method);
@@ -298,11 +299,11 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
     /* Arg0 = APIC ID */
     method = aml_method(AML_NOTIFY_METHOD, 2, AML_NOTSERIALIZED);
     for (i = 0; i < apic_ids->len; i++) {
-        int apic_id = apic_ids->cpus[i].arch_id;
+        int cpu_apic_id = apic_ids->cpus[i].arch_id;
 
-        if_ctx = aml_if(aml_equal(aml_arg(0), aml_int(apic_id)));
+        if_ctx = aml_if(aml_equal(aml_arg(0), aml_int(cpu_apic_id)));
         aml_append(if_ctx,
-            aml_notify(aml_name("CP%.02X", apic_id), aml_arg(1))
+            aml_notify(aml_name("CP%.02X", cpu_apic_id), aml_arg(1))
         );
         aml_append(method, if_ctx);
     }
@@ -319,13 +320,13 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
                                         aml_varpackage(x86ms->apic_id_limit);
 
     for (i = 0, apic_idx = 0; i < apic_ids->len; i++) {
-        int apic_id = apic_ids->cpus[i].arch_id;
+        int cpu_apic_id = apic_ids->cpus[i].arch_id;
 
-        for (; apic_idx < apic_id; apic_idx++) {
+        for (; apic_idx < cpu_apic_id; apic_idx++) {
             aml_append(pkg, aml_int(0));
         }
         aml_append(pkg, aml_int(apic_ids->cpus[i].cpu ? 1 : 0));
-        apic_idx = apic_id + 1;
+        apic_idx = cpu_apic_id + 1;
     }
     aml_append(sb_scope, aml_name_decl(CPU_ON_BITMAP, pkg));
     aml_append(ctx, sb_scope);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 4d2d40bab5..95199c8900 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1585,12 +1585,12 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
             aml_append(dev, aml_name_decl("_UID", aml_int(bus_num)));
             aml_append(dev, aml_name_decl("_BBN", aml_int(bus_num)));
             if (pci_bus_is_cxl(bus)) {
-                struct Aml *pkg = aml_package(2);
+                struct Aml *aml_pkg = aml_package(2);
 
                 aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0016")));
-                aml_append(pkg, aml_eisaid("PNP0A08"));
-                aml_append(pkg, aml_eisaid("PNP0A03"));
-                aml_append(dev, aml_name_decl("_CID", pkg));
+                aml_append(aml_pkg, aml_eisaid("PNP0A08"));
+                aml_append(aml_pkg, aml_eisaid("PNP0A03"));
+                aml_append(dev, aml_name_decl("_CID", aml_pkg));
                 build_cxl_osc_method(dev);
             } else if (pci_bus_is_express(bus)) {
                 aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
@@ -1783,14 +1783,14 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         Object *pci_host = acpi_get_i386_pci_host();
 
         if (pci_host) {
-            PCIBus *bus = PCI_HOST_BRIDGE(pci_host)->bus;
-            Aml *scope = aml_scope("PCI0");
+            PCIBus *pbus = PCI_HOST_BRIDGE(pci_host)->bus;
+            Aml *ascope = aml_scope("PCI0");
             /* Scan all PCI buses. Generate tables to support hotplug. */
-            build_append_pci_bus_devices(scope, bus);
-            if (object_property_find(OBJECT(bus), ACPI_PCIHP_PROP_BSEL)) {
-                build_append_pcihp_slots(scope, bus);
+            build_append_pci_bus_devices(ascope, pbus);
+            if (object_property_find(OBJECT(pbus), ACPI_PCIHP_PROP_BSEL)) {
+                build_append_pcihp_slots(ascope, pbus);
             }
-            aml_append(sb_scope, scope);
+            aml_append(sb_scope, ascope);
         }
     }
 
@@ -1842,10 +1842,10 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         bool has_pcnt;
 
         Object *pci_host = acpi_get_i386_pci_host();
-        PCIBus *bus = PCI_HOST_BRIDGE(pci_host)->bus;
+        PCIBus *b = PCI_HOST_BRIDGE(pci_host)->bus;
 
         scope = aml_scope("\\_SB.PCI0");
-        has_pcnt = build_append_notfication_callback(scope, bus);
+        has_pcnt = build_append_notfication_callback(scope, b);
         if (has_pcnt) {
             aml_append(dsdt, scope);
         }
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index b753705856..2a90601ac5 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -1423,13 +1423,14 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
             if (!qemu_opts_validate(opts, qemu_smbios_type8_opts, errp)) {
                 return;
             }
-            struct type8_instance *t;
-            t = g_new0(struct type8_instance, 1);
-            save_opt(&t->internal_reference, opts, "internal_reference");
-            save_opt(&t->external_reference, opts, "external_reference");
-            t->connector_type = qemu_opt_get_number(opts, "connector_type", 0);
-            t->port_type = qemu_opt_get_number(opts, "port_type", 0);
-            QTAILQ_INSERT_TAIL(&type8, t, next);
+            struct type8_instance *t8_i;
+            t8_i = g_new0(struct type8_instance, 1);
+            save_opt(&t8_i->internal_reference, opts, "internal_reference");
+            save_opt(&t8_i->external_reference, opts, "external_reference");
+            t8_i->connector_type = qemu_opt_get_number(opts,
+                                                       "connector_type", 0);
+            t8_i->port_type = qemu_opt_get_number(opts, "port_type", 0);
+            QTAILQ_INSERT_TAIL(&type8, t8_i, next);
             return;
         case 11:
             if (!qemu_opts_validate(opts, qemu_smbios_type11_opts, errp)) {
@@ -1452,27 +1453,27 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
             type17.speed = qemu_opt_get_number(opts, "speed", 0);
             return;
         case 41: {
-            struct type41_instance *t;
+            struct type41_instance *t41_i;
             Error *local_err = NULL;
 
             if (!qemu_opts_validate(opts, qemu_smbios_type41_opts, errp)) {
                 return;
             }
-            t = g_new0(struct type41_instance, 1);
-            save_opt(&t->designation, opts, "designation");
-            t->kind = qapi_enum_parse(&type41_kind_lookup,
-                                      qemu_opt_get(opts, "kind"),
-                                      0, &local_err) + 1;
-            t->kind |= 0x80;     /* enabled */
+            t41_i = g_new0(struct type41_instance, 1);
+            save_opt(&t41_i->designation, opts, "designation");
+            t41_i->kind = qapi_enum_parse(&type41_kind_lookup,
+                                          qemu_opt_get(opts, "kind"),
+                                          0, &local_err) + 1;
+            t41_i->kind |= 0x80;     /* enabled */
             if (local_err != NULL) {
                 error_propagate(errp, local_err);
-                g_free(t);
+                g_free(t41_i);
                 return;
             }
-            t->instance = qemu_opt_get_number(opts, "instance", 1);
-            save_opt(&t->pcidev, opts, "pcidev");
+            t41_i->instance = qemu_opt_get_number(opts, "instance", 1);
+            save_opt(&t41_i->pcidev, opts, "pcidev");
 
-            QTAILQ_INSERT_TAIL(&type41, t, next);
+            QTAILQ_INSERT_TAIL(&type41, t41_i, next);
             return;
         }
         default:
-- 
2.39.1

Re: [PATCH] hw/acpi: changes towards enabling -Wshadow=local

[Michael S. Tsirkin]

On Fri, Sep 22, 2023 at 06:12:02PM +0530, Ani Sinha wrote:
> Code changes in acpi that addresses all compiler complaints coming from enabling
> -Wshadow flags. Enabling -Wshadow catches cases of local variables shadowing
> other local variables or parameters. These makes the code confusing and/or adds
> bugs that are difficult to catch.
> 
> The code is tested to build with and without the flag turned on.
> 
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Philippe Mathieu-Daude <philmd@linaro.org>
> CC: mst@redhat.com
> CC: imammedo@redhat.com
> Message-Id: <87r0mqlf9x.fsf@pond.sub.org>
> Signed-off-by: Ani Sinha <anisinha@redhat.com>

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


> ---
>  hw/acpi/cpu_hotplug.c | 25 +++++++++++++------------
>  hw/i386/acpi-build.c  | 24 ++++++++++++------------
>  hw/smbios/smbios.c    | 37 +++++++++++++++++++------------------
>  3 files changed, 44 insertions(+), 42 deletions(-)
> 
> diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> index ff14c3f410..634bbecb31 100644
> --- a/hw/acpi/cpu_hotplug.c
> +++ b/hw/acpi/cpu_hotplug.c
> @@ -265,26 +265,27 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
>  
>      /* build Processor object for each processor */
>      for (i = 0; i < apic_ids->len; i++) {
> -        int apic_id = apic_ids->cpus[i].arch_id;
> +        int cpu_apic_id = apic_ids->cpus[i].arch_id;
>  
> -        assert(apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
> +        assert(cpu_apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
>  
> -        dev = aml_processor(i, 0, 0, "CP%.02X", apic_id);
> +        dev = aml_processor(i, 0, 0, "CP%.02X", cpu_apic_id);
>  
>          method = aml_method("_MAT", 0, AML_NOTSERIALIZED);
>          aml_append(method,
> -            aml_return(aml_call2(CPU_MAT_METHOD, aml_int(apic_id), aml_int(i))
> +            aml_return(aml_call2(CPU_MAT_METHOD,
> +                                 aml_int(cpu_apic_id), aml_int(i))
>          ));
>          aml_append(dev, method);
>  
>          method = aml_method("_STA", 0, AML_NOTSERIALIZED);
>          aml_append(method,
> -            aml_return(aml_call1(CPU_STATUS_METHOD, aml_int(apic_id))));
> +            aml_return(aml_call1(CPU_STATUS_METHOD, aml_int(cpu_apic_id))));
>          aml_append(dev, method);
>  
>          method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
>          aml_append(method,
> -            aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(apic_id),
> +            aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(cpu_apic_id),
>                  aml_arg(0)))
>          );
>          aml_append(dev, method);
> @@ -298,11 +299,11 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
>      /* Arg0 = APIC ID */
>      method = aml_method(AML_NOTIFY_METHOD, 2, AML_NOTSERIALIZED);
>      for (i = 0; i < apic_ids->len; i++) {
> -        int apic_id = apic_ids->cpus[i].arch_id;
> +        int cpu_apic_id = apic_ids->cpus[i].arch_id;
>  
> -        if_ctx = aml_if(aml_equal(aml_arg(0), aml_int(apic_id)));
> +        if_ctx = aml_if(aml_equal(aml_arg(0), aml_int(cpu_apic_id)));
>          aml_append(if_ctx,
> -            aml_notify(aml_name("CP%.02X", apic_id), aml_arg(1))
> +            aml_notify(aml_name("CP%.02X", cpu_apic_id), aml_arg(1))
>          );
>          aml_append(method, if_ctx);
>      }
> @@ -319,13 +320,13 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
>                                          aml_varpackage(x86ms->apic_id_limit);
>  
>      for (i = 0, apic_idx = 0; i < apic_ids->len; i++) {
> -        int apic_id = apic_ids->cpus[i].arch_id;
> +        int cpu_apic_id = apic_ids->cpus[i].arch_id;
>  
> -        for (; apic_idx < apic_id; apic_idx++) {
> +        for (; apic_idx < cpu_apic_id; apic_idx++) {
>              aml_append(pkg, aml_int(0));
>          }
>          aml_append(pkg, aml_int(apic_ids->cpus[i].cpu ? 1 : 0));
> -        apic_idx = apic_id + 1;
> +        apic_idx = cpu_apic_id + 1;
>      }
>      aml_append(sb_scope, aml_name_decl(CPU_ON_BITMAP, pkg));
>      aml_append(ctx, sb_scope);
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 4d2d40bab5..95199c8900 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1585,12 +1585,12 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>              aml_append(dev, aml_name_decl("_UID", aml_int(bus_num)));
>              aml_append(dev, aml_name_decl("_BBN", aml_int(bus_num)));
>              if (pci_bus_is_cxl(bus)) {
> -                struct Aml *pkg = aml_package(2);
> +                struct Aml *aml_pkg = aml_package(2);
>  
>                  aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0016")));
> -                aml_append(pkg, aml_eisaid("PNP0A08"));
> -                aml_append(pkg, aml_eisaid("PNP0A03"));
> -                aml_append(dev, aml_name_decl("_CID", pkg));
> +                aml_append(aml_pkg, aml_eisaid("PNP0A08"));
> +                aml_append(aml_pkg, aml_eisaid("PNP0A03"));
> +                aml_append(dev, aml_name_decl("_CID", aml_pkg));
>                  build_cxl_osc_method(dev);
>              } else if (pci_bus_is_express(bus)) {
>                  aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
> @@ -1783,14 +1783,14 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>          Object *pci_host = acpi_get_i386_pci_host();
>  
>          if (pci_host) {
> -            PCIBus *bus = PCI_HOST_BRIDGE(pci_host)->bus;
> -            Aml *scope = aml_scope("PCI0");
> +            PCIBus *pbus = PCI_HOST_BRIDGE(pci_host)->bus;
> +            Aml *ascope = aml_scope("PCI0");
>              /* Scan all PCI buses. Generate tables to support hotplug. */
> -            build_append_pci_bus_devices(scope, bus);
> -            if (object_property_find(OBJECT(bus), ACPI_PCIHP_PROP_BSEL)) {
> -                build_append_pcihp_slots(scope, bus);
> +            build_append_pci_bus_devices(ascope, pbus);
> +            if (object_property_find(OBJECT(pbus), ACPI_PCIHP_PROP_BSEL)) {
> +                build_append_pcihp_slots(ascope, pbus);
>              }
> -            aml_append(sb_scope, scope);
> +            aml_append(sb_scope, ascope);
>          }
>      }
>  
> @@ -1842,10 +1842,10 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>          bool has_pcnt;
>  
>          Object *pci_host = acpi_get_i386_pci_host();
> -        PCIBus *bus = PCI_HOST_BRIDGE(pci_host)->bus;
> +        PCIBus *b = PCI_HOST_BRIDGE(pci_host)->bus;
>  
>          scope = aml_scope("\\_SB.PCI0");
> -        has_pcnt = build_append_notfication_callback(scope, bus);
> +        has_pcnt = build_append_notfication_callback(scope, b);
>          if (has_pcnt) {
>              aml_append(dsdt, scope);
>          }
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index b753705856..2a90601ac5 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -1423,13 +1423,14 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
>              if (!qemu_opts_validate(opts, qemu_smbios_type8_opts, errp)) {
>                  return;
>              }
> -            struct type8_instance *t;
> -            t = g_new0(struct type8_instance, 1);
> -            save_opt(&t->internal_reference, opts, "internal_reference");
> -            save_opt(&t->external_reference, opts, "external_reference");
> -            t->connector_type = qemu_opt_get_number(opts, "connector_type", 0);
> -            t->port_type = qemu_opt_get_number(opts, "port_type", 0);
> -            QTAILQ_INSERT_TAIL(&type8, t, next);
> +            struct type8_instance *t8_i;
> +            t8_i = g_new0(struct type8_instance, 1);
> +            save_opt(&t8_i->internal_reference, opts, "internal_reference");
> +            save_opt(&t8_i->external_reference, opts, "external_reference");
> +            t8_i->connector_type = qemu_opt_get_number(opts,
> +                                                       "connector_type", 0);
> +            t8_i->port_type = qemu_opt_get_number(opts, "port_type", 0);
> +            QTAILQ_INSERT_TAIL(&type8, t8_i, next);
>              return;
>          case 11:
>              if (!qemu_opts_validate(opts, qemu_smbios_type11_opts, errp)) {
> @@ -1452,27 +1453,27 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
>              type17.speed = qemu_opt_get_number(opts, "speed", 0);
>              return;
>          case 41: {
> -            struct type41_instance *t;
> +            struct type41_instance *t41_i;
>              Error *local_err = NULL;
>  
>              if (!qemu_opts_validate(opts, qemu_smbios_type41_opts, errp)) {
>                  return;
>              }
> -            t = g_new0(struct type41_instance, 1);
> -            save_opt(&t->designation, opts, "designation");
> -            t->kind = qapi_enum_parse(&type41_kind_lookup,
> -                                      qemu_opt_get(opts, "kind"),
> -                                      0, &local_err) + 1;
> -            t->kind |= 0x80;     /* enabled */
> +            t41_i = g_new0(struct type41_instance, 1);
> +            save_opt(&t41_i->designation, opts, "designation");
> +            t41_i->kind = qapi_enum_parse(&type41_kind_lookup,
> +                                          qemu_opt_get(opts, "kind"),
> +                                          0, &local_err) + 1;
> +            t41_i->kind |= 0x80;     /* enabled */
>              if (local_err != NULL) {
>                  error_propagate(errp, local_err);
> -                g_free(t);
> +                g_free(t41_i);
>                  return;
>              }
> -            t->instance = qemu_opt_get_number(opts, "instance", 1);
> -            save_opt(&t->pcidev, opts, "pcidev");
> +            t41_i->instance = qemu_opt_get_number(opts, "instance", 1);
> +            save_opt(&t41_i->pcidev, opts, "pcidev");
>  
> -            QTAILQ_INSERT_TAIL(&type41, t, next);
> +            QTAILQ_INSERT_TAIL(&type41, t41_i, next);
>              return;
>          }
>          default:
> -- 
> 2.39.1

Re: [PATCH] hw/acpi: changes towards enabling -Wshadow=local

[Markus Armbruster]

Ani Sinha <anisinha@redhat.com> writes:

> Code changes in acpi that addresses all compiler complaints coming from enabling
> -Wshadow flags. Enabling -Wshadow catches cases of local variables shadowing
> other local variables or parameters. These makes the code confusing and/or adds
> bugs that are difficult to catch.
>
> The code is tested to build with and without the flag turned on.
>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Philippe Mathieu-Daude <philmd@linaro.org>
> CC: mst@redhat.com
> CC: imammedo@redhat.com
> Message-Id: <87r0mqlf9x.fsf@pond.sub.org>

This is my "Help wanted for enabling -Wshadow=local" post.

A commit's Message-Id tag is supposed to point to the patch submission
e-mail, and git-am will add that:

  Message-ID: <20230922124203.127110-1-anisinha@redhat.com>

We'll have two Message-IDs then.  Confusing.

Could perhaps use

  See-also: Message-Id: <87r0mqlf9x.fsf@pond.sub.org>

but I doubt it's worth the bother.

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

Queued less the extra Message-Id, thanks!

Re: [PATCH] hw/acpi: changes towards enabling -Wshadow=local

[Ani Sinha]

> On 29-Sep-2023, at 11:17 AM, Markus Armbruster <armbru@redhat.com> wrote:
> 
> Ani Sinha <anisinha@redhat.com> writes:
> 
>> Code changes in acpi that addresses all compiler complaints coming from enabling
>> -Wshadow flags. Enabling -Wshadow catches cases of local variables shadowing
>> other local variables or parameters. These makes the code confusing and/or adds
>> bugs that are difficult to catch.
>> 
>> The code is tested to build with and without the flag turned on.
>> 
>> CC: Markus Armbruster <armbru@redhat.com>
>> CC: Philippe Mathieu-Daude <philmd@linaro.org>
>> CC: mst@redhat.com
>> CC: imammedo@redhat.com
>> Message-Id: <87r0mqlf9x.fsf@pond.sub.org>
> 
> This is my "Help wanted for enabling -Wshadow=local" post.

Yes indeed. I wanted to refer to that thread for context in the commit log.

> 
> A commit's Message-Id tag is supposed to point to the patch submission
> e-mail, and git-am will add that:
> 
>  Message-ID: <20230922124203.127110-1-anisinha@redhat.com>
> 
> We'll have two Message-IDs then.  Confusing.
> 
> Could perhaps use
> 
>  See-also: Message-Id: <87r0mqlf9x.fsf@pond.sub.org>
> 
> but I doubt it's worth the bother.

OK

> 
>> Signed-off-by: Ani Sinha <anisinha@redhat.com>
> 
> Queued less the extra Message-Id, thanks!

Re: [PATCH] hw/acpi: changes towards enabling -Wshadow=local

[Markus Armbruster]

Ani Sinha <anisinha@redhat.com> writes:

>> On 29-Sep-2023, at 11:17 AM, Markus Armbruster <armbru@redhat.com> wrote:
>> 
>> Ani Sinha <anisinha@redhat.com> writes:
>> 
>>> Code changes in acpi that addresses all compiler complaints coming from enabling
>>> -Wshadow flags. Enabling -Wshadow catches cases of local variables shadowing
>>> other local variables or parameters. These makes the code confusing and/or adds
>>> bugs that are difficult to catch.
>>> 
>>> The code is tested to build with and without the flag turned on.
>>> 
>>> CC: Markus Armbruster <armbru@redhat.com>
>>> CC: Philippe Mathieu-Daude <philmd@linaro.org>
>>> CC: mst@redhat.com
>>> CC: imammedo@redhat.com
>>> Message-Id: <87r0mqlf9x.fsf@pond.sub.org>
>> 
>> This is my "Help wanted for enabling -Wshadow=local" post.
>
> Yes indeed. I wanted to refer to that thread for context in the commit log.

I appreciate your diligence.  We just don't have an established tag
convention for "see also" references to e-mail.  I could append

    See also

        Subject: Help wanted for enabling -Wshadow=local
        Message-Id: <87r0mqlf9x.fsf@pond.sub.org>
        https://lore.kernel.org/qemu-devel/87r0mqlf9x.fsf@pond.sub.org

to your first paragraph.  Want me to?

>> A commit's Message-Id tag is supposed to point to the patch submission
>> e-mail, and git-am will add that:
>> 
>>  Message-ID: <20230922124203.127110-1-anisinha@redhat.com>
>> 
>> We'll have two Message-IDs then.  Confusing.
>> 
>> Could perhaps use
>> 
>>  See-also: Message-Id: <87r0mqlf9x.fsf@pond.sub.org>
>> 
>> but I doubt it's worth the bother.
>
> OK
>
>> 
>>> Signed-off-by: Ani Sinha <anisinha@redhat.com>
>> 
>> Queued less the extra Message-Id, thanks!

Re: [PATCH] hw/acpi: changes towards enabling -Wshadow=local

[Ani Sinha]

> On 29-Sep-2023, at 11:43 AM, Markus Armbruster <armbru@redhat.com> wrote:
> 
> Ani Sinha <anisinha@redhat.com> writes:
> 
>>> On 29-Sep-2023, at 11:17 AM, Markus Armbruster <armbru@redhat.com> wrote:
>>> 
>>> Ani Sinha <anisinha@redhat.com> writes:
>>> 
>>>> Code changes in acpi that addresses all compiler complaints coming from enabling
>>>> -Wshadow flags. Enabling -Wshadow catches cases of local variables shadowing
>>>> other local variables or parameters. These makes the code confusing and/or adds
>>>> bugs that are difficult to catch.
>>>> 
>>>> The code is tested to build with and without the flag turned on.
>>>> 
>>>> CC: Markus Armbruster <armbru@redhat.com>
>>>> CC: Philippe Mathieu-Daude <philmd@linaro.org>
>>>> CC: mst@redhat.com
>>>> CC: imammedo@redhat.com
>>>> Message-Id: <87r0mqlf9x.fsf@pond.sub.org>
>>> 
>>> This is my "Help wanted for enabling -Wshadow=local" post.
>> 
>> Yes indeed. I wanted to refer to that thread for context in the commit log.
> 
> I appreciate your diligence.  We just don't have an established tag
> convention for "see also" references to e-mail.  I could append
> 
>    See also
> 
>        Subject: Help wanted for enabling -Wshadow=local
>        Message-Id: <87r0mqlf9x.fsf@pond.sub.org>
>        https://lore.kernel.org/qemu-devel/87r0mqlf9x.fsf@pond.sub.org
> 
> to your first paragraph.  Want me to?

Sure, if that is ok.

> 
>>> A commit's Message-Id tag is supposed to point to the patch submission
>>> e-mail, and git-am will add that:
>>> 
>>> Message-ID: <20230922124203.127110-1-anisinha@redhat.com>
>>> 
>>> We'll have two Message-IDs then.  Confusing.
>>> 
>>> Could perhaps use
>>> 
>>> See-also: Message-Id: <87r0mqlf9x.fsf@pond.sub.org>
>>> 
>>> but I doubt it's worth the bother.
>> 
>> OK
>> 
>>> 
>>>> Signed-off-by: Ani Sinha <anisinha@redhat.com>
>>> 
>>> Queued less the extra Message-Id, thanks!

Re: [PATCH] hw/acpi: changes towards enabling -Wshadow=local

[Markus Armbruster]

Ani Sinha <anisinha@redhat.com> writes:

>> On 29-Sep-2023, at 11:43 AM, Markus Armbruster <armbru@redhat.com> wrote:
>> 
>> Ani Sinha <anisinha@redhat.com> writes:
>> 
>>>> On 29-Sep-2023, at 11:17 AM, Markus Armbruster <armbru@redhat.com> wrote:
>>>> 
>>>> Ani Sinha <anisinha@redhat.com> writes:
>>>> 
>>>>> Code changes in acpi that addresses all compiler complaints coming from enabling
>>>>> -Wshadow flags. Enabling -Wshadow catches cases of local variables shadowing
>>>>> other local variables or parameters. These makes the code confusing and/or adds
>>>>> bugs that are difficult to catch.
>>>>> 
>>>>> The code is tested to build with and without the flag turned on.
>>>>> 
>>>>> CC: Markus Armbruster <armbru@redhat.com>
>>>>> CC: Philippe Mathieu-Daude <philmd@linaro.org>
>>>>> CC: mst@redhat.com
>>>>> CC: imammedo@redhat.com
>>>>> Message-Id: <87r0mqlf9x.fsf@pond.sub.org>
>>>> 
>>>> This is my "Help wanted for enabling -Wshadow=local" post.
>>> 
>>> Yes indeed. I wanted to refer to that thread for context in the commit log.
>> 
>> I appreciate your diligence.  We just don't have an established tag
>> convention for "see also" references to e-mail.  I could append
>> 
>>    See also
>> 
>>        Subject: Help wanted for enabling -Wshadow=local
>>        Message-Id: <87r0mqlf9x.fsf@pond.sub.org>
>>        https://lore.kernel.org/qemu-devel/87r0mqlf9x.fsf@pond.sub.org
>> 
>> to your first paragraph.  Want me to?
>
> Sure, if that is ok.

Done!

Re: [PATCH] hw/acpi: changes towards enabling -Wshadow=local

[Ani Sinha]

> On 29-Sep-2023, at 1:32 PM, Markus Armbruster <armbru@redhat.com> wrote:
> 
> Ani Sinha <anisinha@redhat.com> writes:
> 
>>> On 29-Sep-2023, at 11:43 AM, Markus Armbruster <armbru@redhat.com> wrote:
>>> 
>>> Ani Sinha <anisinha@redhat.com> writes:
>>> 
>>>>> On 29-Sep-2023, at 11:17 AM, Markus Armbruster <armbru@redhat.com> wrote:
>>>>> 
>>>>> Ani Sinha <anisinha@redhat.com> writes:
>>>>> 
>>>>>> Code changes in acpi that addresses all compiler complaints coming from enabling
>>>>>> -Wshadow flags. Enabling -Wshadow catches cases of local variables shadowing
>>>>>> other local variables or parameters. These makes the code confusing and/or adds
>>>>>> bugs that are difficult to catch.
>>>>>> 
>>>>>> The code is tested to build with and without the flag turned on.
>>>>>> 
>>>>>> CC: Markus Armbruster <armbru@redhat.com>
>>>>>> CC: Philippe Mathieu-Daude <philmd@linaro.org>
>>>>>> CC: mst@redhat.com
>>>>>> CC: imammedo@redhat.com
>>>>>> Message-Id: <87r0mqlf9x.fsf@pond.sub.org>
>>>>> 
>>>>> This is my "Help wanted for enabling -Wshadow=local" post.
>>>> 
>>>> Yes indeed. I wanted to refer to that thread for context in the commit log.
>>> 
>>> I appreciate your diligence.  We just don't have an established tag
>>> convention for "see also" references to e-mail.  I could append
>>> 
>>>   See also
>>> 
>>>       Subject: Help wanted for enabling -Wshadow=local
>>>       Message-Id: <87r0mqlf9x.fsf@pond.sub.org>
>>>       https://lore.kernel.org/qemu-devel/87r0mqlf9x.fsf@pond.sub.org
>>> 
>>> to your first paragraph.  Want me to?
>> 
>> Sure, if that is ok.
> 
> Done!

Your shadow-next has no changes. Have you not pushed to that branch?

Re: [PATCH] hw/acpi: changes towards enabling -Wshadow=local

[Markus Armbruster]

Ani Sinha <anisinha@redhat.com> writes:

> Your shadow-next has no changes. Have you not pushed to that branch?

I did, but only after I was done updating patches.  Intend to post my
pull request shortly.