Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice

[Johannes Sixt <j6t@kdbg.org>]

Am 05.09.2014 12:06, schrieb Chris Packham:
> In check_patch_format we feed $1 to a block that attempts to determine
> the patch format. Since we've already redirected $1 to stdin there is no
> need to redirect it again when we invoke tr. This prevents the following
> errors when invoking git am
> 
>   $ git am patch.patch
>   tr: write error: Broken pipe
>   tr: write error
>   Patch format detection failed.
> 
> Cc: Stephen Boyd <bebarino@gmail.com>
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
> ---
> Nothing new since http://article.gmane.org/gmane.comp.version-control.git/256425
> 
>  git-am.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/git-am.sh b/git-am.sh
> index ee61a77..fade7f8 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -250,7 +250,7 @@ check_patch_format () {
>  			# discarding the indented remainder of folded lines,
>  			# and see if it looks like that they all begin with the
>  			# header field names...
> -			tr -d '\015' <"$1" |
> +			tr -d '\015' |
>  			sed -n -e '/^$/q' -e '/^[ 	]/d' -e p |
>  			sane_egrep -v '^[!-9;-~]+:' >/dev/null ||
>  			patch_format=mbox
> 

I think this change is wrong. This pipeline checks whether one of the
lines at the top of the file contains something that looks like an email
header. With your change, the first three lines would not be looked at
because they were already consumed earlier.

I wonder why tr (assuming it is *this* instance of tr) dies with a write
error instead of from a SIGPIPE. Is SIGPIPE ignored somewhere and then
the tr invocation inherits this "ignore SIGPIPE" setting?

The only thing your version changes is that tr writes a bit less text
into the pipe. Perhaps its just sufficient that the output fits into the
pipe buffer, and no error occurs anymore? Then the new version is not a
real fix: make the patch text a bit longer, and the error is back.

-- Hannes