Re: [Qemu-ppc] [PATCH 5/6] spapr-cpu-core: release ICP object when realization fails

[Greg Kurz]

On Mon, 15 May 2017 14:02:34 +0200
Cédric Le Goater <address@hidden> wrote:

> On 05/15/2017 01:39 PM, Greg Kurz wrote:
> > While here we introduce a single error path to avoid code duplication.
> > 
> > Signed-off-by: Greg Kurz <address@hidden>
> > ---
> >  hw/ppc/spapr_cpu_core.c |   16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > index 4389ef4c2aef..63d160f7e010 100644
> > --- a/hw/ppc/spapr_cpu_core.c
> > +++ b/hw/ppc/spapr_cpu_core.c
> > @@ -147,25 +147,25 @@ static void spapr_cpu_core_realize_child(Object 
> > *child, Error **errp)
> >      object_property_add_const_link(obj, "xics", OBJECT(spapr), 
> > &error_abort);
> >      object_property_set_bool(obj, true, "realized", &local_err);
> >      if (local_err) {
> > -        error_propagate(errp, local_err);
> > -        return;
> > +        goto error;  
> 
> the error: statement below adds an object_unparent(). is that ok ? 
> 

Yes, this is the purpose of the patch actually.

The ICP object gets parented to the CPU a few lines above, so I guess we
should unparent in this error path as well.

Maybe I should rename the patch to "unparent ICP object when realization fails" 
?

> C. 
> 
> >      }
> >  
> >      object_property_set_bool(child, true, "realized", &local_err);
> >      if (local_err) {
> > -        object_unparent(obj);
> > -        error_propagate(errp, local_err);
> > -        return;
> > +        goto error;
> >      }
> >  
> >      spapr_cpu_init(spapr, cpu, &local_err);
> >      if (local_err) {
> > -        object_unparent(obj);
> > -        error_propagate(errp, local_err);
> > -        return;
> > +        goto error;
> >      }
> >  
> >      xics_cpu_setup(XICS_FABRIC(spapr), cpu, ICP(obj));
> > +    return;
> > +
> > +error:
> > +    object_unparent(obj);
> > +    error_propagate(errp, local_err);
> >  }
> >  
> >  static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
> >   
>

pgpnHasyX5kt7.pgp
Description: OpenPGP digital signature