From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36619) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c7Lss-0005kJ-8E for qemu-devel@nongnu.org; Thu, 17 Nov 2016 07:33:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c7Lso-0006Vv-0q for qemu-devel@nongnu.org; Thu, 17 Nov 2016 07:33:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48848) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c7Lsn-0006Vi-P9 for qemu-devel@nongnu.org; Thu, 17 Nov 2016 07:33:37 -0500 From: Markus Armbruster References: <1479322664-2253-1-git-send-email-ehabkost@redhat.com> <1479322664-2253-2-git-send-email-ehabkost@redhat.com> Date: Thu, 17 Nov 2016 13:33:34 +0100 In-Reply-To: <1479322664-2253-2-git-send-email-ehabkost@redhat.com> (Eduardo Habkost's message of "Wed, 16 Nov 2016 16:57:43 -0200") Message-ID: <8737iql201.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: > 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 > --- > backends/hostmem.c | 4 ++-- > hw/core/machine.c | 6 +++--- > hw/i386/pc.c | 8 ++++---- > hw/ppc/pnv.c | 2 +- > include/qom/object.h | 1 - > qom/object.c | 14 ++++---------- > 6 files changed, 14 insertions(+), 21 deletions(-) > > diff --git a/backends/hostmem.c b/backends/hostmem.c > index 4256d24..856e96e 100644 > --- a/backends/hostmem.c > +++ b/backends/hostmem.c > @@ -368,11 +368,11 @@ host_memory_backend_class_init(ObjectClass *oc, void *data) > object_class_property_add(oc, "size", "int", > host_memory_backend_get_size, > host_memory_backend_set_size, > - NULL, NULL, &error_abort); > + NULL, &error_abort); > object_class_property_add(oc, "host-nodes", "int", > host_memory_backend_get_host_nodes, > host_memory_backend_set_host_nodes, > - NULL, NULL, &error_abort); > + NULL, &error_abort); > object_class_property_add_enum(oc, "policy", "HostMemPolicy", > HostMemPolicy_lookup, > host_memory_backend_get_policy, > diff --git a/hw/core/machine.c b/hw/core/machine.c > index b0fd91f..c64e5f1 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -372,13 +372,13 @@ static void machine_class_init(ObjectClass *oc, void *data) > > object_class_property_add(oc, "kernel-irqchip", "OnOffSplit", > NULL, machine_set_kernel_irqchip, > - NULL, NULL, &error_abort); > + NULL, &error_abort); > object_class_property_set_description(oc, "kernel-irqchip", > "Configure KVM in-kernel irqchip", &error_abort); > > object_class_property_add(oc, "kvm-shadow-mem", "int", > machine_get_kvm_shadow_mem, machine_set_kvm_shadow_mem, > - NULL, NULL, &error_abort); > + NULL, &error_abort); > object_class_property_set_description(oc, "kvm-shadow-mem", > "KVM shadow MMU size", &error_abort); > > @@ -409,7 +409,7 @@ static void machine_class_init(ObjectClass *oc, void *data) > > object_class_property_add(oc, "phandle-start", "int", > machine_get_phandle_start, machine_set_phandle_start, > - NULL, NULL, &error_abort); > + NULL, &error_abort); > object_class_property_set_description(oc, "phandle-start", > "The first phandle ID we may generate dynamically", &error_abort); > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index a9b1950..46f95bf 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -2308,24 +2308,24 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) > > object_class_property_add(oc, PC_MACHINE_MEMHP_REGION_SIZE, "int", > pc_machine_get_hotplug_memory_region_size, NULL, > - NULL, NULL, &error_abort); > + NULL, &error_abort); > > object_class_property_add(oc, PC_MACHINE_MAX_RAM_BELOW_4G, "size", > pc_machine_get_max_ram_below_4g, pc_machine_set_max_ram_below_4g, > - NULL, NULL, &error_abort); > + NULL, &error_abort); > > object_class_property_set_description(oc, PC_MACHINE_MAX_RAM_BELOW_4G, > "Maximum ram below the 4G boundary (32bit boundary)", &error_abort); > > object_class_property_add(oc, PC_MACHINE_SMM, "OnOffAuto", > pc_machine_get_smm, pc_machine_set_smm, > - NULL, NULL, &error_abort); > + NULL, &error_abort); > object_class_property_set_description(oc, PC_MACHINE_SMM, > "Enable SMM (pc & q35)", &error_abort); > > object_class_property_add(oc, PC_MACHINE_VMPORT, "OnOffAuto", > pc_machine_get_vmport, pc_machine_set_vmport, > - NULL, NULL, &error_abort); > + NULL, &error_abort); > object_class_property_set_description(oc, PC_MACHINE_VMPORT, > "Enable vmport (pc & q35)", &error_abort); > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index 9df7b25..3fb68c3 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -777,7 +777,7 @@ static void powernv_machine_class_props_init(ObjectClass *oc) > { > object_class_property_add(oc, "num-chips", "uint32_t", > pnv_get_num_chips, pnv_set_num_chips, > - NULL, NULL, NULL); > + NULL, NULL); > object_class_property_set_description(oc, "num-chips", > "Specifies the number of processor chips", > NULL); The above all drop null releases. No functional change. > diff --git a/include/qom/object.h b/include/qom/object.h > index 5ecc2d1..fbf9df2 100644 > --- a/include/qom/object.h > +++ b/include/qom/object.h > @@ -945,7 +945,6 @@ ObjectProperty *object_class_property_add(ObjectClass *klass, const char *name, > const char *type, > ObjectPropertyAccessor *get, > ObjectPropertyAccessor *set, > - ObjectPropertyRelease *release, > void *opaque, Error **errp); > > /** > diff --git a/qom/object.c b/qom/object.c > index 7a05e35..c6c0255 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -955,7 +955,6 @@ object_class_property_add(ObjectClass *klass, > const char *type, > ObjectPropertyAccessor *get, > ObjectPropertyAccessor *set, > - ObjectPropertyRelease *release, > void *opaque, > Error **errp) > { > @@ -975,7 +974,6 @@ object_class_property_add(ObjectClass *klass, > > prop->get = get; > prop->set = set; > - prop->release = release; > prop->opaque = opaque; > > 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? > @@ -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. > @@ -2134,7 +2128,7 @@ void object_class_property_add_uint8_ptr(ObjectClass *klass, const char *name, > const uint8_t *v, Error **errp) > { > object_class_property_add(klass, name, "uint8", property_get_uint8_ptr, > - NULL, NULL, (void *)v, errp); > + NULL, (void *)v, errp); > } > > void object_property_add_uint16_ptr(Object *obj, const char *name, > @@ -2148,7 +2142,7 @@ void object_class_property_add_uint16_ptr(ObjectClass *klass, const char *name, > const uint16_t *v, Error **errp) > { > object_class_property_add(klass, name, "uint16", property_get_uint16_ptr, > - NULL, NULL, (void *)v, errp); > + NULL, (void *)v, errp); > } > > void object_property_add_uint32_ptr(Object *obj, const char *name, > @@ -2162,7 +2156,7 @@ void object_class_property_add_uint32_ptr(ObjectClass *klass, const char *name, > const uint32_t *v, Error **errp) > { > object_class_property_add(klass, name, "uint32", property_get_uint32_ptr, > - NULL, NULL, (void *)v, errp); > + NULL, (void *)v, errp); > } > > void object_property_add_uint64_ptr(Object *obj, const char *name, > @@ -2176,7 +2170,7 @@ void object_class_property_add_uint64_ptr(ObjectClass *klass, const char *name, > const uint64_t *v, Error **errp) > { > object_class_property_add(klass, name, "uint64", property_get_uint64_ptr, > - NULL, NULL, (void *)v, errp); > + NULL, (void *)v, errp); > } > > typedef struct { More null releases.