Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/2] script/clean-includes: added duplicate #include check

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2142 bytes --]

On 10/20/2016 04:52 AM, Anand J wrote:
> Added script to check duplicate #include entries. This check will scan
> and print the files in which duplicate #include entries are present.

Subject line typo: s,script/,scripts/,

> 
> 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-duphead option with
> script/clean-includes.

Bike-shedding, but if you are going to put a dash between words, put it
between ALL words, as in --check-dup-head.  Or spell it out:
--check-duplicate-headers.  If that feels to long, and you want to
abbreviate, then abbreviate further, as in --duphead.

> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Anand J <anand.indukala@gmail.com>
> ---

>  #
> +# If --check-duphead option is used, then check for duplicate
> +# header files will be enabled.

Grammar; I suggest:

If --check-duphead is specified, additionally check for duplicate header
includes.

> +while true
> +do
> +    case $1 in
> +    "--git")
> +         if [ $# -eq 1 ]; then
> +             echo "--git option requires an argument"
> +             exit 1
> +         fi
> +         GITSUBJ="$2"
> +         GIT=yes
> +         shift
> +         shift
> +         ;;
> +    "--check-duphead")
> +        DUPHEAD=yes
> +        shift
> +        ;;
> +    *)

Your option-parsing loop is missing a special case for --; for
consistency with POSIX recommendations, you should have:

--)
  shift
  break
  ;;
*)

as a way to end option parsing early (and force the next argument to be
treated as a filename, even if it starts with -).

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

Missing quotes around $@.

Looking forward to v3.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]