Re: [Qemu-devel] [PATCH 1/4] object: add object_property_add_bool

[Andreas Färber]

Am 25.06.2012 18:09, schrieb Anthony Liguori:
> Signed-off-by: Anthony Liguori <address@hidden>
> ---
>  include/qemu/object.h |   16 ++++++++++++++
>  qom/object.c          |   56 
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 72 insertions(+), 0 deletions(-)
> 
> diff --git a/include/qemu/object.h b/include/qemu/object.h
> index 8b17776..3711054 100644
> --- a/include/qemu/object.h
> +++ b/include/qemu/object.h
> @@ -933,6 +933,22 @@ void object_property_add_str(Object *obj, const char 
> *name,
>                               struct Error **errp);
>  
>  /**
> + * object_property_add_bool:
> + * @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 bool property using getters/setters.  This function will add a
> + * property of type 'bool'.
> + */
> +void object_property_add_bool(Object *obj, const char *name,
> +                             bool (*get)(Object *, struct Error **),
> +                             void (*set)(Object *, bool, struct Error **),
> +                             struct Error **errp);

Indentation is off by one.

> +
> +/**
>   * object_child_foreach:
>   * @obj: the object whose children will be navigated
>   * @fn: the iterator function to be called
> diff --git a/qom/object.c b/qom/object.c
> index 00bb3b0..1357397 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1232,6 +1232,62 @@ void object_property_add_str(Object *obj, const char 
> *name,
>                          prop, errp);
>  }
>  
> +typedef struct BoolProperty
> +{
> +    bool (*get)(Object *, Error **);
> +    void (*set)(Object *, bool, Error **);
> +} BoolProperty;

This follows the modelling of the StringProperty and looks okay to me.
However, I stumbled over whether we should allow passing an opaque to
the typed accessors, too?
The use case I have in mind is having a dummy device bundle properties
for some parent (wild example: /machine/cpu[1]/features x2apic rather
than /machine/cpu[1] x2apic-feature), i.e. where the obj passed to the
property and the storage of the value differ somehow.

Not a blocker for this series, general design question. CC'ing Paolo.

Andreas

> +
> +static void property_get_bool(Object *obj, Visitor *v, void *opaque,
> +                              const char *name, Error **errp)
> +{
> +    BoolProperty *prop = opaque;
> +    bool value;
> +
> +    value = prop->get(obj, errp);
> +    visit_type_bool(v, &value, name, errp);
> +}
> +
> +static void property_set_bool(Object *obj, Visitor *v, void *opaque,
> +                              const char *name, Error **errp)
> +{
> +    BoolProperty *prop = opaque;
> +    bool value;
> +    Error *local_err = NULL;
> +
> +    visit_type_bool(v, &value, name, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +
> +    prop->set(obj, value, errp);
> +}
> +
> +static void property_release_bool(Object *obj, const char *name,
> +                                  void *opaque)
> +{
> +    BoolProperty *prop = opaque;
> +    g_free(prop);
> +}
> +
> +void object_property_add_bool(Object *obj, const char *name,
> +                              bool (*get)(Object *, Error **),
> +                              void (*set)(Object *, bool, Error **),
> +                              Error **errp)
> +{
> +    BoolProperty *prop = g_malloc0(sizeof(*prop));
> +
> +    prop->get = get;
> +    prop->set = set;
> +
> +    object_property_add(obj, name, "bool",
> +                        get ? property_get_bool : NULL,
> +                        set ? property_set_bool : NULL,
> +                        property_release_bool,
> +                        prop, errp);
> +}
> +
>  static char *qdev_get_type(Object *obj, Error **errp)
>  {
>      return g_strdup(object_get_typename(obj));


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg