Re: [Qemu-devel] [patch 2/3] add object_property_add_alias

[Paolo Bonzini]

Il 04/06/2014 19:52, address@hidden ha scritto:
> Allowing addition of a link without keeping pointer-to-pointer.
>
> Signed-off-by: Marcelo Tosatti <address@hidden>
>
> ---
>  include/qom/object.h |   13 +++++++++
>  qom/object.c         |   73 +++++++++++++++++++++++++++++++++++++++++----------
>  2 files changed, 72 insertions(+), 14 deletions(-)
>
> Index: qemu/include/qom/object.h
> ===================================================================
> --- qemu.orig/include/qom/object.h      2014-06-02 23:12:51.875693325 -0300
> +++ qemu/include/qom/object.h   2014-06-02 23:14:13.045432426 -0300
> @@ -1073,6 +1073,19 @@
>  } ObjectPropertyLinkFlags;
>
>  /**
> + * object_property_add_alias:
> + * @obj: the object to add a property to
> + * @name: the name of the property
> + * @alias: the alias object
> + * @errp: if an error occurs, a pointer to an area to store the area
> + *
> + * Add a link under obj, named name, pointing to alias.
> + *
> + */
> +void object_property_add_alias(Object *obj, const char *name,
> +                               Object *alias, Error **errp);
> +
> +/**
>   * object_property_allow_set_link:
>   *
>   * The default implementation of the object_property_add_link() check()
> Index: qemu/qom/object.c
> ===================================================================
> --- qemu.orig/qom/object.c      2014-06-02 23:12:51.875693325 -0300
> +++ qemu/qom/object.c   2014-06-02 23:14:13.046432423 -0300
> @@ -1023,27 +1023,71 @@
>      g_free(type);
>  }
>
> +typedef struct {
> +    Object *child;
> +    Object **childp;

These field names are ugly... not your fault, but perhaps

    Object **linkp;
    Object *alias_dest;

would be better.  It would also avoid the mistake below:

> @@ -1096,7 +1140,7 @@
>  {
>      Error *local_err = NULL;
>      LinkProperty *prop = opaque;
> -    Object **child = prop->child;
> +    Object **child = prop->childp;
>      Object *old_target = *child;
>      Object *new_target = NULL;
>      char *path = NULL;

This is object_set_link_property.  It writes *child but not prop->child, 
and subsequent calls to object_get_link_property incorrect.

However, since a similar need arose recently in one of Peter 
Crosthwaite's patches, let's add a generic resolve mechanism.  I'll post 
a short series in a second, as soon as I finish testing it.

Paolo

> @@ -1133,8 +1177,8 @@
>  {
>      LinkProperty *prop = opaque;
>
> -    if ((prop->flags & OBJ_PROP_LINK_UNREF_ON_RELEASE) && *prop->child) {
> -        object_unref(*prop->child);
> +    if ((prop->flags & OBJ_PROP_LINK_UNREF_ON_RELEASE) && prop->child) {
> +        object_unref(prop->child);
>      }
>      g_free(prop);
>  }
> @@ -1150,7 +1194,8 @@
>      LinkProperty *prop = g_malloc(sizeof(*prop));
>      gchar *full_type;
>
> -    prop->child = child;
> +    prop->childp = child;
> +    prop->child = *child;
>      prop->check = check;
>      prop->flags = flags;
>
> @@ -1227,7 +1272,7 @@
>
>      if (object_property_is_link(prop)) {
>          LinkProperty *lprop = prop->opaque;
> -        return *lprop->child;
> +        return lprop->child;
>      } else if (object_property_is_child(prop)) {
>          return prop->opaque;
>      } else {