[PATCH-for-5.2] acpi-build: Fix maybe-uninitialized warning when ACPI hotplug is off

[PATCH-for-5.2] acpi-build: Fix maybe-uninitialized warning when ACPI hotplug is off

[Ani Sinha]

 This fixes the following warning (gcc 9.3.0 on Ubuntu):

  ../hw/i386/acpi-build.c: In function 'build_append_pci_bus_devices':
  ../hw/i386/acpi-build.c:496:9: error: 'method' may be used uninitialized
  in this function [-Werror=maybe-uninitialized]
    496 |         aml_append(parent_scope, method);
        |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  cc1: all warnings being treated as errors

Fixes: df4008c9c59 ("piix4: don't reserve hw resources when hotplug is off globally")
Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Ani Sinha <ani@anisinha.ca>
---
 hw/i386/acpi-build.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 4f66642d88..79b86d4a36 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -349,7 +349,7 @@ static void build_append_pcihp_notify_entry(Aml *method, int slot)
 static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
                                          bool pcihp_bridge_en)
 {
-    Aml *dev, *notify_method = NULL, *method;
+    Aml *dev, *notify_method = NULL, *method = NULL;
     QObject *bsel;
     PCIBus *sec;
     int i;
@@ -492,7 +492,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
         }
     }
 
-    if (bsel || pcihp_bridge_en) {
+    if (method) {
         aml_append(parent_scope, method);
     }
     qobject_unref(bsel);
-- 
2.25.1

Re: [PATCH-for-5.2] acpi-build: Fix maybe-uninitialized warning when ACPI hotplug is off

[Michael S. Tsirkin]

On Sat, Nov 07, 2020 at 08:38:51PM +0530, Ani Sinha wrote:
>  This fixes the following warning (gcc 9.3.0 on Ubuntu):
> 
>   ../hw/i386/acpi-build.c: In function 'build_append_pci_bus_devices':
>   ../hw/i386/acpi-build.c:496:9: error: 'method' may be used uninitialized
>   in this function [-Werror=maybe-uninitialized]
>     496 |         aml_append(parent_scope, method);
>         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   cc1: all warnings being treated as errors
> 
> Fixes: df4008c9c59 ("piix4: don't reserve hw resources when hotplug is off globally")
> Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Ani Sinha <ani@anisinha.ca>
> ---
>  hw/i386/acpi-build.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 4f66642d88..79b86d4a36 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -349,7 +349,7 @@ static void build_append_pcihp_notify_entry(Aml *method, int slot)
>  static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
>                                           bool pcihp_bridge_en)
>  {
> -    Aml *dev, *notify_method = NULL, *method;
> +    Aml *dev, *notify_method = NULL, *method = NULL;
>      QObject *bsel;
>      PCIBus *sec;
>      int i;
> @@ -492,7 +492,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
>          }
>      }
>  
> -    if (bsel || pcihp_bridge_en) {
> +    if (method) {
>          aml_append(parent_scope, method);
>      }
>      qobject_unref(bsel);

I prefer Philippe's fix I think - gcc does not warn about it but
using method when it's NULL would lead to a crash.

> -- 
> 2.25.1

Re: [PATCH-for-5.2] acpi-build: Fix maybe-uninitialized warning when ACPI hotplug is off

[Ani Sinha]

On Sat, Nov 7, 2020 at 9:56 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Sat, Nov 07, 2020 at 08:38:51PM +0530, Ani Sinha wrote:
> >  This fixes the following warning (gcc 9.3.0 on Ubuntu):
> >
> >   ../hw/i386/acpi-build.c: In function 'build_append_pci_bus_devices':
> >   ../hw/i386/acpi-build.c:496:9: error: 'method' may be used uninitialized
> >   in this function [-Werror=maybe-uninitialized]
> >     496 |         aml_append(parent_scope, method);
> >         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >   cc1: all warnings being treated as errors
> >
> > Fixes: df4008c9c59 ("piix4: don't reserve hw resources when hotplug is off globally")
> > Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > Signed-off-by: Ani Sinha <ani@anisinha.ca>
> > ---
> >  hw/i386/acpi-build.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 4f66642d88..79b86d4a36 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -349,7 +349,7 @@ static void build_append_pcihp_notify_entry(Aml *method, int slot)
> >  static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> >                                           bool pcihp_bridge_en)
> >  {
> > -    Aml *dev, *notify_method = NULL, *method;
> > +    Aml *dev, *notify_method = NULL, *method = NULL;
> >      QObject *bsel;
> >      PCIBus *sec;
> >      int i;
> > @@ -492,7 +492,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> >          }
> >      }
> >
> > -    if (bsel || pcihp_bridge_en) {
> > +    if (method) {
> >          aml_append(parent_scope, method);
> >      }
> >      qobject_unref(bsel);
>
> I prefer Philippe's fix I think - gcc does not warn about it but
> using method when it's NULL would lead to a crash.

I could be wrong but I do not see a case where we are using a method
which is uninitialized first.
I did see another bug in my V1 and I fixed it in V2.

>
> > --
> > 2.25.1
>