From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38818) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aGTQj-0001NU-Av for qemu-devel@nongnu.org; Tue, 05 Jan 2016 10:21:50 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aGTQg-0001LG-2t for qemu-devel@nongnu.org; Tue, 05 Jan 2016 10:21:49 -0500 Received: from mx1.redhat.com ([209.132.183.28]:34308) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aGTQf-0001Kz-RE for qemu-devel@nongnu.org; Tue, 05 Jan 2016 10:21:46 -0500 References: <1450717720-9627-1-git-send-email-eblake@redhat.com> <1450717720-9627-8-git-send-email-eblake@redhat.com> From: Eric Blake Message-ID: <568BDF82.10006@redhat.com> Date: Tue, 5 Jan 2016 08:21:38 -0700 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="5L30H09eEgmpPRBNQiqXHhJQQslGC46Pp" Subject: Re: [Qemu-devel] [PATCH v8 07/35] qapi: Improve generated event use of qapi visitor List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?Marc-Andr=c3=a9_Lureau?= Cc: Michael Roth , QEMU , Markus Armbruster This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --5L30H09eEgmpPRBNQiqXHhJQQslGC46Pp Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 01/05/2016 07:07 AM, Marc-Andr=C3=A9 Lureau wrote: > Hi >=20 > On Mon, Dec 21, 2015 at 6:08 PM, Eric Blake wrote: >> All other successful clients of visit_start_struct() were paired >> with an unconditional visit_end_struct(); but the generated >> code for events was relying on qmp_output_visitor_cleanup() to >> work on an incomplete visit. Alter the code to guarantee that >> the struct is completed, which will make a future patch to >> split visit_end_struct() easier to reason about. While at it, >> drop some assertions and comments that are not present in other >> uses of the qmp output visitor, rearrange the declaration to >> make it easier for a future patch to introduce the notion of >> a boxed event visit, and pass NULL rather than "" as the 'kind' >> parameter (matching most other uses where obj is NULL). >> >> Signed-off-by: Eric Blake >> >> --- >> v8: no change >> v7: place earlier in series, adjust handling of 'kind' >> v6: new patch >> >> If desired, I can defer the hunk re-ordering the declaration of >> obj to later in the series where it actually comes in handy. See [1] >> --- >> scripts/qapi-event.py | 14 ++++++-------- >> scripts/qapi.py | 5 +++-- >> 2 files changed, 9 insertions(+), 10 deletions(-) >> >> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py >> index 720486f..e37c07a 100644 >> --- a/scripts/qapi-event.py >> +++ b/scripts/qapi-event.py >> @@ -41,9 +41,9 @@ def gen_event_send(name, arg_type): >> >> if arg_type and arg_type.members: >> ret +=3D mcgen(''' >> + QObject *obj; >> QmpOutputVisitor *qov; >> Visitor *v; >> - QObject *obj; >=20 > This looks like churning code. This is my comment [1], and I am fine if we sink it to occur as part of https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04394.html >> >> - /* Fake visit, as if all members are under a structure */ >> - visit_start_struct(v, NULL, "", "%(name)s", 0, &err); >> + visit_start_struct(v, NULL, NULL, "%(name)s", 0, &err); >=20 > The comment seemed somewhat useful to me. The pattern visit_start_struct(v, NULL, NULL, name, 0, &err) occurs in other places in the code without the comment, so it felt like a common enough idiom that it didn't need special-casing here. Also, even if we want a comment, I didn't see a reason for the comment to be present in the generated code multiple times (generated once per event); at most, a simple Python '# comment' prior to the mcgen() would probably be sufficient if we still think the comment adds anything. >=20 > Reviewed-by: Marc-Andr=C3=A9 Lureau >=20 I guess when Markus gets back he can weigh in on it. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --5L30H09eEgmpPRBNQiqXHhJQQslGC46Pp 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/ iQEcBAEBCAAGBQJWi9+DAAoJEKeha0olJ0NqSuwIAJx0D7915N3ekdAszUXCDp19 iAJsHl8FxECoopQrhIajXg7X9wUk9OiEt8BVQujym9qfaXkmtdNja0/tT+w6ji6l svSqavZhQkTma5/PaTQ1HY+wbDm+v6pdQtfBb1Q123Gn29uAoO6nrX4/ocWXl40i antokcZ+MG2WtsBECwNJGqpn3z/LfixsIbCeIV5b+4sbWn+xvcJGETP7UGMQ4Qfa 2P/R2OoWjB+oHB9+rTPe0zK0jk4tQO3M4v2n31Rn54CsLtM37zWt1Y9ASsYDDDU7 WR0qhsanq2x3hi0E/TsEqZ6+ZGpohq9pu1G6kISUMoFWl4gmCbKtz+HUmrYyYk4= =BVoU -----END PGP SIGNATURE----- --5L30H09eEgmpPRBNQiqXHhJQQslGC46Pp--