Re: [Qemu-devel] [PATCH] qom: Make object_unref() free the object's memory when refcount goes to 0.

[Alexander Barabash]

On 02/22/2012 09:12 PM, Anthony Liguori wrote:
> On 02/22/2012 12:00 PM, address@hidden wrote:
> > From: Alexander Barabash<address@hidden>
> >
> > In the existing implementation, object_delete()
> > calls object_unref(), then frees the object's storage.
> > Running object_delete() on an object with reference count
> > different from 1 causes program failure.
> >
> > In the existing implementation, object_unref()
> > finalizes the object when its reference count becomes 0.
> >
> > In the new implementation, object_unref()
> > finalizes and frees the object's storage when the reference count 
> > becomes 0.
> >
> > In the new implementation, object_delete()
> > just calls object_unref().
> > Running object_delete() on an object with reference count
> > different from 1 still causes program failure.
>
> This isn't correct.  QOM objects don't necessarily have heap allocated 
> objects.
>
> I've been thinking about this general problem and I think the right 
> way to solve it is to have a delete notifier list.  That way, 
> object_new() can register a delete notifier that calls g_free() 
> whenever refcount=0.  That way an explicit object_delete() isn't 
> needed anymore.

Why do you want to have a delete notifier list, rather than just a 
delete callback.
At the point where refcount == 0, the destructor has been called already,
so there is not much to be done, except for reclaim the memory.

Regards,
Alex

> Although I think we should keep the call around as it's convenient for 
> replacing occurrences of qdev_free() where you really want the assert.
>
> Regards,
>
> Anthony Liguori
>
> > Signed-off-by: Alexander Barabash<address@hidden>
> > ---
> >   qom/object.c |    4 ++--
> >   1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/qom/object.c b/qom/object.c
> > index e6591e1..8d36a9c 100644
> > --- a/qom/object.c
> > +++ b/qom/object.c
> > @@ -373,9 +373,8 @@ Object *object_new(const char *typename)
> >
> >   void object_delete(Object *obj)
> >   {
> > +    g_assert(obj->ref == 1);
> >       object_unref(obj);
> > -    g_assert(obj->ref == 0);
> > -    g_free(obj);
> >   }
> >
> >   static bool type_is_ancestor(TypeImpl *type, TypeImpl *target_type)
> > @@ -585,6 +584,7 @@ void object_unref(Object *obj)
> >       /* parent always holds a reference to its children */
> >       if (obj->ref == 0) {
> >           object_finalize(obj);
> > +        g_free(obj);
> >       }
> >   }