From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:48273) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S0bQ9-00006o-BG for qemu-devel@nongnu.org; Thu, 23 Feb 2012 11:21:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S0bQ5-0001Ba-75 for qemu-devel@nongnu.org; Thu, 23 Feb 2012 11:21:29 -0500 Received: from relay1.mentorg.com ([192.94.38.131]:65300) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S0bQ4-0001BI-SY for qemu-devel@nongnu.org; Thu, 23 Feb 2012 11:21:25 -0500 Message-ID: <4F46679C.2080301@mentor.com> Date: Thu, 23 Feb 2012 18:21:48 +0200 From: Alexander Barabash MIME-Version: 1.0 References: <1329933650-30419-1-git-send-email-alexander_barabash@mentor.com> <4F453E1A.9030701@codemonkey.ws> In-Reply-To: <4F453E1A.9030701@codemonkey.ws> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] qom: Make object_unref() free the object's memory when refcount goes to 0. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: pbonzini@redhat.com, qemu-devel@nongnu.org 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 >> >> 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 >> --- >> 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); >> } >> } >> >