From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49628) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZHKZM-0007R4-HY for qemu-devel@nongnu.org; Mon, 20 Jul 2015 19:34:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZHKZI-0004Fq-FN for qemu-devel@nongnu.org; Mon, 20 Jul 2015 19:34:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42542) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZHK9P-0002JI-Dt for qemu-devel@nongnu.org; Mon, 20 Jul 2015 19:07:11 -0400 References: <1435782155-31412-1-git-send-email-armbru@redhat.com> <1435782155-31412-8-git-send-email-armbru@redhat.com> From: Eric Blake Message-ID: <55AD7F18.1010205@redhat.com> Date: Mon, 20 Jul 2015 17:07:04 -0600 MIME-Version: 1.0 In-Reply-To: <1435782155-31412-8-git-send-email-armbru@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="pOeWF0ho85McwU7u5oBnWfePvr5pPVf9s" Subject: Re: [Qemu-devel] [PATCH RFC v2 07/47] qapi: Generate a nicer struct for flat unions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , qemu-devel@nongnu.org Cc: kwolf@redhat.com, berto@igalia.com, mdroth@linux.vnet.ibm.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --pOeWF0ho85McwU7u5oBnWfePvr5pPVf9s Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 07/01/2015 02:21 PM, Markus Armbruster wrote: > The struct generated for a flat union is weird: the members of its > base are at the end, except for the union tag, which is renamed to > 'kind' and put at the beginning. >=20 > Change to put all base members at the beginning, unadulterated. Not > only is this easier to understand, it also permits casting the flat > union to its base, if that should become useful. >=20 > We now generate: >=20 > struct UserDefFlatUnion > { It might be worth tweaking the generator to output a C comment either here (at the start of the larger struct)... > char *string; > EnumOne enum1; =2E..or here (at the end of the base struct) mentioning that UserDefFlatUnion can be cast into its base struct UserDefUnionBase, to make it easier when reading the generated code to trace back to that "inheritance" relationship. Right now, there is nothing in the generated UserDefFlatUnion that points you back to the qapi relationship of a base class. But it's not a show-stopper if you don't like my suggestion. > /* union tag is EnumOne enum1 */ > union { > void *data; > UserDefA *value1; > UserDefB *value2; > UserDefB *value3; > }; > }; Only impact in files generated for qemu was to struct BlockdevOptions, in the files qapi-types.[ch], and I agree that it is an improvement. Oddly enough, it seems none of the C code cared about the field being renamed from 'kind' to 'driver' (I guess that's because a lot of our use of the blockdev code is not directly through the generated C structs, but through QObject dictionary queries). >=20 > Signed-off-by: Markus Armbruster > --- > scripts/qapi-types.py | 32 ++++++++++++++++++-------------- > scripts/qapi-visit.py | 7 +++++-- > tests/test-qmp-input-visitor.c | 2 +- > tests/test-qmp-output-visitor.c | 2 +- > 4 files changed, 25 insertions(+), 18 deletions(-) >=20 > +''', > + name=3Dname) > + if base: > + base_fields =3D find_struct(base)['data'] > + ret +=3D generate_struct_fields(base_fields) > - if base: > - assert discriminator > - base_fields =3D find_struct(base)['data'].copy() > - del base_fields[discriminator] > - ret +=3D generate_struct_fields(base_fields) I also like the fact that you no longer modify the base_fields array (because you are no longer floating a single renamed element out-of-order compared to the rest of the base), and therefore avoid the need for a .copy(). Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --pOeWF0ho85McwU7u5oBnWfePvr5pPVf9s 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/ iQEcBAEBCAAGBQJVrX8YAAoJEKeha0olJ0Nq9C4IAI1yFUz045wVWDKRP4zGfbK4 MSlTL26uV156I9Uy/GUuTorc/kBx7+9XvJmlajhFGQ2wGYpBUaFPVaF5XQOYgAXJ rHbCykEoIr23Vv0MoyY8IMJi0eZ2fDkgM8Mrlvf78zZJbXwyNLqlfVWNjP/jwRNf vIk+8lmehMn17Tl8qJoR/a+uinBTSYYlWcn/14xHh3ZUaj3bCOJTomfLkcYCyjPv lW2ouNjptcAXjAOIX4sEdKCeqFdTFWl1bn/J/GECxgVTkqyh0e2uXOUSLskP/T9/ 0szL9o4iZmSLvFdDgiULSjHBpQEv/+W2RACvMPp/ZXGIfODX19vCQ1T+nGB4SsI= =b5+f -----END PGP SIGNATURE----- --pOeWF0ho85McwU7u5oBnWfePvr5pPVf9s--