qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [Qemu-devel] [PATCH v3 1/2] scripts/clean-includes: a


From: Eric Blake
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH v3 1/2] scripts/clean-includes: added duplicate #include check
Date: Thu, 20 Oct 2016 11:04:21 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0

On 10/20/2016 10:48 AM, Anand J wrote:
> Added script to check duplicate #include entries. This check will scan

You didn't actually add a script, but enhanced an existing one.  Also,
"Added" is past tense, but commit messages are best written in
imperative tense.  Better might be:

Enhance the clean-includes script to optionally check for duplicate
#include entries.

(But be careful of line wraps; git eats lines starting with # if the
wrap happens at the wrong place while editing your message)

> and print the files in which duplicate #include entries are present.
> 
> Script might output false positive entries as well. Such entries should
> not be removed. So if it finds any duplicate entries script will
> terminate with an exit status 1. Then each and every file should be
> checked manually and corrected if necessary.
> 
> In order to enable the check use --check-dup-head option with
> scripts/clean-includes.
> 
> Reviewed-by: Thomas Huth <address@hidden>
> Signed-off-by: Anand J <address@hidden>
> ---
>  scripts/clean-includes | 54 
> ++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 41 insertions(+), 13 deletions(-)
> 

> +if [ "$DUPHEAD" = "yes" ]; then
> +    grep "^#include" "$@" | sort | uniq -c | awk '{if ($1 > 1) print $0}'

Includes can be indented.  It is valid to write:
 #include ...
or
# include ...
or even
 # include ...

(of those three, we most commonly use the style:
# include ...
to delineate includes inside an #if conditional).

So a better expression might be grep "# *include" "$@", which may in
turn find more duplicates; on the other hand, those duplicates are more
likely to be conditionals where the duplication is necessary (see for
exampel include/qemu/bswap.h vs. <sys/endian.h>).  Then again, if you
allow whitespace in your grep, you have to remove that whitespace before
passing things to sort and uniq, if you want a fair comparison that
finds two duplicate includes that differ only by whitespace.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]