From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53387) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZIIDD-0003mp-EH for qemu-devel@nongnu.org; Thu, 23 Jul 2015 11:15:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZIID9-0001Cp-4a for qemu-devel@nongnu.org; Thu, 23 Jul 2015 11:15:07 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43900) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZIID8-0001AP-US for qemu-devel@nongnu.org; Thu, 23 Jul 2015 11:15:03 -0400 References: <1435782155-31412-1-git-send-email-armbru@redhat.com> <1435782155-31412-33-git-send-email-armbru@redhat.com> From: Eric Blake Message-ID: <55B104F0.8030301@redhat.com> Date: Thu, 23 Jul 2015 09:14:56 -0600 MIME-Version: 1.0 In-Reply-To: <1435782155-31412-33-git-send-email-armbru@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="tjfwAT0AlunOMQ2IJQvGRkPGVMNTo70wA" Subject: Re: [Qemu-devel] [PATCH RFC v2 32/47] qapi-event: Convert to QAPISchemaVisitor, fixing data with base 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) --tjfwAT0AlunOMQ2IJQvGRkPGVMNTo70wA Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 07/01/2015 02:22 PM, Markus Armbruster wrote: > Fixes events whose data is struct with base to include the struct's > base members. Test case is qapi-schema-test.json's event > __org.qemu_x-command: >=20 > { 'event': '__ORG.QEMU_X-EVENT', 'data': '__org.qemu_x-Struct' } >=20 > { 'struct': '__org.qemu_x-Struct', 'base': '__org.qemu_x-Base', > 'data': { '__org.qemu_x-member2': 'str' } } >=20 > { 'struct': '__org.qemu_x-Base', > 'data': { '__org.qemu_x-member1': '__org.qemu_x-Enum' } } No change to generated code in qemu proper, so this is a corner case we are not yet exploiting. But good to have it fixed :) >=20 > Patch's effect on generated qapi_event_send___org_qemu_x_event(): >=20 > -void qapi_event_send___org_qemu_x_event(const char *__org_qemu_x_m= ember2, > +void qapi_event_send___org_qemu_x_event(__org_qemu_x_Enum __org_qe= mu_x_member1, > + const char *__org_qemu_x_m= ember2, > Error **errp) Ouch - I think we have a bug in qapi.py:check_event(). There, we call check_type(... allow_metas=3D['union', 'struct']) - but it looks like the= generated signature requires that we have no variants, which means we cannot have: { 'union': 'Un', 'data': ... } { 'event': 'EV', 'data': 'Un' } because it would fail C generation. Sounds like you should add a check to that in 14/47, and a fix for it in 15/47. > Signed-off-by: Markus Armbruster > --- > scripts/qapi-event.py | 87 ++++++++++++++++---------= -------- > tests/qapi-schema/qapi-schema-test.json | 3 -- > 2 files changed, 43 insertions(+), 47 deletions(-) >=20 > diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py > index 537da17..456e590 100644 > --- a/scripts/qapi-event.py > +++ b/scripts/qapi-event.py > @@ -2,28 +2,29 @@ > # QAPI event generator > # > # Copyright (c) 2014 Wenchao Xia > +# Copyright (c) 2013-2015 Red Hat Inc. Umm - the file didn't exist until 2014; and to my knowledge, you aren't adding anything to it that was copied from some other file dating back to 2013. Using the range 2014-2015 might be better. But that's minor. And assuming that you reject union types for events in an earlier patch, the rest of this is fine: Reviewed-by: Eric Blake > @@ -89,15 +90,15 @@ def generate_event_implement(api_name, event_name, = params): > """, > event_name =3D event_name) > =20 > - for argname, argentry, optional in parse_args(params): > - if optional: > + for memb in params.members: > + if memb.optional: > ret +=3D mcgen(""" > if (has_%(var)s) { > """, > - var =3D c_name(argname)) > + var=3Dc_name(memb.name)) > push_indent() > =20 > - if argentry =3D=3D "str": > + if memb.type.name =3D=3D "str": > var_type =3D "(char **)" Our visitors require us to cast away const. Is that something we should consider reworking, so that we don't need to do that? But it's a question for another day. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --tjfwAT0AlunOMQ2IJQvGRkPGVMNTo70wA 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/ iQEcBAEBCAAGBQJVsQTwAAoJEKeha0olJ0NqcBsH+gMbm7HtF2WgRs9QYUD13kBl bfG6K6anwpBPR42twNcHqA5rq/w6TN0PG1qSeowD1ro5z9/erHL76FgXqhaCRGro fdVnQXMV5+mtbBWA0FvOdOWnSgR5GQA749/BOP3LC9GyMVUVdFe034TsexzqakS6 ZzX3F1h0RHNxytW/RLv9LBFFMihd/CtRxTjhh8HlO6s925TsKpa5wiJZeZUqY9Wv Erzll4KzTchVIfaG5V64riQKcpmO1fkWJuBLqICeKPENwbcfz8IDlGnPyJDrcxzi Y5+RV7ug922cSNJBMRZIKlZCGeDj+ZTH+041haZZM3uoEsnXNLvavzkA7OjAEOM= =ixvn -----END PGP SIGNATURE----- --tjfwAT0AlunOMQ2IJQvGRkPGVMNTo70wA--