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

[Paolo Bonzini]

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);
> 
> 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 <address@hidden>
> ---
>  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 <address@hidden>