From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56752) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZhdUT-00029O-PU for qemu-devel@nongnu.org; Thu, 01 Oct 2015 09:01:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZhdUP-00066p-2p for qemu-devel@nongnu.org; Thu, 01 Oct 2015 09:01:41 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44138) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZhdUO-00066e-Ri for qemu-devel@nongnu.org; Thu, 01 Oct 2015 09:01:37 -0400 References: <1443565276-4535-1-git-send-email-eblake@redhat.com> <1443565276-4535-16-git-send-email-eblake@redhat.com> <87k2r6zrqd.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <560D2EA7.8010602@redhat.com> Date: Thu, 1 Oct 2015 07:01:27 -0600 MIME-Version: 1.0 In-Reply-To: <87k2r6zrqd.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Ag01SjOxUMfwcaB5F66QALeowdV19iAD7" Subject: Re: [Qemu-devel] [PATCH v7 15/18] qapi: Share gen_err_check() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: marcandre.lureau@redhat.com, qemu-devel@nongnu.org, ehabkost@redhat.com, Michael Roth This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --Ag01SjOxUMfwcaB5F66QALeowdV19iAD7 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/01/2015 06:40 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> qapi-commands has a nice helper gen_err_check(), but did not >=20 > but does not use >=20 >> use it everywhere. In fact, using it in more places makes it >> easier to reduce the lines of code used for generating error >> checks. This in turn will make it easier for later patches >> to consolidate another common pattern among the generators. >=20 > Does this message need updating now you're using gen_err_check() less > aggressively? No - even though it was used less aggressively, it still made merging gen_visit_fields() easier, and "using it in more places" does not imply "in all possible places". So I think, other than the tense correction, we are okay. >=20 >> The generated code has one less blank line in qapi-event.c >> functions, but has no semantic difference. >=20 > Is it a stylistic improvement or the opposite? >=20 Hmm. In qapi-visit.c, there are no blank lines between visit_start_struct() and the matching visit_end_struct(). In qapi-event.c, pre-patch we had (roughly): visit_start_struct() check err visit fields =2E.. visit_end_struct() check err This patch deleted the first blank line (after visit_start_struct()) but not the second (prior to visit_end_struct()), so the post-patch looks a bit lopsided. But removing both blank lines doesn't seem too bad to me, if you wanted to squash this in (not sure the line numbers are right): diff --git i/scripts/qapi-event.py w/scripts/qapi-event.py index b5e4d59..720486f 100644 --- i/scripts/qapi-event.py +++ w/scripts/qapi-event.py @@ -73,7 +73,6 @@ def gen_event_send(name, arg_type): ret +=3D gen_err_check() ret +=3D gen_visit_fields(arg_type.members, need_cast=3DTrue) ret +=3D mcgen(''' - visit_end_struct(v, &err); if (err) { goto out; Or the alternative is to keep generated output unchanged, by keeping the newline: diff --git i/scripts/qapi-event.py w/scripts/qapi-event.py index b5e4d59..d261a46 100644 --- i/scripts/qapi-event.py +++ w/scripts/qapi-event.py @@ -71,6 +71,7 @@ def gen_event_send(name, arg_type): ''', name=3Dname) ret +=3D gen_err_check() + ret +=3D '\n' ret +=3D gen_visit_fields(arg_type.members, need_cast=3DTrue) ret +=3D mcgen(''' I'm fine if you want to do either (or nothing) when turning this into a pull request. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --Ag01SjOxUMfwcaB5F66QALeowdV19iAD7 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/ iQEcBAEBCAAGBQJWDS6nAAoJEKeha0olJ0Nqs84IAKMZOluMEDvOevvwjBSIBpwZ EEuMVbBl35DCZHkmb9CxjfRMU2sC8Hh2bi6eKS4BjpnK5FcwFiijdQJYUOGSWTwz kP0kNLSO6lbq0CC00IBSWdj/bJ+c9ckP278X0LVcMU5+bSe8Fb/cYwrDUf8Masmb z17VvqQBCbZswl6eyXk3j6HV0TfTLvVr5vTV3SqY1DUwCywqgc7Ae/uH8Eb/HHR1 /IA4Pdd/WKt4xrV/82B9/hWbgtTscabVAR900ElrYDY7l4Jo9uLq3dyhuttyiA5n zkqWj8E+9v1FIpxbuPROelsxFrBB6MlY1jSeR0ajSQriWtxviJ1wwv6XpXR/R9w= =4HM5 -----END PGP SIGNATURE----- --Ag01SjOxUMfwcaB5F66QALeowdV19iAD7--