Re: gzip 1.4 make check error

[Paul Eggert]

On 11/09/10 06:58, Thomas Schulz wrote:

> The first line in indeed '#!/bin/ksh'.

Thanks for checking that.  I verified the bug on my Solaris 8 sparc
host, and installed the following patch.  I'll CC: this to Carl Worth,
as I think he wrote the code in question, in the hopes that he can
point out any gotchas in this fix.

>From fed8ea269a1ee84cb8fa54c338985667284a7004 Mon Sep 17 00:00:00 2001
From: Paul Eggert <address@hidden>
Date: Tue, 9 Nov 2010 13:17:57 -0800
Subject: [PATCH] zgrep: fix shell portability bug with -f; fix mishandling of 
"-e -"

* tests/zgrep-f: Check for "zgrep -e -" bug, too.
* zgrep.in: Don't assume that if the shell redirects fd 6, then
this redirection is visible to the subsidiary grep.  POSIX doesn't
guarantee this visibility except for file descriptors 0, 1, and 2,
and ksh does not support it.  Problem reported by Thomas Schulz in
<http://lists.gnu.org/archive/html/bug-gzip/2010-11/msg00000.html>.
Also, fix a related bug: "-e -" was mishandled.  These two bugs
were introduced by commit 5b54db4546b84ec97ff57a62f8ddb98faacf77f2
dated 2009-10-09.
(escape): Change the convention: do not assume that a stray X
is present at the end of the last line.  All uses changed.
There was no longer any need for this convention, and fixing this
made it a bit easier to use 'sed' in a later part of the fix.
---
 tests/zgrep-f |    3 ++
 zgrep.in      |   61 +++++++++++++++++++++++++-------------------------------
 2 files changed, 30 insertions(+), 34 deletions(-)

diff --git a/tests/zgrep-f b/tests/zgrep-f
index ba76bfd..ec8cf77 100644
--- a/tests/zgrep-f
+++ b/tests/zgrep-f
@@ -31,4 +31,7 @@ zgrep -f - haystack.gz < n > out 2>&1 || fail=1
 
 compare out n || fail=1
 
+# This failed with gzip 1.4.
+echo a-b | zgrep -e - > /dev/null || fail=1
+
 Exit $fail
diff --git a/zgrep.in b/zgrep.in
index 6afa62c..68396db 100644
--- a/zgrep.in
+++ b/zgrep.in
@@ -47,14 +47,13 @@ OPTIONs are the same as for 'grep'.
 Report bugs to <address@hidden>."
 
 # sed script to escape all ' for the shell, and then (to handle trailing
-# newlines correctly) turn trailing X on last line into '.
+# newlines correctly) append ' to the last line.
 escape='
   s/'\''/'\''\\'\'''\''/g
-  $s/X$/'\''/
+  $s/$/'\''/
 '
 operands=
 have_pat=0
-pat_on_stdin=0
 files_with_matches=0
 files_without_matches=0
 no_filename=0
@@ -67,7 +66,7 @@ while test $# -ne 0; do
 
   case $option in
   (-[0123456789EFGHIKLPRTUVZabchilnoqrsuvwxyz]?*)
-    arg2=-\'$(expr "X${option}X" : 'X-.[0-9]*\(.*\)' | sed "$escape")
+    arg2=-\'$(expr "X$option" : 'X-.[0-9]*\(.*\)' | sed "$escape")
     eval "set -- $arg2 "'${1+"$@"}'
     option=$(expr "X$option" : 'X\(-.[0-9]*\)');;
   (--binary-*=* | --[lm]a*=* | --reg*=*)
@@ -75,11 +74,23 @@ while test $# -ne 0; do
   (-[ABCDXdefm] | binary-* | --file | --[lm]a* | --reg*)
     case ${1?"$option option requires an argument"} in
     (*\'*)
-      optarg=" '"$(printf '%sX\n' "$1" | sed "$escape");;
+      optarg=" '"$(printf '%s\n' "$1" | sed "$escape");;
     (*)
       optarg=" '$1'";;
     esac
     shift;;
+  (-f?*\'*)
+    optarg=" '"$(expr "X$option" : 'X-f\(.*\)' | sed "$escape")
+    option=-f;;
+  (-f?*)
+    optarg=" '"$(expr "X$option" : 'X-f\(.*\)')\'
+    option=-f;;
+  (--file=*\'*)
+    optarg=" '"$(expr "X$option" : 'X--file=\(.*\)' | sed "$escape")
+    option=--file;;
+  (--file=*)
+    optarg=" '"$(expr "X$option" : 'X--file=\(.*\)')\'
+    option=--file;;
   (--)
     break;;
   (-?*)
@@ -87,7 +98,7 @@ while test $# -ne 0; do
   (*)
     case $option in
     (*\'*)
-      operands="$operands '"$(printf '%sX\n' "$option" | sed "$escape");;
+      operands="$operands '"$(printf '%s\n' "$option" | sed "$escape");;
     (*)
       operands="$operands '$option'";;
     esac
@@ -99,33 +110,18 @@ while test $# -ne 0; do
   (-[drRzZ] | --di* | --exc* | --inc* | --rec* | --nu*)
     printf >&2 '%s: %s: option not supported\n' "$0" "$option"
     exit 2;;
-  (-[ef]* | --file | --file=* | --reg*)
+  (-e* | --reg*)
+    have_pat=1;;
+  (-f | --file)
     # The pattern is coming from a file rather than the command-line.
     # If the file is actually stdin then we need to do a little
-    # magic, (since we use stdin to pass the gzip output to grep).
-    # So find a free fd and change the argument to then use this
-    # file descriptor for the pattern.
+    # magic, since we use stdin to pass the gzip output to grep.
+    # Turn the -f option into an -e option by copying the file's
+    # contents into OPTARG.
     case $optarg in
     (" '-'" | " '/dev/stdin'" | " '/dev/fd/0'")
-      pat_on_stdin=1
-      eval 'test -e .' 2>/dev/null \
-        && eval 'exists(){ test -e "$@"; }' \
-        || eval 'exists(){ test -r "$@" || test -w "$@"; }'
-      # Start search from 6 since the script already uses 3 and 5
-      fd=6
-      pat_fd=
-      while : ; do
-        if ! exists /proc/$$/fd/$fd; then
-          pat_fd=$fd
-          break;
-        fi
-        fd=$(expr $fd + 1)
-        if test $fd = 255; then
-          printf >&2 '%s: no free file descriptor\n' "$0"
-          exit 2
-        fi
-      done
-      optarg=/dev/fd/$pat_fd;
+      option=-e
+      optarg=" '"$(sed "$escape") || exit 2;;
     esac
     have_pat=1;;
   (--h | --he | --hel | --help)
@@ -149,7 +145,7 @@ while test $# -ne 0; do
 
   case $option in
   (*\'?*)
-    option=\'$(expr "X${option}X" : 'X\(.*\)' | sed "$escape");;
+    option=\'$(printf '%s\n' "$option" | sed "$escape");;
   (*)
     option="'$option'";;
   esac
@@ -162,7 +158,7 @@ eval "set -- $operands "'${1+"$@"}'
 if test $have_pat -eq 0; then
   case ${1?"missing pattern; try \`$0 --help' for help"} in
   (*\'*)
-    grep="$grep -- '"$(printf '%sX\n' "$1" | sed "$escape");;
+    grep="$grep -- '"$(printf '%s\n' "$1" | sed "$escape");;
   (*)
     grep="$grep -- '$1'";;
   esac
@@ -181,9 +177,6 @@ do
   # Fail if gzip or grep (or sed) fails.
   gzip_status=$(
     exec 5>&1
-    if test $pat_on_stdin -eq 1; then
-      eval "exec $pat_fd<&0"
-    fi
     (gzip -cdfq -- "$i" 5>&-; echo $? >&5) 3>&- |
     if test $files_with_matches -eq 1; then
       eval "$grep" >/dev/null && { printf '%s\n' "$i" || exit 2; }
-- 
1.7.2