| Message ID | 1404251378-5242-2-git-send-email-agraf@suse.de |
|---|---|
| State | New |
| Headers | show |
On Wed, Jul 2, 2014 at 7:49 AM, Alexander Graf <agraf@suse.de> 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 <agraf@suse.de> > --- > 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. Regards, Peter > typedef struct { > Object *target_obj; > -- > 1.8.1.4 > >
On 02.07.14 05:29, Peter Crosthwaite wrote: > On Wed, Jul 2, 2014 at 7:49 AM, Alexander Graf <agraf@suse.de> 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 <agraf@suse.de> >> --- >> 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
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) \ + \ +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) typedef struct { Object *target_obj;
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 <agraf@suse.de> --- qom/object.c | 83 ++++++++++++++++++------------------------------------------ 1 file changed, 24 insertions(+), 59 deletions(-)