Re: [Qemu-devel] [PATCH for-4.0] hw/i386/pc: Fix crash when hot-plugging nvdimm on older machine types

[Eduardo Habkost <ehabkost@redhat.com>]

On Mon, Apr 08, 2019 at 05:06:49PM +0200, Thomas Huth wrote:
> On 08/04/2019 15.45, Wei Yang wrote:
[...]
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 6077d27361..b11f3b15c1 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -2091,6 +2091,9 @@ static void pc_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> >      }
> >  
> >      hotplug_handler_pre_plug(pcms->acpi_dev, dev, errp);
> > +    if (*errp) {
> > +        return;
> > +    }
> 
> Not sure, but I think you can not rely on the fact that the caller set
> *errp = NULL already... that's why it is more common to use a local_err
> variable and error_propagate() for such cases (which is what I did in my
> patch).

*errp can't be non-NULL (otherwise functions calling error_setg()
would crash).  errp can be NULL, though, and that's why you need
a local_err variable.

I'd love to eliminate NULL errp from our codebase, but I couldn't
find a way to do it that is safe and simple (i.e. not letting us
pass NULL errp by mistake and not requiring a macro wrapping
every `&local_err` expression).

-- 
Eduardo