Re: [Qemu-devel] [PATCH] qom: change object property iterator API contract

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2607 bytes --]

On 11/27/2015 08:27 AM, Daniel P. Berrange wrote:
> Currently the object property iterator API works as follows
> 
>   ObjectPropertyIterator *iter;
> 
>   iter = object_property_iter_init(obj);
>   while ((prop = object_property_iter_next(iter))) {
>      ...
>   }
>   object_property_iter_free(iter);
> 
> This has the benefit that the ObjectPropertyIterator struct
> can be opaque, but has the downside that callers need to
> explicitly call a free function. It is also not in keeping
> with iterator style used elsewhere in QEMU/glib2
> 
> This patch changes the API to use stack allocation instead
> 
>   ObjectPropertyIterator iter;
> 
>   object_property_iter_init(&iter, obj);
>   while ((prop = object_property_iter_next(&iter))) {
>      ...
>   }
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> 
> NB, this patch is not against master, it is intended to apply
> after
> 
>   "qom: allow properties to be registered against classes"
> 
> which is queued in qom-next for 2.6
> 
>  hw/ppc/spapr_drc.c         |  7 +++----
>  include/qom/object.h       | 42 +++++++++++++++++++++++++++---------------
>  net/filter.c               |  7 +++----
>  qmp.c                      | 14 ++++++--------
>  qom/object.c               | 22 ++++------------------
>  tests/check-qom-proplist.c |  7 +++----
>  vl.c                       |  7 +++----
>  7 files changed, 49 insertions(+), 57 deletions(-)
> 

> +++ b/include/qom/object.h
> @@ -346,6 +346,7 @@ typedef struct ObjectProperty
>      void *opaque;
>  } ObjectProperty;
>  
> +
>  /**
>   * ObjectUnparent:
>   * @obj: the object that is being removed from the composition tree

Spurious whitespace change?


>  
> + /**
> + * object_property_iter_free:
> + * @iter: the iterator instance
> + *
> + * Releases any resources associated with the iterator. It is
> + * not necessary to call this method if object_property_iter_next
> + * has returned %NULL. It is only required if an application wishes
> + * to abort iteration before it is complete
> + */
> +void object_property_iter_free(ObjectPropertyIterator *iter);
> +

Huh? Why is this being added?  I thought the point was to get rid of the
need for object_property_iter_free().

> +++ b/qom/object.c
> @@ -67,11 +67,6 @@ struct TypeImpl

Other than that snafu, everything else looked fine.   If that's all you
fix for v2, you can add:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]