qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC PATCH 03/13] qom: move unparenting to the child pr


From: Peter Crosthwaite
Subject: Re: [Qemu-devel] [RFC PATCH 03/13] qom: move unparenting to the child property's release callback
Date: Wed, 11 Jun 2014 23:35:13 +1000

On Wed, Jun 11, 2014 at 10:19 PM, Paolo Bonzini <address@hidden> wrote:
> This ensures that the unparent callback is called automatically
> when the parent object is finalized.
>
> Note that there's no need to keep a reference neither in
> object_unparent nor in object_finalize_child_property.  The
> reference held by the child property itself will do.
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  qom/object.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/qom/object.c b/qom/object.c
> index 01087e5..69a95d6 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -386,19 +386,9 @@ static void object_property_del_child(Object *obj, 
> Object *child, Error **errp)
>
>  void object_unparent(Object *obj)
>  {
> -    if (!obj->parent) {
> -        return;
> -    }
> -
> -    object_ref(obj);
> -    if (obj->class->unparent) {
> -        (obj->class->unparent)(obj);
> -    }

This changes this publicly visible API significantly, in that unparent
from the childs point-of-view is now invisible in cases where you are
doing a standalone object_unparent() call.

I'm not sure why parent finalisation is the only occasion on which a
genuine and complete unparenting is allowed? I think this API should
remain complete should a QOM user wish to re-arrange child
relationships seperate from any finalisations.

>      if (obj->parent) {
>          object_property_del_child(obj->parent, obj, NULL);
> -        obj->parent = NULL;
>      }
> -    object_unref(obj);
>  }
>
>  static void object_deinit(Object *obj, TypeImpl *type)
> @@ -1017,6 +1007,10 @@ static void object_finalize_child_property(Object 
> *obj, const char *name,
>  {
>      Object *child = opaque;
>
> +    if (child->class->unparent) {
> +        (child->class->unparent)(child);
> +    }
> +    child->parent = NULL;

Can you just leave object_unparent() mostly as-is and call it from
here? It's already guarded against repeated calls should there be
users calling it manually before parent finalisations.

Regards,
Peter

>      object_unref(child);
>  }
>
> --
> 1.8.3.1
>
>
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]