Re: [PATCH] hw/acpi: some cosmetic improvements to existing code

[Ani Sinha]

> > > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> > > > index f4d706e47d..856c6e1b47 100644
> > > > --- a/hw/acpi/pcihp.c
> > > > +++ b/hw/acpi/pcihp.c
> > > > @@ -136,6 +136,11 @@ static void acpi_pcihp_disable_root_bus(void)
> > > >          return;
> > > >      }
> > > >
> > > > +    if (!host) {
> > > > +        root_hp_disabled = true;
> > > > +        return;
> > > > +    }
> > > It should be a separate patch,
> > > when this could return NULL?
> > > If it should never be null then assert here would be better.
> >
> > I have sent a v2 without the comment addition. I left the code this way
> > because everywhere else, the code checking host for NULL value is similar.
> > I wanted to keep the symmetry. However, if you strongly feel about the
> > assertion, I will send a v3.
>
> So the first thing is to confirm if NULL return value is valid or not.
> When it clear we can decide whether copy existing check like you do or
> replace all such checks with asserts.

I thought about this a little and I am now inclined to put assertions
everywhere. On i386 we have either q35 or i440fx host bridges. Having a
null host bridge does not sound right. I will make the change, build and
make check before sending out v3 with assertions.