Re: [Qemu-devel] [PATCH v2 20/27] qdev: let QOM free properties

[Anthony Liguori <anthony@codemonkey.ws>]

On 02/04/2012 02:02 AM, Paolo Bonzini wrote:
> Drop the special free callback.  Instead, register a "regular"
> release method in the non-legacy property.
>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   hw/qdev-properties.c |   19 ++++++++++++-------
>   hw/qdev.c            |    8 +-------
>   hw/qdev.h            |    2 +-
>   3 files changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 67995a3..d69a987 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -510,9 +510,10 @@ PropertyInfo qdev_prop_hex64 = {
>
>   /* --- string --- */
>
> -static void free_string(DeviceState *dev, Property *prop)
> +static void release_string(Object *obj, const char *name, void *opaque)
>   {
> -    g_free(*(char **)qdev_get_prop_ptr(dev, prop));
> +    Property *prop = opaque;
> +    g_free(*(char **)qdev_get_prop_ptr(DEVICE(obj), prop));
>   }
>
>   static int print_string(DeviceState *dev, Property *prop, char *dest, size_t len)
> @@ -572,7 +573,7 @@ PropertyInfo qdev_prop_string = {
>       .type  = PROP_TYPE_STRING,
>       .size  = sizeof(char*),
>       .print = print_string,
> -    .free  = free_string,
> +    .release = release_string,
>       .get   = get_string,
>       .set   = set_string,
>   };
> @@ -592,8 +593,10 @@ static int parse_drive(DeviceState *dev, const char *str, void **ptr)
>       return 0;
>   }
>
> -static void free_drive(DeviceState *dev, Property *prop)
> +static void release_drive(Object *obj, const char *name, void *opaque)
>   {
> +    DeviceState *dev = DEVICE(obj);
> +    Property *prop = opaque;
>       BlockDriverState **ptr = qdev_get_prop_ptr(dev, prop);
>
>       if (*ptr) {
> @@ -667,7 +670,7 @@ PropertyInfo qdev_prop_drive = {
>       .size  = sizeof(BlockDriverState *),
>       .get   = get_drive,
>       .set   = set_drive,
> -    .free  = free_drive,
> +    .release = release_drive,
>   };
>
>   /* --- character device --- */
> @@ -686,8 +689,10 @@ static int parse_chr(DeviceState *dev, const char *str, void **ptr)
>       return 0;
>   }
>
> -static void free_chr(DeviceState *dev, Property *prop)
> +static void release_chr(Object *obj, const char *name, void *opaque)
>   {
> +    DeviceState *dev = DEVICE(obj);
> +    Property *prop = opaque;
>       CharDriverState **ptr = qdev_get_prop_ptr(dev, prop);
>
>       if (*ptr) {
> @@ -721,7 +726,7 @@ PropertyInfo qdev_prop_chr = {
>       .size  = sizeof(CharDriverState*),
>       .get   = get_chr,
>       .set   = set_chr,
> -    .free  = free_chr,
> +    .release = release_chr,
>   };
>
>   /* --- netdev device --- */
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 660ee38..f719f14 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -595,7 +595,7 @@ void qdev_property_add_static(DeviceState *dev, Property *prop,
>
>       object_property_add(OBJECT(dev), prop->name, prop->info->name,
>                           prop->info->get, prop->info->set,
> -                        NULL,
> +                        prop->info->release,
>                           prop, errp);
>   }
>
> @@ -626,7 +626,6 @@ static void device_finalize(Object *obj)
>   {
>       DeviceState *dev = DEVICE(obj);
>       BusState *bus;
> -    Property *prop;
>       DeviceClass *dc = DEVICE_GET_CLASS(dev);
>
>       if (dev->state == DEV_STATE_INITIALIZED) {
> @@ -645,11 +644,6 @@ static void device_finalize(Object *obj)
>           }
>       }
>       QTAILQ_REMOVE(&dev->parent_bus->children, dev, sibling);
> -    for (prop = qdev_get_props(dev); prop&&  prop->name; prop++) {
> -        if (prop->info->free) {
> -            prop->info->free(dev, prop);
> -        }
> -    }
>   }
>
>   void device_reset(DeviceState *dev)
> diff --git a/hw/qdev.h b/hw/qdev.h
> index c31dc4e..acccf26 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -145,9 +145,9 @@ struct PropertyInfo {
>       int64_t max;
>       int (*parse)(DeviceState *dev, Property *prop, const char *str);
>       int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len);
> -    void (*free)(DeviceState *dev, Property *prop);
>       ObjectPropertyAccessor *get;
>       ObjectPropertyAccessor *set;
> +    ObjectPropertyRelease *release;
>   };
>
>   typedef struct GlobalProperty {