Skip to contentSkip to conversation

Replace object_delete() with object_unref().

PATCH In review Alexander Barabash 3 participants 3 messages Feb 16, 2012 – Feb 17, 2012

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Replace object_delete() with object_unref().

Replace object_delete() with object_unref().

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 one (1) causes program failure.

In the existing implementation, object_unref() finalizes the object when its reference count becomes zero (0).

In the new implementation, object_unref()
finalizes and frees the object's storage when the reference count
becomes zero (0).

One usage of object_delete() replaced by object_unref().

hw/qdev.c +1 -1
index f0eb3a7..891981a 100644
@@ -247,7 +247,7 @@ void qdev_init_nofail(DeviceState *dev)
 /* Unlink device from bus and free the structure.  */
 void qdev_free(DeviceState *dev)
 {
-    object_delete(OBJECT(dev));
+    object_unref(OBJECT(dev));
 }
 
 void qdev_machine_creation_done(void)
include/qemu/object.h +9 -14
index 69cc2ab..e7e32fe 100644
@@ -415,8 +415,9 @@ struct InterfaceInfo
  * object_new:
  * @typename: The name of the type of the object to instantiate.
  *
- * This function will initialize a new object using heap allocated memory.  This
- * function should be paired with object_delete() to free the resources
+ * This function will initialize a new object using heap allocated memory.
+ * The object's reference count will be set to one (1) upon successful completion.
+ * This function should be paired with object_unref() to free the resources
  * associated with the object.
  *
  * Returns: The newly allocated and instantiated object.
@@ -427,8 +428,9 @@ Object *object_new(const char *typename);
  * object_new_with_type:
  * @type: The type of the object to instantiate.
  *
- * This function will initialize a new object using heap allocated memory.  This
- * function should be paired with object_delete() to free the resources
+ * This function will initialize a new object using heap allocated memory.
+ * The object's reference count will be set to one (1) upon successful completion.
+ * This function should be paired with object_unref() to free the resources
  * associated with the object.
  *
  * Returns: The newly allocated and instantiated object.
@@ -436,15 +438,6 @@ Object *object_new(const char *typename);
 Object *object_new_with_type(Type type);
 
 /**
- * object_delete:
- * @obj: The object to free.
- *
- * Finalize an object and then free the memory associated with it.  This should
- * be paired with object_new() to free the resources associated with an object.
- */
-void object_delete(Object *obj);
-
-/**
  * object_initialize_with_type:
  * @obj: A pointer to the memory to be used for the object.
  * @type: The type of the object to instantiate.
@@ -573,8 +566,10 @@ void object_ref(Object *obj);
  * qdef_unref:
  * @obj: the object
  *
- * Decrease the reference count of a object.  A object cannot be freed as long
+ * Decrease the reference count of a object.  A object is not freed as long
  * as its reference count is greater than zero.
+ * Once an object's reference count gets to zero (0),
+ * the object is finalized and the memory associated with it is freed.
  */
 void object_unref(Object *obj);
 
qom/object.c +0 -0
index 0cbd9bb..2de6eaf 100644
@@ -328,7 +328,7 @@ static void object_deinit(Object *obj, TypeImpl *type)
     while (obj->interfaces) {
         Interface *iface_obj = obj->interfaces->data;
         obj->interfaces = g_slist_delete_link(obj->interfaces, 

obj->interfaces);
- object_delete(OBJECT(iface_obj));

+        object_unref(OBJECT(iface_obj));
     }

     if (type_has_parent(type)) {
@@ -369,13 +369,6 @@ Object *object_new(const char *typename)
     return object_new_with_type(ti);

}

-void object_delete(Object *obj)
-{

-    object_unref(obj);
-    g_assert(obj->ref == 0);
-    g_free(obj);
-}
-
 static bool type_is_ancestor(TypeImpl *type, TypeImpl *target_type)
 {
     assert(target_type);
@@ -583,6 +576,7 @@ void object_unref(Object *obj)
     /* parent always holds a reference to its children */
     if (obj->ref == 0) {
         object_finalize(obj);
+        g_free(obj);
     }

}

#1 lore raw

Object model lifecycle changes aren't trivial IMHO. [no comment on actual patch contents implied]

-- PMM

Quoted 133 lines · Alexander Barabash

   Replace object_delete() with object_unref().

   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 one (1) causes program failure.

   In the existing implementation, object_unref() finalizes the object when its reference count becomes zero (0).

   In the new implementation, object_unref()
   finalizes and frees the object's storage when the reference count becomes zero (0).

   One usage of object_delete() replaced by object_unref().

hw/qdev.c +1 -1
index f0eb3a7..891981a 100644
@@ -247,7 +247,7 @@ void qdev_init_nofail(DeviceState *dev)
  /* Unlink device from bus and free the structure.  */
  void qdev_free(DeviceState *dev)
  {
-    object_delete(OBJECT(dev));
+    object_unref(OBJECT(dev));
  }
 
  void qdev_machine_creation_done(void)
include/qemu/object.h +0 -1
index 69cc2ab..e7e32fe 100644
@@ -415,8 +415,9 @@ struct InterfaceInfo
  * object_new:
  * @typename: The name of the type of the object to instantiate.
  *
- * This function will initialize a new object using heap allocated memory.

 This
- * function should be paired with object_delete() to free the resources
+ * This function will initialize a new object using heap allocated memory.
+ * The object's reference count will be set to one (1) upon successful completion.
+ * This function should be paired with object_unref() to free the resources
 * associated with the object.
 *
 * Returns: The newly allocated and instantiated object.

include/qemu/object.h +0 -1
@@ -427,8 +428,9 @@ Object *object_new(const char *typename);
  * object_new_with_type:
  * @type: The type of the object to instantiate.
  *
- * This function will initialize a new object using heap allocated memory.

 This
- * function should be paired with object_delete() to free the resources
+ * This function will initialize a new object using heap allocated memory.
+ * The object's reference count will be set to one (1) upon successful completion.
+ * This function should be paired with object_unref() to free the resources
 * associated with the object.
 *
 * Returns: The newly allocated and instantiated object.

include/qemu/object.h +0 -4
@@ -436,15 +438,6 @@ Object *object_new(const char *typename);
  Object *object_new_with_type(Type type);
 
  /**
- * object_delete:
- * @obj: The object to free.
- *
- * Finalize an object and then free the memory associated with it.  This

should
- * be paired with object_new() to free the resources associated with an object.
- */

include/qemu/object.h +0 -3
-void object_delete(Object *obj);
-
-/**

 * object_initialize_with_type:
 * @obj: A pointer to the memory to be used for the object.
 * @type: The type of the object to instantiate.

include/qemu/object.h +0 -1
@@ -573,8 +566,10 @@ void object_ref(Object *obj);
  * qdef_unref:
  * @obj: the object
  *
- * Decrease the reference count of a object.  A object cannot be freed as

long
+ * Decrease the reference count of a object.  A object is not freed as long
 * as its reference count is greater than zero.
+ * Once an object's reference count gets to zero (0),
+ * the object is finalized and the memory associated with it is freed. */
 void object_unref(Object *obj);

qom/object.c +1 -1
index 0cbd9bb..2de6eaf 100644
@@ -328,7 +328,7 @@ static void object_deinit(Object *obj, TypeImpl *type)
     while (obj->interfaces) {
         Interface *iface_obj = obj->interfaces->data;
         obj->interfaces = g_slist_delete_link(obj->interfaces,
 obj->interfaces);
-        object_delete(OBJECT(iface_obj));
+        object_unref(OBJECT(iface_obj));
     }
 

    if (type_has_parent(type)) {

qom/object.c +1 -7
@@ -369,13 +369,6 @@ Object *object_new(const char *typename)
     return object_new_with_type(ti);
  }
 
-void object_delete(Object *obj)
-{
-    object_unref(obj);
-    g_assert(obj->ref == 0);
-    g_free(obj);
-}
-
  static bool type_is_ancestor(TypeImpl *type, TypeImpl *target_type)
  {
     assert(target_type);
@@ -583,6 +576,7 @@ void object_unref(Object *obj)
     /* parent always holds a reference to its children */
     if (obj->ref == 0) {
         object_finalize(obj);
+        g_free(obj);
     }
  }
 
#2 lore raw
Quoted 2 lines · Peter Maydell

Object model lifecycle changes aren't trivial IMHO. [no comment on actual patch contents implied]

Agreed.

Stefan