From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60630) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ae6yY-0007pK-MO for qemu-devel@nongnu.org; Thu, 10 Mar 2016 15:14:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ae6yT-0005K1-Nx for qemu-devel@nongnu.org; Thu, 10 Mar 2016 15:14:26 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49435) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ae6yT-0005Jq-Fw for qemu-devel@nongnu.org; Thu, 10 Mar 2016 15:14:21 -0500 References: <1457571335-10938-1-git-send-email-eblake@redhat.com> <1457571335-10938-7-git-send-email-eblake@redhat.com> <87wppap3q2.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <56E1D59B.3010106@redhat.com> Date: Thu, 10 Mar 2016 13:14:19 -0700 MIME-Version: 1.0 In-Reply-To: <87wppap3q2.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="wnPwEew5jSc5EP91XdEEChCBCof7L9mAD" Subject: Re: [Qemu-devel] [PATCH v5 06/14] qapi-event: Slightly shrink generated code 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) --wnPwEew5jSc5EP91XdEEChCBCof7L9mAD Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 03/10/2016 11:50 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> Slightly rearrange the code in gen_event_send() for less generated >> output, by initializing 'emit' sooner, deferring an assertion >> to qdict_put_obj, and dropping a now-unused 'obj' local variable. >> >> While at it, document a FIXME related to the potential for a >> compiler naming collision - if the user ever creates a QAPI event >> whose name matches 'errp' or one of our local variables (like >> 'emit'), we'll have to revisit how we generate functions to >> avoid the problem. >> >=20 > We're not "deferring an assertion to qdict_put_obj()", we're dropping a= > dead one: qmp_output_get_qobject() never returns null. Oh, good point; I can improve the commit message. >=20 > I figure the assertion dates back to the time when it still did. Back > then, getting null here meant we screwed up. >=20 > I just searched the code for similarly dead assertions. Found one in > qapi_clone_InputEvent(), and serveral more in test-qmp-output-visitor.c= =2E Speaking of that, I have a patch pending (but not yet posted) that adds a clone visitor, so that we don't need qapi_clone_InputEvent() (it's rather wasteful to convert into and back out of QObject when you can just directly clone). >=20 > There's also an error return in qapi_copy_SocketAddress(). Useless? And that's the other hand-rolled clone that also gets nuked by my patch. Some obvious copy-and-paste between the two. > Should check for qnull instead? Not necessary; we can't return qnull unless we visit nothing (or, when my visit_type_null() lands, if we explicitly ask for it), but these callers are visiting something that is not null. >> %(proto)s >> { >> QDict *qmp; >> Error *err =3D NULL; >> - QMPEventFuncEmit emit; >> + QMPEventFuncEmit emit =3D qmp_event_get_func_emit(); >> ''', >> proto=3Dgen_event_send_proto(name, arg_type)) >> >> @@ -43,16 +49,13 @@ def gen_event_send(name, arg_type): >> ret +=3D mcgen(''' >> QmpOutputVisitor *qov; >> Visitor *v; >> - QObject *obj; >> - >=20 > Please keep the blank line here... >=20 >> ''') >> >> ret +=3D mcgen(''' >> - emit =3D qmp_event_get_func_emit(); >> + >=20 > ... instead of adding it here. Except that the next patch added one more declaration after Visitor *v, but not in direct text, where keeping the blank line unmoved would require splitting the mcgen() call into two parts. Or I could do ret +=3D= '\n'. >=20 >> if (!emit) { >> return; >> } >> - >=20 > Let's keep this one. Okay. >=20 >> qmp =3D qmp_event_build_dict("%(name)s"); >> >> ''', >> @@ -76,11 +79,7 @@ out_obj: >> if (err) { >> goto out; >> } >> - >> - obj =3D qmp_output_get_qobject(qov); >> - g_assert(obj); >> - >> - qdict_put_obj(qmp, "data", obj); >> + qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov)); >> ''') >> >> ret +=3D mcgen(''' >=20 > Small improvements are welcome, too :) >=20 --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --wnPwEew5jSc5EP91XdEEChCBCof7L9mAD 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/ iQEcBAEBCAAGBQJW4dWbAAoJEKeha0olJ0NqVSYIAIvHISMg1k+pIo6AGv7CzjAv ydkxgGPD24BeNvV/u3G4J6ukIkJevbwAkPMCJE+W2mVTPdL2srcZooArOICINl4G AafnW6jsPejD2nMgiWtCU+q8Deyi3MZ7lpzy+h3tTipPu+HaMs29exVEmm8x14xe jlF+U2hOtMyuuqtGl0IoMoQ4JePD1ekZZUjQwfwpXTDxGyB1oDFRih286kYr2i7F iALi5VYKHcvG3CHwzlqBGaC6oALfzEHy0J38pDrqpB0YGTFmY5UXlt+DuqsiOksU dXs4OV8KXgKJYNd/VvvCaD5eBOkBXZw7X/i9P0B5ZG1o2Kcvh2VPbXeJh2vtV8o= =f3G9 -----END PGP SIGNATURE----- --wnPwEew5jSc5EP91XdEEChCBCof7L9mAD--