From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52258) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a6drw-0006rt-QX for qemu-devel@nongnu.org; Wed, 09 Dec 2015 07:29:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a6drt-0006tY-IY for qemu-devel@nongnu.org; Wed, 09 Dec 2015 07:29:16 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54684) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a6drt-0006tU-BW for qemu-devel@nongnu.org; Wed, 09 Dec 2015 07:29:13 -0500 Date: Wed, 9 Dec 2015 12:29:09 +0000 From: "Daniel P. Berrange" Message-ID: <20151209122909.GC19914@redhat.com> References: <1448638055-22113-1-git-send-email-berrange@redhat.com> <5666FFE0.10703@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <5666FFE0.10703@redhat.com> Subject: Re: [Qemu-devel] [PATCH] qom: change object property iterator API contract Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Andreas =?utf-8?Q?F=C3=A4rber?= , qemu-devel@nongnu.org, Markus Armbruster 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 > > --- > > > > 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 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 :|