From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39337) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c7Nwp-0006Yo-EH for qemu-devel@nongnu.org; Thu, 17 Nov 2016 09:45:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c7Nwm-0007R2-CD for qemu-devel@nongnu.org; Thu, 17 Nov 2016 09:45:55 -0500 Received: from mx1.redhat.com ([209.132.183.28]:53426) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c7Nwm-0007QR-4H for qemu-devel@nongnu.org; Thu, 17 Nov 2016 09:45:52 -0500 From: Markus Armbruster References: <1479322664-2253-1-git-send-email-ehabkost@redhat.com> <1479322664-2253-2-git-send-email-ehabkost@redhat.com> <8737iql201.fsf@dusky.pond.sub.org> <20161117133623.GU5057@thinpad.lan.raisama.net> Date: Thu, 17 Nov 2016 15:45:49 +0100 In-Reply-To: <20161117133623.GU5057@thinpad.lan.raisama.net> (Eduardo Habkost's message of "Thu, 17 Nov 2016 11:36:23 -0200") Message-ID: <871syaf9lu.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH for-2.9 1/2] qom: Remove release function from class properties List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: qemu-devel@nongnu.org, Andreas =?utf-8?Q?F=C3=A4rber?= Eduardo Habkost writes: > On Thu, Nov 17, 2016 at 01:33:34PM +0100, Markus Armbruster wrote: >> Eduardo Habkost writes: >> >> > The release functions are never called for class properties, and >> > their semantics aren't even defined clearly (should the release >> > function be called when an instance is destroyed, or when a class >> > is destroyed?). Remove the unused functionality. >> > >> > Signed-off-by: Eduardo Habkost > [...] >> > g_hash_table_insert(klass->properties, g_strdup(name), prop); >> > @@ -1808,7 +1806,6 @@ void object_class_property_add_str(ObjectClass *klass, const char *name, >> > object_class_property_add(klass, name, "string", >> > get ? property_get_str : NULL, >> > set ? property_set_str : NULL, >> > - property_release_str, >> > prop, &local_err); >> > if (local_err) { >> > error_propagate(errp, local_err); >> >> Here, you drop property_release_str(), which calls g_free(). Assuming >> you claim that it's never called is correct, this is again no functional >> change. But that begs the question why not freeing the stuff >> property_release_str() frees is correct. Can you explain? > > property_release_str() frees prop. The only right moment to free > prop is on class destruction or class property removal, but we > have no mechanisms for class destruction or class property > removal today. Would be a nice addition to the commit message. >> > @@ -1897,7 +1894,6 @@ void object_class_property_add_bool(ObjectClass *klass, const char *name, >> > object_class_property_add(klass, name, "bool", >> > get ? property_get_bool : NULL, >> > set ? property_set_bool : NULL, >> > - property_release_bool, >> > prop, &local_err); >> > if (local_err) { >> > error_propagate(errp, local_err); >> > @@ -1985,7 +1981,6 @@ void object_class_property_add_enum(ObjectClass *klass, const char *name, >> > object_class_property_add(klass, name, typename, >> > get ? property_get_enum : NULL, >> > set ? property_set_enum : NULL, >> > - property_release_enum, >> > prop, &local_err); >> > if (local_err) { >> > error_propagate(errp, local_err); >> > @@ -2082,7 +2077,6 @@ void object_class_property_add_tm(ObjectClass *klass, const char *name, >> > >> > object_class_property_add(klass, name, "struct tm", >> > get ? property_get_tm : NULL, NULL, >> > - property_release_tm, >> > prop, &local_err); >> > if (local_err) { >> > error_propagate(errp, local_err); >> >> Likewise. > > Same as above. > >> [...] Reviewed-by: Markus Armbruster