FYI, zgrep -f - vs. portability

[Jim Meyering]

I've applied Carl's patch, and then adjusted it
so that zgrep is more portable, but anything that
pokes around in /proc/$$/fd can't claim much in the
way of portability, so it's sure to fail elsewhere.
If someone can propose a better way (patches welcome!),
or point out a system on which this approach does not work,
please speak up.  I tested on recent Linux and Solaris 10.

>From 5b54db4546b84ec97ff57a62f8ddb98faacf77f2 Mon Sep 17 00:00:00 2001
From: Carl Worth <address@hidden>
Date: Fri, 9 Oct 2009 17:32:48 +0200
Subject: [PATCH 1/3] zgrep: handle "-f -" the same way that it works with grep

Before this change, echo needle|zgrep -f - haystack.gz would not work.
* zgrep.in: When the pattern comes from stdin, redirect it to a
different file descriptor, since we're about to use stdin.
---
 zgrep.in |   23 ++++++++++++++++++++++-
 1 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/zgrep.in b/zgrep.in
index 590aea2..d30ec25 100644
--- a/zgrep.in
+++ b/zgrep.in
@@ -3,7 +3,7 @@
 # zgrep -- a wrapper around a grep program that decompresses files as needed
 # Adapted from a version sent by Charles Levert <address@hidden>

-# Copyright (C) 1998, 2001, 2002, 2006, 2007 Free Software Foundation
+# Copyright (C) 1998, 2001, 2002, 2006, 2007, 2009 Free Software Foundation
 # Copyright (C) 1993 Jean-loup Gailly

 # This program is free software; you can redistribute it and/or modify
@@ -52,6 +52,7 @@ escape='
 '
 operands=
 have_pat=0
+pat_on_stdin=0
 files_with_matches=0
 files_without_matches=0
 no_filename=0
@@ -97,6 +98,23 @@ while test $# -ne 0; do
     printf >&2 '%s: %s: option not supported\n' "$0" "$option"
     exit 2;;
   (-[ef]* | --file | --file=* | --reg*)
+    # 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.
+    case $optarg in
+    (" '-'" | " '/dev/stdin'" | " '/dev/fd/0'")
+      pat_on_stdin=1
+      # Start search from 6 since the script already uses 3 and 5
+      for fd in $(seq 6 254); do
+       if test ! -e /dev/fd/$fd; then
+         pat_fd=$fd
+         break;
+       fi
+      done
+      optarg=/dev/fd/$pat_fd;
+    esac
     have_pat=1;;
   (--h | --he | --hel | --help)
     echo "$usage" || exit 2
@@ -151,6 +169,9 @@ 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.6.5.rc3.227.g2ff1c


>From beb6a3e7a74a0415826752b25d418d2b54f3d49f Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 9 Oct 2009 20:03:09 +0200
Subject: [PATCH 2/3] zgrep: portability improvements; exercise "-f -"

* zgrep.in: Adjust loop not to use seq; it's not portable enough.
Fail if we don't find a free file descriptor.
(exists): New function; Use it in place of less portable "test -e".
Testing for existence of /dev/fd/$fd doesn't work on Solaris 10,
since all 256 always exist (as char devices), but testing for
/proc/$$/fd/$fd does work, so do that instead.
* Makefile.am (TESTS): Add tests/zgrep-f.
* tests/zgrep-f: New test; exercise this bug.
* NEWS (Bug fixes): Mention it.
---
 Makefile.am   |    3 ++-
 NEWS          |    2 ++
 tests/zgrep-f |   38 ++++++++++++++++++++++++++++++++++++++
 zgrep.in      |   20 +++++++++++++++-----
 4 files changed, 57 insertions(+), 6 deletions(-)
 create mode 100644 tests/zgrep-f

diff --git a/Makefile.am b/Makefile.am
index af75eba..206d5bf 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -101,7 +101,8 @@ check-local: $(FILES_TO_CHECK) $(bin_PROGRAMS) gzip.doc.gz
        @echo 'Test succeeded.'

 TESTS =                \
-  tests/zdiff
+  tests/zdiff  \
+  tests/zgrep-f
 EXTRA_DIST += $(TESTS)

 install-exec-hook: remove-installed-links
diff --git a/NEWS b/NEWS
index 20e09d8..5071241 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,8 @@ GNU gzip NEWS                                    -*- outline -*-

   zdiff would fail to print differences in two compressed inputs

+  zgrep -f - didn't work
+

 * Noteworthy changes in release 1.3.13 (2009-09-30) [stable]

diff --git a/tests/zgrep-f b/tests/zgrep-f
new file mode 100644
index 0000000..9184b9c
--- /dev/null
+++ b/tests/zgrep-f
@@ -0,0 +1,38 @@
+#!/bin/sh
+# Ensure that zgrep -f - works like grep -f -
+# Before gzip-1.4, it would fail.
+
+# Copyright (C) 2009 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+# limit so don't run it by default.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  zgrep --version
+fi
+
+. $srcdir/tests/test-lib.sh
+
+echo needle > n || framework_failure
+echo needle > haystack || framework_failure
+gzip haystack || framework_failure
+
+
+fail=0
+zgrep -f - haystack.gz < n > out 2>&1 || fail=1
+
+compare out n || fail=1
+
+Exit $fail
diff --git a/zgrep.in b/zgrep.in
index d30ec25..aced372 100644
--- a/zgrep.in
+++ b/zgrep.in
@@ -106,12 +106,22 @@ while test $# -ne 0; do
     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
-      for fd in $(seq 6 254); do
-       if test ! -e /dev/fd/$fd; then
-         pat_fd=$fd
-         break;
-       fi
+      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;
     esac
--
1.6.5.rc3.227.g2ff1c


>From 1a085b1446a23bead55437a131fade8e26051fe5 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 9 Oct 2009 20:12:30 +0200
Subject: [PATCH 3/3] build: enable automake color- and parallel-test options

* configure.ac (AM_INIT_AUTOMAKE): Enable color-tests and parallel-tests.
---
 configure.ac |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/configure.ac b/configure.ac
index 9b127d2..174a59e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -27,7 +27,7 @@ AC_INIT([gzip],
 AC_CONFIG_SRCDIR(gzip.c)
 AC_CONFIG_AUX_DIR(build-aux)
 AC_CONFIG_HEADERS([lib/config.h:lib/config.hin])
-AM_INIT_AUTOMAKE([1.11 dist-xz])
+AM_INIT_AUTOMAKE([1.11 dist-xz color-tests parallel-tests])
 AM_SILENT_RULES([yes]) # make --enable-silent-rules the default.

 AC_PROG_CC_STDC
--
1.6.5.rc3.227.g2ff1c