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

[Alexander Barabash <alexander_barabash@mentor.com>]

On 02/22/2012 09:12 PM, Anthony Liguori wrote:
> On 02/22/2012 12:00 PM, alexander_barabash@mentor.com wrote:
>> From: Alexander Barabash<alexander_barabash@mentor.com>
>>
>> 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<alexander_barabash@mentor.com>
>> ---
>>   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);
>>       }
>>   }
>>
>