[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] qom: change object property iterator API contra
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-devel] [PATCH] qom: change object property iterator API contract |
Date: |
Wed, 9 Dec 2015 12:29:09 +0000 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Tue, Dec 08, 2015 at 09:05:52AM -0700, Eric Blake wrote:
> 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 <address@hidden>
> > ---
> >
> > 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().
Yeh, forgot to remove the header declaration.
>
> > +++ 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 <address@hidden>
Ok, thanks for the review
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|