From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36781) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zwtps-0002AI-NR for qemu-devel@nongnu.org; Thu, 12 Nov 2015 10:30:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zwtpo-0005YJ-Gy for qemu-devel@nongnu.org; Thu, 12 Nov 2015 10:30:52 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51922) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zwtpo-0005YF-7O for qemu-devel@nongnu.org; Thu, 12 Nov 2015 10:30:48 -0500 References: <1447224690-9743-1-git-send-email-eblake@redhat.com> <1447224690-9743-26-git-send-email-eblake@redhat.com> <877flnjlqr.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <5644B0A2.3020606@redhat.com> Date: Thu, 12 Nov 2015 08:30:42 -0700 MIME-Version: 1.0 In-Reply-To: <877flnjlqr.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="VfFfT1cB95vSij9ILpL9GHrsAnQfU5gwd" Subject: Re: [Qemu-devel] [PATCH v11 25/28] qapi: Simplify visits of optional fields 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) --VfFfT1cB95vSij9ILpL9GHrsAnQfU5gwd Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/12/2015 08:11 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> None of the visitor callbacks would set an error when testing >> if an optional field was present; make this part of the interface >> contract by eliminating the errp argument. Then, for less code, >> reflect the determined boolean value back to the caller instead >> of making the caller read the boolean after the fact. >> >> The resulting generated code has a nice diff: >> >> |- visit_optional(v, &has_fdset_id, "fdset-id", &err); >> |- if (err) { >> |- goto out; >> |- } >> |- if (has_fdset_id) { >> |+ if (visit_optional(v, &has_fdset_id, "fdset-id")) { >> | visit_type_int(v, &fdset_id, "fdset-id", &err); >> | if (err) { >> | goto out; >> | } >> | } >=20 > Any particular reason not to do >=20 > has_fdset_id =3D visit_optional(v, "fdset-id"); > if (has_fdset_id) { We can't. Output visitors do not implement visit_optional() callbacks, but must rely on the incoming value of has_fdset_id. Which means assigning to has_fdset_id without an incoming value will do the wrong thing. Or worded differently, &has_fdset_id is modified as an output parameter by input visitors, and read unchanged as an input parameter by output visitors. >> +++ b/qapi/qapi-visit-core.c >> @@ -73,12 +73,12 @@ void visit_end_union(Visitor *v, bool data_present= , Error **errp) >> } >> } >> >> -void visit_optional(Visitor *v, bool *present, const char *name, >> - Error **errp) >> +bool visit_optional(Visitor *v, bool *present, const char *name) >> { >> if (v->optional) { >> - v->optional(v, present, name, errp); >> + v->optional(v, present, name); >> } >> + return *present; >> } >=20 > Slightly ugly: struct Visitor method optional returns void, but the > wrapper returns bool. I could make all the callbacks (all 3 of them: opts-visitor, qmp-input-visitor, string-input-visitor) return bool, but didn't see the point in the churn, especially since the contract of the return value is easy to do in the wrapper. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --VfFfT1cB95vSij9ILpL9GHrsAnQfU5gwd 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/ iQEcBAEBCAAGBQJWRLCiAAoJEKeha0olJ0Nq4cAH/ipwyDCwvZ0dvLjReg3UT25q +s5cHe+JzNLqcnZNeZMoA40K1SuPEt7pdHImnJNyZZ4xAS6/PqjUhfO6gNTebwzT xhGAardvhDhK75iCC5tWDJHfJoCz1D1+IN0BxE7ccCtmk+vSVJMRD5xXi6WKZGFU al9PPhASSwBq5/lDzc6RvtmohBW16KjmVqkIWyB+R5DGzHot1JgD1Pv0SuTsr05d zWPZGpobKITMws2rJuknBFe6Vq0FKiTINLOg7d7RDO5bI3hTcNnRc1JObhFd09FN H7iEq35c6Am+fqbQhJBAl09Vn7hEj2jH90BUaHnGgjV7UUKihzJfG9BzorVCctU= =/IJR -----END PGP SIGNATURE----- --VfFfT1cB95vSij9ILpL9GHrsAnQfU5gwd--