qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v2] checkpatch: add a little script to run check


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v2] checkpatch: add a little script to run checkpatch against a git refspec
Date: Mon, 21 Jan 2013 14:17:17 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2

On 01/21/2013 12:20 PM, Anthony Liguori wrote:
> This makes it easier to use checkpatch with a git hook or via patches.
> 
> Signed-off-by: Anthony Liguori <address@hidden>
> ---
> v1 -> v2
>  - Add the subject to the output to indicate which patch failed
>  - Only display output for patches that fail the check
> ---
>  scripts/check-patches.sh | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100755 scripts/check-patches.sh
> 

> +TMPFILE=/tmp/check-patches.out.$$

This name is highly predictable, and thus rather insecure.  Is it worth
using common idioms for safer temporary files?

> +
> +ret=0
> +git log --format="%H %s" "$@" | while read LINE; do
> +    commit="`echo $LINE | cut -f1 -d' '`"
> +    subject="`echo $LINE | cut -f2- -d' '`"
> +    echo "Subject: $subject"

This won't work if $subject contains backslash.  You must use printf(1)
to be portable here.

> +    git show --format=email $commit | scripts/checkpatch.pl - > $TMPFILE
> +    
> +    rc=$?
> +    if test $rc -ne 0; then
> +     ret=rc
> +     cat $TMPFILE
> +     echo
> +    fi
> +done
> +
> +rm -f $TMPFILE
> 

Where do you use $ret?  Also, you are executing the assignment to ret
inside a while loop that was part of a pipeline, but POSIX says a
compliant shell might execute that loop in a subshell (and bash does
just that), so that the parent shell cannot not see the change in the
value of $ret.  If you really must propagate errors outside of the while
loop, then instead of doing:

command | while read; done
use results from loop

you have to instead use an alternative such as:

command | { while read; done
  use results from loop
}

or a named fifo (but that gets back to my question of how do you intend
to generate a secure name for your fifo).

http://mywiki.wooledge.org/BashFAQ/024

-- 
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]