From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45628) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1avAs7-0006bT-VH for qemu-devel@nongnu.org; Tue, 26 Apr 2016 17:50:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1avAs4-0000UN-Mr for qemu-devel@nongnu.org; Tue, 26 Apr 2016 17:50:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60579) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1avAs4-0000UG-Ep for qemu-devel@nongnu.org; Tue, 26 Apr 2016 17:50:16 -0400 References: <1460131992-32278-1-git-send-email-eblake@redhat.com> <1460131992-32278-9-git-send-email-eblake@redhat.com> <87inzk43ml.fsf@dusky.pond.sub.org> From: Eric Blake Message-ID: <571FE295.30102@redhat.com> Date: Tue, 26 Apr 2016 15:50:13 -0600 MIME-Version: 1.0 In-Reply-To: <87inzk43ml.fsf@dusky.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="WtEXRorJ6KsbBBbt1GOXKfItmUlWRpKWP" Subject: Re: [Qemu-devel] [PATCH v14 08/19] qapi: Document visitor interfaces, add assertions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, Michael Roth This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --WtEXRorJ6KsbBBbt1GOXKfItmUlWRpKWP Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 04/14/2016 09:22 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> The visitor interface for mapping between QObject/QemuOpts/string >> and QAPI is scandalously under-documented, making changes to visitor >> core, individual visitors, and users of visitors difficult to >> coordinate. Among other questions: when is it safe to pass NULL, >> vs. when a string must be provided; which visitors implement which >> callbacks; the difference between concrete and virtual visits. >> >> Correct this by retrofitting proper contracts, and document where some= >> of the interface warts remain (for example, we may want to modify >> visit_end_* to require the same 'obj' as the visit_start counterpart, >> so the dealloc visitor can be simplified). Later patches in this >> series will tackle some, but not all, of these warts. >> >> Add assertions to (partially) enforce the contract. Some of these >> were only made possible by recent cleanup commits. >> >> +/* >> + * The QAPI schema defines both a set of C data types, and a QMP wire= >> + * format. A QAPI object is formed as a directed acyclic graph of >> + * QAPI values. >=20 > I understand what you're trying to say, but I find the value / object > dichotomy odd. For me, A QAPI object isn't a DAG, it's a node in a DAG= =2E > Perhaps: "QAPI objects can contain references to other QAPI objects, > resulting in a directed acyclic graph." Thanks for a lot of good comments. I'm replying only to the questions you left amidst all the good review. >> +++ b/qapi/qapi-visit-core.c >> @@ -23,6 +23,10 @@ >> void visit_start_struct(Visitor *v, const char *name, void **obj, >> size_t size, Error **errp) >> { >> + if (obj) { >> + assert(size); >=20 > Yes, because the generator puts a dummy member into empty structs. >=20 >> + assert(v->type !=3D VISITOR_OUTPUT || *obj); >=20 > Can you point me to the spot in the contract that requires this? Translation of the assert: If you are using an output visitor, and not doing a virtual walk (obj is non-NULL), then the object must be completely built (*obj is non-NULL). For an input visitor, *obj is NULL on entry (we're allocating it, after all); for the dealloc visitor, *obj may or may not be NULL (since we handle cleanup of partial allocation). In the text, "output visitors (QMP and string) take a completed QAPI graph", but maybe I can further clarify that a completed object means that *obj is non-NULL and all 'has_member' and 'member' members are complete. >=20 > Unlike last time, my remarks are pretty much only about how to say > things, not about what to say. Progress! Yay! Hopefully I'll post v15 soon and we can get this in at the start of the 2.7 cycle. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --WtEXRorJ6KsbBBbt1GOXKfItmUlWRpKWP 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/ iQEcBAEBCAAGBQJXH+KVAAoJEKeha0olJ0NqGH4H/0EKm//ayLtFwatrAm9ZE+S3 eYwv50/vC3CQ7w6/4nA40sRZnsuL/F5vb1/hpky/GsiAqyP4zqMm6fEorInzbIKB p7tfBE3lH/Uw/las6yUfnvZB2JjY4O+QOd3VA9sQ0nSt+mu+Dh/7PUDYqPeE/Dbt pkAWJLexDmVibLhyqGBX3U657WMJ0dHBN1q1xZ+mEJDRqHCMuMXWFibBoTBW9Su7 lIgf+BxeHx04atghyfgfW7BbAqjc6Efa83r+jH/lniYa51T+fzk+6LE4HbDIYTXv svzf58w99qXPRRirMPtNaC6Ox+yK/G36YrZpdtTbJCiV0ClEPFqK7zIBcpbZbU8= =iPZO -----END PGP SIGNATURE----- --WtEXRorJ6KsbBBbt1GOXKfItmUlWRpKWP--