From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53132) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X2F98-0005nm-0H for qemu-devel@nongnu.org; Wed, 02 Jul 2014 03:40:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X2F91-00041j-MS for qemu-devel@nongnu.org; Wed, 02 Jul 2014 03:40:01 -0400 Message-ID: <53B3B748.4090301@suse.de> Date: Wed, 02 Jul 2014 09:39:52 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1404251378-5242-1-git-send-email-agraf@suse.de> <1404251378-5242-2-git-send-email-agraf@suse.de> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/6] qom: macroify integer property helpers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: Peter Maydell , Eric Auger , "qemu-devel@nongnu.org Developers" , qemu-ppc Mailing List , sean.stalley@intel.com, Paolo Bonzini , =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= On 02.07.14 05:29, Peter Crosthwaite wrote: > On Wed, Jul 2, 2014 at 7:49 AM, Alexander Graf wrote: >> We have a bunch of nice helpers that allow us to easily register an integer >> field as QOM property. However, we have those duplicated for every integer >> size available. >> >> This is very cumbersome (and prone to bugs) to work with and extend, so let's >> strip out the only difference there is (the size) and generate the actual >> functions via a macro. >> >> Signed-off-by: Alexander Graf >> --- >> qom/object.c | 83 ++++++++++++++++++------------------------------------------ >> 1 file changed, 24 insertions(+), 59 deletions(-) >> >> diff --git a/qom/object.c b/qom/object.c >> index 0e8267b..73cd717 100644 >> --- a/qom/object.c >> +++ b/qom/object.c >> @@ -1507,65 +1507,30 @@ static char *qdev_get_type(Object *obj, Error **errp) >> return g_strdup(object_get_typename(obj)); >> } >> >> -static void property_get_uint8_ptr(Object *obj, Visitor *v, >> - void *opaque, const char *name, >> - Error **errp) >> -{ >> - uint8_t value = *(uint8_t *)opaque; >> - visit_type_uint8(v, &value, name, errp); >> -} >> - >> -static void property_get_uint16_ptr(Object *obj, Visitor *v, >> - void *opaque, const char *name, >> - Error **errp) >> -{ >> - uint16_t value = *(uint16_t *)opaque; >> - visit_type_uint16(v, &value, name, errp); >> -} >> - >> -static void property_get_uint32_ptr(Object *obj, Visitor *v, >> - void *opaque, const char *name, >> - Error **errp) >> -{ >> - uint32_t value = *(uint32_t *)opaque; >> - visit_type_uint32(v, &value, name, errp); >> -} >> - >> -static void property_get_uint64_ptr(Object *obj, Visitor *v, >> - void *opaque, const char *name, >> - Error **errp) >> -{ >> - uint64_t value = *(uint64_t *)opaque; >> - visit_type_uint64(v, &value, name, errp); >> -} >> - >> -void object_property_add_uint8_ptr(Object *obj, const char *name, >> - const uint8_t *v, Error **errp) >> -{ >> - object_property_add(obj, name, "uint8", property_get_uint8_ptr, >> - NULL, NULL, (void *)v, errp); >> -} >> - >> -void object_property_add_uint16_ptr(Object *obj, const char *name, >> - const uint16_t *v, Error **errp) >> -{ >> - object_property_add(obj, name, "uint16", property_get_uint16_ptr, >> - NULL, NULL, (void *)v, errp); >> -} >> - >> -void object_property_add_uint32_ptr(Object *obj, const char *name, >> - const uint32_t *v, Error **errp) >> -{ >> - object_property_add(obj, name, "uint32", property_get_uint32_ptr, >> - NULL, NULL, (void *)v, errp); >> -} >> - >> -void object_property_add_uint64_ptr(Object *obj, const char *name, >> - const uint64_t *v, Error **errp) >> -{ >> - object_property_add(obj, name, "uint64", property_get_uint64_ptr, >> - NULL, NULL, (void *)v, errp); >> -} >> +#define DECLARE_INTEGER_VISITOR(size) \ > macro needs a better name. DECLARE_PROP_SET_GET - something to make it > clear its about properties. > >> + \ >> +static void glue(glue(property_get_,size),_ptr)(Object *obj, Visitor *v, \ >> + void *opaque, \ >> + const char *name, \ >> + Error **errp) \ >> +{ \ >> + glue(size,_t) value = *(glue(size,_t) *)opaque; \ >> + glue(visit_type_,size)(v, &value, name, errp); \ >> +} \ >> + \ >> +void glue(glue(object_property_add_,size),_ptr)(Object *obj, const char *name, \ >> + const glue(size,_t) *v, \ >> + Error **errp) \ >> +{ \ >> + ObjectPropertyAccessor *get = glue(glue(property_get_,size),_ptr); \ >> + object_property_add(obj, name, stringify(size), get, NULL, NULL, (void *)v,\ >> + errp); \ >> +} \ >> + >> +DECLARE_INTEGER_VISITOR(uint8) >> +DECLARE_INTEGER_VISITOR(uint16) >> +DECLARE_INTEGER_VISITOR(uint32) >> +DECLARE_INTEGER_VISITOR(uint64) >> > Worth getting bool working this way too? Theres been a few times now > where I have wanted to object_property_add_bool_ptr. Perhaps: > > #define DECLARE_PROP_SET_GET(name, type) > > DECLARE_PROP_SET_GET(uint8, uint8_t) > DECLARE_PROP_SET_GET(uint16, uint16_t) > DECLARE_PROP_SET_GET(uint32, uint32_t) > DECLARE_PROP_SET_GET(uint64, uint64_t) > DECLARE_PROP_SET_GET(bool, bool) > > Can actually add the bool one later in follow-up patch but just > thinking ahead to get the macro right for wider usage. I think that makes sense. Let me change it :). Alex