[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
signature.asc
Description: OpenPGP digital signature