From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42029) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yj7lv-0000a8-Qz for qemu-devel@nongnu.org; Fri, 17 Apr 2015 11:01:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yj7lr-00019J-Pk for qemu-devel@nongnu.org; Fri, 17 Apr 2015 11:01:35 -0400 Received: from mail-wi0-x233.google.com ([2a00:1450:400c:c05::233]:33691) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yj7lr-00019A-EH for qemu-devel@nongnu.org; Fri, 17 Apr 2015 11:01:31 -0400 Received: by wiax7 with SMTP id x7so35926236wia.0 for ; Fri, 17 Apr 2015 08:01:29 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <55312044.90604@redhat.com> Date: Fri, 17 Apr 2015 17:01:24 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1429280557-8887-1-git-send-email-berrange@redhat.com> <1429280557-8887-7-git-send-email-berrange@redhat.com> <55311F3B.8060305@redhat.com> In-Reply-To: <55311F3B.8060305@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v1 RFC 06/34] qom: add a object_property_add_enum helper method List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org Cc: Gerd Hoffmann , Stefan Hajnoczi On 17/04/2015 16:56, Paolo Bonzini wrote: > > > On 17/04/2015 16:22, Daniel P. Berrange wrote: >> A QOM property can be parsed as enum using the visit_type_enum() >> helper method, but this forces callers to use the more complex >> generic object_property_add() method when registering it. It >> also requires that users of that object have access to the >> string map when they want to read the property value. >> >> This patch introduces a specialized object_property_add_enum() >> method which simplifies the use of enum properties, so the >> setters/getters directly get passed the int value. >> >> typedef enum { >> MYDEV_TYPE_FROG, >> MYDEV_TYPE_ALLIGATOR, >> MYDEV_TYPE_PLATYPUS, >> >> MYDEV_TYPE_LAST >> } MyDevType; >> >> Then provide a table of enum <-> string mappings >> >> static const char *const mydevtypemap[MYDEV_TYPE_LAST + 1] = { >> [MYDEV_TYPE_FROG] = "frog", >> [MYDEV_TYPE_ALLIGATOR] = "alligator", >> [MYDEV_TYPE_PLATYPUS] = "platypus", >> [MYDEV_TYPE_LAST] = NULL, >> }; >> >> Assuming an object struct of >> >> typedef struct { >> Object parent; >> MyDevType devtype; >> ...other fields... >> } MyDev; >> >> The property can then be registered as follows: >> >> static int mydev_prop_get_devtype(Object *obj, >> Error **errp G_GNUC_UNUSED) >> { >> MyDev *dev = MYDEV(obj); >> >> return dev->devtype; >> } >> >> static void mydev_prop_set_devtype(Object *obj, >> int value, >> Error **errp G_GNUC_UNUSED) >> { >> MyDev *dev = MYDEV(obj); >> >> dev->endpoint = value; >> } >> >> object_property_add_enum(obj, "devtype", >> mydevtypemap, >> mydev_prop_get_devtype, >> mydev_prop_set_devtype, >> NULL); On second thought (after seeing patch 7), please add a property type argument here. We lose introspection of enum property types otherwise. It's possible to use macros so that 'MyEnum' gets translated to two arguments '"MyEnum", MyEnum_lookup', but I don't think that's worth the obfuscation. Paolo >> Note there is no need to check the range of 'value' in >> the setter, because the string->enum conversion code will >> have already done that and reported an error as required. >> >> Signed-off-by: Daniel P. Berrange >> --- >> include/qom/object.h | 17 ++++++++++++++++ >> qom/object.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 74 insertions(+) >> >> diff --git a/include/qom/object.h b/include/qom/object.h >> index dfdba2f..3462821 100644 >> --- a/include/qom/object.h >> +++ b/include/qom/object.h >> @@ -1262,6 +1262,23 @@ void object_property_add_bool(Object *obj, const char *name, >> Error **errp); >> >> /** >> + * object_property_add_enum: >> + * @obj: the object to add a property to >> + * @name: the name of the property >> + * @get: the getter or NULL if the property is write-only. >> + * @set: the setter or NULL if the property is read-only >> + * @errp: if an error occurs, a pointer to an area to store the error >> + * >> + * Add a enum property using getters/setters. This function will add a >> + * property of type 'enum'. >> + */ >> +void object_property_add_enum(Object *obj, const char *name, >> + const char * const *strings, >> + int (*get)(Object *, Error **), >> + void (*set)(Object *, int, Error **), >> + Error **errp); >> + >> +/** >> * object_property_add_tm: >> * @obj: the object to add a property to >> * @name: the name of the property >> diff --git a/qom/object.c b/qom/object.c >> index 2534398..543cc57 100644 >> --- a/qom/object.c >> +++ b/qom/object.c >> @@ -1607,6 +1607,63 @@ void object_property_add_bool(Object *obj, const char *name, >> } >> } >> >> +typedef struct EnumProperty { >> + const char * const *strings; >> + int (*get)(Object *, Error **); >> + void (*set)(Object *, int, Error **); >> +} EnumProperty; >> + >> +static void property_get_enum(Object *obj, Visitor *v, void *opaque, >> + const char *name, Error **errp) >> +{ >> + EnumProperty *prop = opaque; >> + int value; >> + >> + value = prop->get(obj, errp); >> + visit_type_enum(v, &value, prop->strings, NULL, name, errp); >> +} >> + >> +static void property_set_enum(Object *obj, Visitor *v, void *opaque, >> + const char *name, Error **errp) >> +{ >> + EnumProperty *prop = opaque; >> + int value; >> + >> + visit_type_enum(v, &value, prop->strings, NULL, name, errp); >> + prop->set(obj, value, errp); >> +} >> + >> +static void property_release_enum(Object *obj, const char *name, >> + void *opaque) >> +{ >> + EnumProperty *prop = opaque; >> + g_free(prop); >> +} >> + >> +void object_property_add_enum(Object *obj, const char *name, >> + const char * const *strings, >> + int (*get)(Object *, Error **), >> + void (*set)(Object *, int, Error **), >> + Error **errp) >> +{ >> + Error *local_err = NULL; >> + EnumProperty *prop = g_malloc0(sizeof(*prop)); >> + >> + prop->strings = strings; >> + prop->get = get; >> + prop->set = set; >> + >> + object_property_add(obj, name, "enum", >> + get ? property_get_enum : NULL, >> + set ? property_set_enum : NULL, >> + property_release_enum, >> + prop, &local_err); >> + if (local_err) { >> + error_propagate(errp, local_err); >> + g_free(prop); >> + } >> +} >> + >> typedef struct TMProperty { >> void (*get)(Object *, struct tm *, Error **); >> } TMProperty; >> > > Reviewed-by: Paolo Bonzini > >