From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50278) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aW2OK-0005aS-OU for qemu-devel@nongnu.org; Wed, 17 Feb 2016 08:43:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aW2OH-0003iM-GY for qemu-devel@nongnu.org; Wed, 17 Feb 2016 08:43:40 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48619) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aW2OH-0003iF-8y for qemu-devel@nongnu.org; Wed, 17 Feb 2016 08:43:37 -0500 References: <1455665965-27638-1-git-send-email-eblake@redhat.com> <87h9h7iltx.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <56C47907.4000106@redhat.com> Date: Wed, 17 Feb 2016 06:43:35 -0700 MIME-Version: 1.0 In-Reply-To: <87h9h7iltx.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="KVkDsPFLOSeveUdwPKFQSWOjOdLepRxnI" Subject: Re: [Qemu-devel] [PATCH] qapi-visit: Honor prefix of discriminator enum 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) --KVkDsPFLOSeveUdwPKFQSWOjOdLepRxnI Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 02/17/2016 05:05 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> When we added support for a user-specified prefix for an enum >> type (commit 351d36e), we forgot to teach the qapi-visit code >> to honor that prefix in the case of using a prefixed enum as >> the discriminator for a flat union. While there is still some >> on-list debate on whether we want to keep prefixes, we should >> at least make it work as long as it is still part of the code >> base. >> > Let's check for completeness. Calls of c_enum_const(): >=20 > * QAPISchemaEnumType.c_null() and (with your patch) gen_visit_union() > call it like >=20 > c_enum_const(TYPE.name, MEMBER, TYPE.prefix) >=20 > where MEMBER is a member of enumeration type TYPE. >=20 > As your patch shows, the prefix is easy to forget. A safer function > would take just TYPE and MEMBER: >=20 > TYPE.c_member(MEMBER) Yes, that would be nicer, but more invasive... >=20 > * gen_event_send() calls >=20 > c_enum_const(event_enum_name, name) >=20 > where name is member of the enum type named event_enum_name. That's > okay because the type is auto-generated without a prefix. Regardless= , > we could do something like >=20 > schema.lookup_type(event_enum_name).c_member(name) >=20 > Requires actually constructing the type, which is probably a good ide= a > anyway, because it gets us the necessary collision checks. Replacing= > global event_enum_name by event_enum_type would be nice then. Out of= > scope for this patch. =2E..and that's why the quick patch now rather than a more invasive refactor is reasonable. I've added this refactor to my list of things that might be worth doing some later day, but certainly not before the current queue is flushed. >=20 > * gen_enum_lookup() and gen_enum() work on name, values, prefix instead= > of the type. I figure they do to support qapi-event.py. If we clean= > it up to create the type, these functions could use the type as well,= > and then c_enum_const() could be dropped. >=20 > Bottom line for this patch: the fix is complete. Good, your analysis matched mine. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --KVkDsPFLOSeveUdwPKFQSWOjOdLepRxnI 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/ iQEcBAEBCAAGBQJWxHkHAAoJEKeha0olJ0NqVz4H/2Mgd5tY2ggQ9lmJE5YUBxmR 0MiEFMELc3v0aIzQIKCRUy3Zp76symVmF8Ztl3UZCwdiaV6OZSM7O+rEkalW6M1Z Xk+UqEbx/Gf6YR1dcBA2cPxNwbgL5lITMqBdbWUZti6vISkWCQ4Y94Bs7iP4cJek bwI72Z7m0vpJR30/VfD3SftHLDfNj4gVG1683dU7inXSNq6C5tl4LuI0PPk+DK8/ jl4GLfwvAiDCIizIRxn59laZc4bpuuld0FL9k8oA2w7A2QqIRDyirPRFcwYYuT9L gxuDEpWj4hhXMVZBfaRDFbrNM1xZcfz9eHZQEUxvaxxZa/o3XyoRJjBilwIUvHE= =VAT0 -----END PGP SIGNATURE----- --KVkDsPFLOSeveUdwPKFQSWOjOdLepRxnI--