diff mbox

[4/9] object: remove object_finalize

Message ID 1345996298-4892-5-git-send-email-aliguori@us.ibm.com
State New
Headers show

Commit Message

Anthony Liguori Aug. 26, 2012, 3:51 p.m. UTC
Callers should just use object_unref

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/qdev.c             |    4 ----
 include/qemu/object.h |    9 ---------
 qom/object.c          |    2 +-
 3 files changed, 1 insertions(+), 14 deletions(-)

Comments

Andreas Färber Aug. 27, 2012, 3:01 p.m. UTC | #1
Am 26.08.2012 17:51, schrieb Anthony Liguori:
> Callers should just use object_unref
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  hw/qdev.c             |    4 ----
>  include/qemu/object.h |    9 ---------
>  qom/object.c          |    2 +-
>  3 files changed, 1 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 6b61daa..fdee91f 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -678,13 +678,9 @@ static void device_initfn(Object *obj)
>  static void device_finalize(Object *obj)
>  {
>      DeviceState *dev = DEVICE(obj);
> -    BusState *bus;
>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
>  
>      if (dev->state == DEV_STATE_INITIALIZED) {
> -        while (dev->num_child_bus) {
> -            bus = QLIST_FIRST(&dev->child_bus);
> -        }
>          if (qdev_get_vmsd(dev)) {
>              vmstate_unregister(dev, qdev_get_vmsd(dev), dev);
>          }

This seems to answer my remark on 3/9, should've been squashed into that
one.

> diff --git a/include/qemu/object.h b/include/qemu/object.h
> index 487adcd..8bc9935 100644
> --- a/include/qemu/object.h
> +++ b/include/qemu/object.h
> @@ -490,15 +490,6 @@ void object_initialize_with_type(void *data, Type type);
>  void object_initialize(void *obj, const char *typename);
>  
>  /**
> - * object_finalize:
> - * @obj: The object to finalize.
> - *
> - * This function destroys and object without freeing the memory associated with
> - * it.
> - */
> -void object_finalize(void *obj);
> -
> -/**
>   * object_dynamic_cast:
>   * @obj: The object to cast.
>   * @typename: The @typename to cast to.
> diff --git a/qom/object.c b/qom/object.c
> index 44135c3..1144f79 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -375,7 +375,7 @@ static void object_deinit(Object *obj, TypeImpl *type)
>      }
>  }
>  
> -void object_finalize(void *data)
> +static void object_finalize(void *data)
>  {
>      Object *obj = data;
>      TypeImpl *ti = obj->class->type;

This is what I was referring to with breaking the encapsulation on 3/9:
When we have a PHB with embedded PCIDevice on its PCIBus, as
demonstrated with i440fx and prep_pci, then when doing object_delete()
on the whole thing I expect the main object's finalizer to call
object_finalize() on its embedded childs, forcing their uninit or an
assert if a programming error. Not just an unref that might or might not
finalize it.

If however finalize is called only at refcount 0 then who will unref the
self-created children? Finalize would never be called due to pending
references by its children...

Andreas
Anthony Liguori Aug. 27, 2012, 3:49 p.m. UTC | #2
Andreas Färber <afaerber@suse.de> writes:

> Am 26.08.2012 17:51, schrieb Anthony Liguori:
>> Callers should just use object_unref
>> 
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> ---
>>  hw/qdev.c             |    4 ----
>>  include/qemu/object.h |    9 ---------
>>  qom/object.c          |    2 +-
>>  3 files changed, 1 insertions(+), 14 deletions(-)
>> 
>> diff --git a/hw/qdev.c b/hw/qdev.c
>> index 6b61daa..fdee91f 100644
>> --- a/hw/qdev.c
>> +++ b/hw/qdev.c
>> @@ -678,13 +678,9 @@ static void device_initfn(Object *obj)
>>  static void device_finalize(Object *obj)
>>  {
>>      DeviceState *dev = DEVICE(obj);
>> -    BusState *bus;
>>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
>>  
>>      if (dev->state == DEV_STATE_INITIALIZED) {
>> -        while (dev->num_child_bus) {
>> -            bus = QLIST_FIRST(&dev->child_bus);
>> -        }
>>          if (qdev_get_vmsd(dev)) {
>>              vmstate_unregister(dev, qdev_get_vmsd(dev), dev);
>>          }
>
> This seems to answer my remark on 3/9, should've been squashed into that
> one.
>
>> diff --git a/include/qemu/object.h b/include/qemu/object.h
>> index 487adcd..8bc9935 100644
>> --- a/include/qemu/object.h
>> +++ b/include/qemu/object.h
>> @@ -490,15 +490,6 @@ void object_initialize_with_type(void *data, Type type);
>>  void object_initialize(void *obj, const char *typename);
>>  
>>  /**
>> - * object_finalize:
>> - * @obj: The object to finalize.
>> - *
>> - * This function destroys and object without freeing the memory associated with
>> - * it.
>> - */
>> -void object_finalize(void *obj);
>> -
>> -/**
>>   * object_dynamic_cast:
>>   * @obj: The object to cast.
>>   * @typename: The @typename to cast to.
>> diff --git a/qom/object.c b/qom/object.c
>> index 44135c3..1144f79 100644
>> --- a/qom/object.c
>> +++ b/qom/object.c
>> @@ -375,7 +375,7 @@ static void object_deinit(Object *obj, TypeImpl *type)
>>      }
>>  }
>>  
>> -void object_finalize(void *data)
>> +static void object_finalize(void *data)
>>  {
>>      Object *obj = data;
>>      TypeImpl *ti = obj->class->type;
>
> This is what I was referring to with breaking the encapsulation on 3/9:
> When we have a PHB with embedded PCIDevice on its PCIBus, as
> demonstrated with i440fx and prep_pci, then when doing object_delete()
> on the whole thing I expect the main object's finalizer to call
> object_finalize() on its embedded childs, forcing their uninit or an
> assert if a programming error. Not just an unref that might or might not
> finalize it.
>
> If however finalize is called only at refcount 0 then who will unref the
> self-created children? Finalize would never be called due to pending
> references by its children...

The refs are stored as properties and when properties are removed during
object destruction, those references are dropped.

That is what takes the refcount to 0.  We could add a
g_assert(child->ref == 1) before removing the property to catch
programming errors.

Regards,

Anthony Liguori

>
> Andreas
>
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
diff mbox

Patch

diff --git a/hw/qdev.c b/hw/qdev.c
index 6b61daa..fdee91f 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -678,13 +678,9 @@  static void device_initfn(Object *obj)
 static void device_finalize(Object *obj)
 {
     DeviceState *dev = DEVICE(obj);
-    BusState *bus;
     DeviceClass *dc = DEVICE_GET_CLASS(dev);
 
     if (dev->state == DEV_STATE_INITIALIZED) {
-        while (dev->num_child_bus) {
-            bus = QLIST_FIRST(&dev->child_bus);
-        }
         if (qdev_get_vmsd(dev)) {
             vmstate_unregister(dev, qdev_get_vmsd(dev), dev);
         }
diff --git a/include/qemu/object.h b/include/qemu/object.h
index 487adcd..8bc9935 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -490,15 +490,6 @@  void object_initialize_with_type(void *data, Type type);
 void object_initialize(void *obj, const char *typename);
 
 /**
- * object_finalize:
- * @obj: The object to finalize.
- *
- * This function destroys and object without freeing the memory associated with
- * it.
- */
-void object_finalize(void *obj);
-
-/**
  * object_dynamic_cast:
  * @obj: The object to cast.
  * @typename: The @typename to cast to.
diff --git a/qom/object.c b/qom/object.c
index 44135c3..1144f79 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -375,7 +375,7 @@  static void object_deinit(Object *obj, TypeImpl *type)
     }
 }
 
-void object_finalize(void *data)
+static void object_finalize(void *data)
 {
     Object *obj = data;
     TypeImpl *ti = obj->class->type;