From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43890) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZHIOF-00080T-9C for qemu-devel@nongnu.org; Mon, 20 Jul 2015 17:14:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZHIOB-0000GA-5G for qemu-devel@nongnu.org; Mon, 20 Jul 2015 17:14:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47751) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZHIOA-0000G5-TB for qemu-devel@nongnu.org; Mon, 20 Jul 2015 17:14:19 -0400 References: <1435782155-31412-1-git-send-email-armbru@redhat.com> <1435782155-31412-7-git-send-email-armbru@redhat.com> From: Eric Blake Message-ID: <55AD64A4.1080809@redhat.com> Date: Mon, 20 Jul 2015 15:14:12 -0600 MIME-Version: 1.0 In-Reply-To: <1435782155-31412-7-git-send-email-armbru@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="H0bwIqFlXfWgQXWFlB0E2OcGtgEExwH0M" Subject: Re: [Qemu-devel] [PATCH RFC v2 06/47] qapi: Drop unused and useless parameters and variables 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) --H0bwIqFlXfWgQXWFlB0E2OcGtgEExwH0M Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 07/01/2015 02:21 PM, Markus Armbruster wrote: > gen_sync_call()'s parameter indent is useless: gen_sync_call() uses it > only as optional argument for push_indent() and pop_indent(), their > default is four, and gen_sync_call()'s only caller passes four. >=20 > gen_visitor_input_containers_decl()'s parameter obj is always > "QOBJECT(args)". It might be nice to call out that several other functions are also stripped of unused arguments. I was assuming that only two functions (and their callers) would be modified, but the patch touched more: >=20 > Signed-off-by: Markus Armbruster > --- > scripts/qapi-commands.py | 27 +++++++++++++-------------- > scripts/qapi-event.py | 1 - > scripts/qapi-types.py | 1 - > scripts/qapi-visit.py | 47 ++++++++++++++++++++++------------------= ------- > scripts/qapi.py | 1 - > 5 files changed, 35 insertions(+), 42 deletions(-) > @@ -161,7 +160,7 @@ qapi_dealloc_visitor_cleanup(md); > pop_indent() > return ret.rstrip() > =20 > -def gen_marshal_output(name, args, ret_type, middle_mode): > +def gen_marshal_output(name, ret_type): > if not ret_type: > return "" For example, gen_marshal_output() was not mentioned in the commit message= =2E > =20 > @@ -194,14 +193,14 @@ out: > =20 > return ret > =20 > -def gen_marshal_input_decl(name, args, ret_type, middle_mode): > +def gen_marshal_input_decl(name, middle_mode): Or gen_marshal_input_decl() > +++ b/scripts/qapi-event.py > @@ -199,7 +199,6 @@ const char *%(event_enum_name)s_lookup[] =3D { > ''', > event_enum_name =3D event_enum_name) > =20 > - i =3D 0 > for string in event_enum_strings: I guess the subject line covered deletion of unused internal variables. > +++ b/scripts/qapi-visit.py > @@ -441,44 +440,42 @@ fdecl.write(guardend("QAPI_VISIT_BUILTIN_VISITOR_= DECL")) > elif expr.has_key('union'): > ret =3D generate_visit_union(expr) > - ret +=3D generate_visit_list(expr['union'], expr['data']) > + ret +=3D generate_visit_list(expr['union']) > fdef.write(ret) > =20 > enum_define =3D discriminator_find_enum_define(expr) > ret =3D "" > if not enum_define: > - ret =3D generate_decl_enum('%sKind' % expr['union'], > - expr['data'].keys()) > - ret +=3D generate_declaration(expr['union'], expr['data']) > + ret =3D generate_decl_enum('%sKind' % expr['union']) > + ret +=3D generate_declaration(expr['union']) As long as you are touching this, generate_decl_enum(expr['union'] + 'Kind') would read nicer. > fdecl.write(ret) > elif expr.has_key('alternate'): > ret =3D generate_visit_alternate(expr['alternate'], expr['data= ']) > - ret +=3D generate_visit_list(expr['alternate'], expr['data']) > + ret +=3D generate_visit_list(expr['alternate']) > fdef.write(ret) > =20 > - ret =3D generate_decl_enum('%sKind' % expr['alternate'], > - expr['data'].keys()) > - ret +=3D generate_declaration(expr['alternate'], expr['data'])= > + ret =3D generate_decl_enum('%sKind' % expr['alternate']) Same here. At any rate, no change to generated output. So assuming you are happy with the commit message, or take my advice to improve it, the code cleanup itself is fine. Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --H0bwIqFlXfWgQXWFlB0E2OcGtgEExwH0M 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/ iQEcBAEBCAAGBQJVrWSkAAoJEKeha0olJ0Nq9VgH/2V8i93dfUkBG0/crLY5vwgK dQRmMztkxNviTlcP++QNxRIdx+Kjt2sjYN36XJP9WL/2+ZVJ7PC1FT6n4MtDtBEM 24ppDP4ZB8ivuYVFphYcMHhFc5Naxp3fya/z5jbj52cOed6Aw53trEU2nkQU5D66 zQa+Fr+qupl6INvsUfvbqunQmpOTl0zswm8At7aT21Pwij30n7OY6z0ljAluu+al b1mYLE0/uRTp/hjUvZ38cPlcTVdh3LCo9T5/jzKwZJ3jIhkQna0AsVxMQOrZNMCT VRK6Qj3Gy+OssilV1h8AHgYeIQIcv5iFkRvJjYnajcb22dS0m/u2vwu5kCZqdRY= =7uHz -----END PGP SIGNATURE----- --H0bwIqFlXfWgQXWFlB0E2OcGtgEExwH0M--