From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53634) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWTsn-00023S-Rz for qemu-devel@nongnu.org; Thu, 18 Feb 2016 14:04:59 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aWTsj-0002fi-FL for qemu-devel@nongnu.org; Thu, 18 Feb 2016 14:04:57 -0500 Received: from mx1.redhat.com ([209.132.183.28]:2787) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWTsj-0002fa-7H for qemu-devel@nongnu.org; Thu, 18 Feb 2016 14:04:53 -0500 References: <1455818725-7647-1-git-send-email-peter.maydell@linaro.org> <1455818725-7647-5-git-send-email-peter.maydell@linaro.org> From: Eric Blake Message-ID: <56C615D3.7080303@redhat.com> Date: Thu, 18 Feb 2016 12:04:51 -0700 MIME-Version: 1.0 In-Reply-To: <1455818725-7647-5-git-send-email-peter.maydell@linaro.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="LVtXjUmBph0lweNCQ9ow18NUrqK2qj1SG" Subject: Re: [Qemu-devel] [PATCH 4/8] scripts/clean-includes: Enhance to handle header files List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell , qemu-devel@nongnu.org Cc: patches@linaro.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --LVtXjUmBph0lweNCQ9ow18NUrqK2qj1SG Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 02/18/2016 11:05 AM, Peter Maydell wrote: > Enhance clean-includes to handle header files as well as .c source > files. For headers we merely remove all the redundant #include > lines, including any includes of qemu/osdep.h itself. >=20 > There is a simple mollyguard on the include file processing to > skip a few key headers like osdep.h itself, to avoid producing > bad patches if the script is run on every file in include/. >=20 > Signed-off-by: Peter Maydell > Reviewed-by: Eric Blake > --- > scripts/clean-includes | 50 ++++++++++++++++++++++++++++++++++++++++++= -------- > 1 file changed, 42 insertions(+), 8 deletions(-) I already saw your followup explaining about the spurious R-b, so let's make it real this time :) >=20 > diff --git a/scripts/clean-includes b/scripts/clean-includes > index 1af1f82..737a5ce 100755 > --- a/scripts/clean-includes > +++ b/scripts/clean-includes > @@ -1,7 +1,8 @@ > #!/bin/sh -e > # > # Clean up QEMU #include lines by ensuring that qemu/osdep.h > -# is the first include listed. > +# is the first include listed, and no headers provided by > +# osdep.h itself are redundantly included. Do you want to mention 'is the first include listed in .c files', now that this cleanup also scrubs .h files? Or, since you go into details below and this is just the summary, maybe 'is the first include encountered'? > # > # Copyright (c) 2015 Linaro Limited Want to add 2016? > # > @@ -22,6 +23,11 @@ > =20 > # This script requires Coccinelle to be installed. > =20 > +# .c files will have the osdep.h included added, and redundant > +# includes removed. > +# .h files will have redundant includes (including includes of osdep.h= ) > +# removed. Maybe a note here that "this is because any other .h file will not be included by a .c file until after osdep.h" ? Or is that too verbose? > + case "$f" in > + *.c) > + MODE=3Dc > + ;; > + *include/qemu/osdep.h|include/qemu/compiler.h|include/config.h|inc= lude/standard-headers/) Spaces around | may make this more legible, and doesn't affect correctnes= s. > + > + if [ "$MODE" =3D "c" ]; then > + # First, use coccinelle to add qemu/osdep.h before the first exist= ing include Should the tool name be capitalized as Coccinelle? > + # (this will add two lines if the file uses both "..." and <...> #= includes, > + # but we will remove the extras in the next step) > + spatch --in-place --no-show-diff --cocci-file "$COCCIFILE" "$f" > + > + # Now remove any duplicate osdep.h includes > + perl -n -i -e 'print if !/#include "qemu\/osdep.h"/ || !$n++;' "$= f" Why two spaces before -e? I can understand the use of perl here (detecting duplicates lines is doable, but a lot harder, in sed),... > + else > + # Remove includes of osdep.h itself > + perl -n -i -e 'print if !/\s*#\s*include\s*(["<][^>"]*[">])/ || > + ! (grep { $_ eq $1 } qw ("qemu/osdep.h"))'= "$f" =2E..but this could be shortened, if you wanted: sed -i '/#[[:space:]]*include[[:space:]]*["<]qemu/osdep.h[">]/d' "$f" My findings are minor; functionally, your patch is sane, so the accidental R-b can now be treated as real, if you don't want to respin. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --LVtXjUmBph0lweNCQ9ow18NUrqK2qj1SG 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/ iQEcBAEBCAAGBQJWxhXTAAoJEKeha0olJ0Nq3wIIAIM7m5CRJgzejeVRMSYnN2wz dTIK0SmWn8BM8iLtpVeujJscAfxH68bcre/MSs99y+69aYHdH7HcSRfK8kqBAlgU 99V81wiEW8KGiaIpEO6K+iglaTLFf1ydphZlk6rxpYhMuq1zsW4Rh3RDszWqcpGC xcdkqjX/Nc2s/ENrHvEVyUlvfzl+0CNEFdC8N+IsYMUePsFUZGjEYGq3PbPl1pN+ r1tUXTBa0NIkXawxufr5cmpOBInuVHIudNYkZnGaHHocEk6ACUIXNNM7rJaHAqNY PdFHEtO2zyrxNRaFlFLNlve6UDJDieHOMHOMZZH6/PUwBs2WHqJylzWAG/n/7zU= =mPuF -----END PGP SIGNATURE----- --LVtXjUmBph0lweNCQ9ow18NUrqK2qj1SG--