Patchwork [1.3,3/5] qom: make object_delete usable for statically-allocated objects

login
register
mail settings
Submitter Paolo Bonzini
Date Nov. 23, 2012, 8:47 a.m.
Message ID <1353660436-8897-4-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/201271/
State New
Headers show

Comments

Paolo Bonzini - Nov. 23, 2012, 8:47 a.m.
Store in the object the freeing function that will be used at deletion
time.  This makes it possible to use object_delete on statically-allocated
(embedded) objects.  Dually, it makes it possible to use object_unparent
and object_unref without leaking memory, when the lifetime of object
might extend until after the call to object_delete.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
        Ping Fan, this is the patch I mentioned at
        http://permalink.gmane.org/gmane.comp.emulators.qemu/180054.
        With this patch, your call of object_unref for qdev_unplug_complete
        will not leak the device anymore (object_finalize will free it
        when the last reference is dropped with object_unref).

 include/qemu/object.h | 9 +++++++++
 qom/object.c          | 5 ++++-
 2 files changed, 13 insertions(+), 1 deletion(-)
Andreas Färber - Nov. 23, 2012, 5:02 p.m.
Am 23.11.2012 09:47, schrieb Paolo Bonzini:
> Store in the object the freeing function that will be used at deletion
> time.  This makes it possible to use object_delete on statically-allocated
> (embedded) objects.  Dually, it makes it possible to use object_unparent
> and object_unref without leaking memory, when the lifetime of object
> might extend until after the call to object_delete.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

The code is

Reviewed-by: Andreas Färber <afaerber@suse.de>

however I do not agree with the goal in the subject. I thought this was
to match C++ in actually deallocating the memory.

Andreas
Paolo Bonzini - Nov. 23, 2012, 5:10 p.m.
Il 23/11/2012 18:02, Andreas Färber ha scritto:
>> Store in the object the freeing function that will be used at deletion
>> > time.  This makes it possible to use object_delete on statically-allocated
>> > (embedded) objects.  Dually, it makes it possible to use object_unparent
>> > and object_unref without leaking memory, when the lifetime of object
>> > might extend until after the call to object_delete.
>> > 
>> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> The code is
> 
> Reviewed-by: Andreas Färber <afaerber@suse.de>
> 
> however I do not agree with the goal in the subject. I thought this was
> to match C++ in actually deallocating the memory.

At the "real" end of the series object_delete disappears completely.  I
posted only the initial part because the rest is not appropriate for
1.3, you can see it at https://github.com/bonzini/qemu/commits/qdev-free.

You just have ref/unref to keep an object alive, and unparent to remove it.

qbus_free and qdev_free become simply synonyms for object_unparent.
After the unparent, the refcount is magically zero for the bus/device
and everything below it in the qtree, and they disappear.

Paolo

Patch

diff --git a/include/qemu/object.h b/include/qemu/object.h
index 232463b..5ddcb4a 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -239,6 +239,14 @@  typedef struct ObjectProperty
 typedef void (ObjectUnparent)(Object *obj);
 
 /**
+ * ObjectFree:
+ * @obj: the object being freed
+ *
+ * Called when an object's last reference is removed.
+ */
+typedef void (ObjectFree)(void *obj);
+
+/**
  * ObjectClass:
  *
  * The base for all classes.  The only thing that #ObjectClass contains is an
@@ -272,6 +280,7 @@  struct Object
 {
     /*< private >*/
     ObjectClass *class;
+    ObjectFree *free;
     QTAILQ_HEAD(, ObjectProperty) properties;
     uint32_t ref;
     Object *parent;
diff --git a/qom/object.c b/qom/object.c
index f4747d0..f3e9517 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -388,6 +388,9 @@  void object_finalize(void *data)
     object_property_del_all(obj);
 
     g_assert(obj->ref == 0);
+    if (obj->free) {
+        obj->free(obj);
+    }
 }
 
 Object *object_new_with_type(Type type)
@@ -399,6 +402,7 @@  Object *object_new_with_type(Type type)
 
     obj = g_malloc(type->instance_size);
     object_initialize_with_type(obj, type);
+    obj->free = g_free;
 
     return obj;
 }
@@ -415,7 +419,6 @@  void object_delete(Object *obj)
     object_unparent(obj);
     g_assert(obj->ref == 1);
     object_unref(obj);
-    g_free(obj);
 }
 
 Object *object_dynamic_cast(Object *obj, const char *typename)