From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39459) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aVosg-0002P8-Fs for qemu-devel@nongnu.org; Tue, 16 Feb 2016 18:18:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aVosd-0004Vl-27 for qemu-devel@nongnu.org; Tue, 16 Feb 2016 18:18:06 -0500 Received: from mx1.redhat.com ([209.132.183.28]:52948) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aVosc-0004VP-Pd for qemu-devel@nongnu.org; Tue, 16 Feb 2016 18:18:02 -0500 References: <1455582057-27565-1-git-send-email-eblake@redhat.com> <1455582057-27565-3-git-send-email-eblake@redhat.com> <87lh6kprih.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <56C3AE29.5000904@redhat.com> Date: Tue, 16 Feb 2016 16:18:01 -0700 MIME-Version: 1.0 In-Reply-To: <87lh6kprih.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="d0aMdX8NAnCPRcHkOB0aBDMdQ5JWe0cnm" Subject: Re: [Qemu-devel] [PATCH v10 02/13] qapi: Forbid empty unions and useless alternates 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) --d0aMdX8NAnCPRcHkOB0aBDMdQ5JWe0cnm Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 02/16/2016 09:08 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> Empty unions serve no purpose, and while we compile with gcc >> which permits them, strict C99 forbids them. We could inject >> a dummy member (and in fact, we do for empty structs), but while >=20 > gen_variants() injects void *data.=20 >=20 >> empty structs make sense in qapi, >=20 > Suggest to cut the paragaph until here. Side effect of rebasing - I originally had this patch after one that deletes the 'data' member, but that requires a few other patches. I'll reword it to mention that we want to delete 'data', at which point we would be left with an empty union if we didn't prohibit it at parse time.= >> @@ -613,7 +616,11 @@ def check_alternate(expr, expr_info): >> members =3D expr['data'] >> types_seen =3D {} >> >> - # Check every branch >> + # Check every branch; require at least two branches >> + if len(members) < 2: >> + raise QAPIExprError(expr_info, >> + "Alternate '%s' should have at least two = branches " >> + "in 'data'" % name) >=20 > This is stricter than the commit message announced. You can either > relax to at least one branch, or amend the commit message. Will amend; a later patch actually relies on the 2-or-more promise for alternates, so it is worth documenting as intentional. >> A simple union type defines a mapping from automatic discriminator >> values to data types like in this example: >=20 > Missing: update of section "Alternate types". Already done there (commit 7b1b98c4): "An alternate type is one that allows a choice between two or more JSON data types (string, integer, number, or object, but currently not..." I didn't see anything worth rewording. >=20 > [tests/ diff looks good] >=20 --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --d0aMdX8NAnCPRcHkOB0aBDMdQ5JWe0cnm 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/ iQEcBAEBCAAGBQJWw64pAAoJEKeha0olJ0NqPXMH/2IjwBQBjeDQV1UK8tbL26eo pkF11wdX8bk0Dr+xlreU3kApti+Tif8sT3Zx1BGIYYTVEmFdoT4aXTE53OT1Zzct TrQ6At/d4pqymx5sDRuIZLfW4C18Mpg3a3ZCqp/0Wyk0UD7PXgQQeoR2YiHCVrjl dDcDqfYq36nCaBxvaquLwGIboifRop3eoZ/E6I+vh09ZVG4OB4I00QzwPw5PbdYB qBj+t6bc86P2Xtx/lrQsX7/3lokXpehKAGXrw5RqstxxO2QgBRITo3Buq3P1EArT 9v0oxu3P8ia3Z8exCPojYDvC1yXO+HAbWbMTdx41PL3/09T/ePr5TH1Wuh9GEeA= =vzMP -----END PGP SIGNATURE----- --d0aMdX8NAnCPRcHkOB0aBDMdQ5JWe0cnm--