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 <aligu...@us.ibm.com> > --- > 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
