Patchwork [TRIVIAL] Replace object_delete() with object_unref().

login
register
mail settings
Submitter Alexander Barabash
Date Feb. 16, 2012, 5:38 p.m.
Message ID <4F3D3F26.8010402@mentor.com>
Download mbox | patch
Permalink /patch/141657/
State New
Headers show

Comments

Alexander Barabash - Feb. 16, 2012, 5:38 p.m.
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().

     Signed-off-by: Alexander Barabash <alexander_barabash@mentor.com>

      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);
      }
  }
Peter Maydell - Feb. 16, 2012, 6:12 p.m.
Object model lifecycle changes aren't trivial IMHO.
[no comment on actual patch contents implied]

-- PMM

On 16 February 2012 17:38, Alexander Barabash
<alexander_barabash@mentor.com> wrote:
>
>    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().
>
>    Signed-off-by: Alexander Barabash <alexander_barabash@mentor.com>
>
> diff --git a/hw/qdev.c b/hw/qdev.c
> index f0eb3a7..891981a 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -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)
> diff --git a/include/qemu/object.h b/include/qemu/object.h
> index 69cc2ab..e7e32fe 100644
> --- a/include/qemu/object.h
> +++ b/include/qemu/object.h
> @@ -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);
>
> diff --git a/qom/object.c b/qom/object.c
> index 0cbd9bb..2de6eaf 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -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);
>     }
>  }
>
>
>
Stefan Hajnoczi - Feb. 17, 2012, 10:57 a.m.
On Thu, Feb 16, 2012 at 6:12 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> Object model lifecycle changes aren't trivial IMHO.
> [no comment on actual patch contents implied]

Agreed.

Stefan

Patch

diff --git a/hw/qdev.c b/hw/qdev.c
index f0eb3a7..891981a 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -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)
diff --git a/include/qemu/object.h b/include/qemu/object.h
index 69cc2ab..e7e32fe 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -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);

diff --git a/qom/object.c b/qom/object.c
index 0cbd9bb..2de6eaf 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -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));
      }