Re: [Qemu-devel] [PATCH v1 RFC 06/34] qom: add a object_property_add_enum helper method

[Paolo Bonzini <pbonzini@redhat.com>]

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 <berrange@redhat.com>
>> ---
>>  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 <pbonzini@redhat.com>
> 
>