From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40813) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZkE4i-00033x-M5 for qemu-devel@nongnu.org; Thu, 08 Oct 2015 12:29:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZkE4Q-0006GP-Ah for qemu-devel@nongnu.org; Thu, 08 Oct 2015 12:29:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51528) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZkE4Q-0006GI-2q for qemu-devel@nongnu.org; Thu, 08 Oct 2015 12:29:30 -0400 References: <1444313344-16196-1-git-send-email-berrange@redhat.com> <1444313344-16196-2-git-send-email-berrange@redhat.com> From: Eric Blake Message-ID: <561699D7.1010105@redhat.com> Date: Thu, 8 Oct 2015 10:29:11 -0600 MIME-Version: 1.0 In-Reply-To: <1444313344-16196-2-git-send-email-berrange@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="S4kCNvM2D7UBL1sAFRf0vg2Jh3O4GqmeU" Subject: Re: [Qemu-devel] [PATCH v3 1/5] qom: introduce object_property_foreach method List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org Cc: Pavel Fedin , =?UTF-8?Q?Andreas_F=c3=a4rber?= This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --S4kCNvM2D7UBL1sAFRf0vg2Jh3O4GqmeU Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/08/2015 08:09 AM, Daniel P. Berrange wrote: > Some users of QOM need to be able to iterate over properties > defined against an object instance. Currently they are just > directly using the QTAIL macros against the object properties > data structure. >=20 > This is bad because it exposes them to changes in the data > structure used to store properties, as well as changes in > functionality such as ability to register properties against > the class. >=20 > Providing an explicit object_property_foreach method provides > a layer of insulation between the QOM user and the QOM internal > implementation. >=20 > Signed-off-by: Daniel P. Berrange > --- > include/qom/object.h | 23 +++++++++++++++++++++++ > qom/object.c | 17 +++++++++++++++++ > 2 files changed, 40 insertions(+) >=20 > diff --git a/include/qom/object.h b/include/qom/object.h > index be7280c..71503af 100644 > --- a/include/qom/object.h > +++ b/include/qom/object.h > @@ -960,6 +960,29 @@ void object_property_del(Object *obj, const char *= name, Error **errp); > ObjectProperty *object_property_find(Object *obj, const char *name, > Error **errp); > =20 > +typedef void (*ObjectPropertyIterator)(Object *obj, > + ObjectProperty *prop, > + Error **errp, > + void *opaque); Do we want the iterator to be able to return a value, and possibly allow a non-zero value to abort iteration? [1] > + > +/** > + * object_property_foreach: > + * @obj: the object > + * @iter: the iterator callback function > + * @errp: returns an error if iterator function fails > + * @opaque: opaque data to pass to @iter > + * > + * Iterates over all properties defined against the object > + * instance calling @iter for each property. Probably should mention that there is an early exit if error gets set [2]= > + * > + * It is forbidden to modify the property list from @iter > + * whether removing or adding properties. > + */ > +void object_property_foreach(Object *obj, > + ObjectPropertyIterator iter, > + Error **errp, > + void *opaque); [1] if we allow the iterator to return a non-zero value to abort iteration (particularly if it wants to abort iteration without setting an error, just to save CPU cycles), should this foreach() function return that value? Of course, a caller can always use opaque to achieve the same purpose, but it gets more verbose (the caller has to reserve space in their opaque for tracking whether an interesting exit value is needed, as well as having an early check on whether the value was already set in a previous visit) and burns more CPU cycles (the iterator runs to completion, even though the later callbacks are doing nothing). > +++ b/qom/object.c > @@ -917,6 +917,23 @@ ObjectProperty *object_property_find(Object *obj, = const char *name, > return NULL; > } > =20 > +void object_property_foreach(Object *obj, > + ObjectPropertyIterator iter, > + Error **errp, > + void *opaque) > +{ > + ObjectProperty *prop; > + Error *local_err =3D NULL; > + > + QTAILQ_FOREACH(prop, &obj->properties, node) { > + iter(obj, prop, &local_err, opaque); > + if (local_err) { > + error_propagate(errp, local_err); > + return; [2] there's the early exit if an error is set. The code looks fine, but I think we need a documentation improvement for issue [2], and we may want a design change for issue [1] if a non-void return would be useful to any client later in the series. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --S4kCNvM2D7UBL1sAFRf0vg2Jh3O4GqmeU Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJWFpnXAAoJEKeha0olJ0Nq0YgH/jbjdDMy2ql2EgWA2tP+BiK0 my8GX1o4PE+fHl50TimMKTxds+ZDteASEWbl1Ftm7H40G1Xfq9JoP/PoL8Dqc9PC SmJqjP5ShH8v3TOvSI9z6uRLR6Y6VtumEoK6vkNxRNhk+iYFv4k4/7l3v3yPWkLr 0RCrduD81acq/aKFFdyANXRgALRj9LpfU2YQ9P6LAncpUVLRpm5wADBN6KjjHdTs 5zFMAk8/sswHcHgQ2eH5TJywqCCak3EWptWmW5eOf5m9y80Wk93C4MOe8ZmpIEPL QXtPyZDM3zJRlebXkmmEJ/GKUlBmrv3dDGSlOBzbFyVuLyqbG8WDxL2KkRQBI5o= =tDYe -----END PGP SIGNATURE----- --S4kCNvM2D7UBL1sAFRf0vg2Jh3O4GqmeU--