gzip: fix exit status on broken pipe

Fix gzip to behave like cat etc. when outputting to a broken pipe:
i.e., exit with nonzero status if SIGPIPE is ignored, and be
terminated by SIGPIPE otherwise.
* NEWS: Mention this.
* gzip.c: Do not install signal handlers unless creating an output
file, for which signal handlers are needed.  This avoids gzip
having to deal with signal handlers when outputting to stdout.
(exiting_signal): Remove.  All uses removed.
(main): Do not install signal handlers at first.
(create_outfile): Instead, install them only when needed.
(finish_up_gzip): New function, which generalizes abort_gzip.
(abort_gzip): Use it.
* tests/pipe-output: New test.
* tests/Makefile.am (TESTS): Add it.
* util.c (EPIPE): Default to 0.
(write_error): Just warn if it is a pipe error, and suppress that
warning if quiet.  In any event exit with status 2 (warning),
not status 1 (error).
* zgrep.in: Treat gzip status 141 like status 2;
it is a broken pipe either way.

7 files changed, 79 insertions, 18 deletions

diff --git a/NEWS b/NEWS
index 20c75fa..da3829d 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,13 @@ GNU gzip NEWS                                    -*- outline -*-
 
 * Noteworthy changes in release ?.? (????-??-??) [?]
 
+** Changes in behavior
+
+  When SIGPIPE is ignored, gzip now exits with status 2 (warning)
+  instead of status 1 (error) when writing to a broken pipe.  This is
+  more useful with programs like 'less' that treat gzip exit status 2
+  as a non-failure.
+
 ** Bug fixes
 
   'gzip -d' no longer fails to report invalid compressed data
diff --git a/gzip.c b/gzip.c
index 7547e19..7865a65 100644
--- a/gzip.c
+++ b/gzip.c
@@ -202,11 +202,6 @@ struct timespec time_stamp;
 /* The set of signals that are caught.  */
 static sigset_t caught_signals;
 
-/* If nonzero then exit with status WARNING, rather than with the usual
-   signal status, on receipt of a signal with this value.  This
-   suppresses a "Broken Pipe" message with some shells.  */
-static int volatile exiting_signal;
-
 /* If nonnegative, close this file descriptor and unlink remove_ofname
    on error.  */
 static int volatile remove_ofname_fd = -1;
@@ -647,11 +642,6 @@ int main (int argc, char **argv)
     ALLOC(ush, tab_prefix1, 1L<<(BITS-1));
 #endif
 
-#if SIGPIPE
-    exiting_signal = quiet ? SIGPIPE : 0;
-#endif
-    install_signal_handlers ();
-
     /* And get to work */
     if (file_count != 0) {
         if (to_stdout && !test && (!decompress || !ascii)) {
@@ -1088,6 +1078,7 @@ volatile_strcpy (char volatile *dst, char const volatile *src)
 static int
 create_outfile ()
 {
+  static bool signal_handlers_installed;
   int name_shortened = 0;
   int flags = (O_WRONLY | O_CREAT | O_EXCL
                | (ascii && decompress ? 0 : O_BINARY));
@@ -1105,6 +1096,12 @@ create_outfile ()
         }
     }
 
+  if (!signal_handlers_installed)
+    {
+      signal_handlers_installed = true;
+      install_signal_handlers ();
+    }
+
   for (;;)
     {
       int open_errno;
@@ -2103,12 +2100,17 @@ remove_output_file (bool signals_already_blocked)
  * Error handler.
  */
 void
+finish_up_gzip (int exitcode)
+{
+  if (0 <= remove_ofname_fd)
+    remove_output_file (false);
+  do_exit (exitcode);
+}
+void
 abort_gzip ()
 {
-   remove_output_file (false);
-   do_exit(ERROR);
+  finish_up_gzip (ERROR);
 }
-
 /* ========================================================================
  * Signal handler.
  */
@@ -2116,8 +2118,6 @@ static void
 abort_gzip_signal (int sig)
 {
    remove_output_file (true);
-   if (sig == exiting_signal)
-     _exit (WARNING);
    signal (sig, SIG_DFL);
    raise (sig);
 }
diff --git a/gzip.h b/gzip.h
index fad2b3f..b26bc4f 100644
--- a/gzip.h
+++ b/gzip.h
@@ -277,6 +277,7 @@ extern int unpack     (int in, int out);
 extern int unlzh      (int in, int out);
 
         /* in gzip.c */
+_Noreturn extern void finish_up_gzip (int);
 _Noreturn extern void abort_gzip (void);
 
         /* in deflate.c */
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 077b25c..80249b1 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -25,6 +25,7 @@ TESTS =					\
   memcpy-abuse				\
   mixed					\
   null-suffix-clobber			\
+  pipe-output				\
   reproducible				\
   stdin					\
   timestamp				\
diff --git a/tests/pipe-output b/tests/pipe-output
new file mode 100755
index 0000000..5419963
--- /dev/null
+++ b/tests/pipe-output
@@ -0,0 +1,46 @@
+#!/bin/sh
+# Check behavior of output to pipes
+
+# Copyright 2023 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 <https://www.gnu.org/licenses/>.
+# limit so don't run it by default.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ..
+
+sleep 0.01 && sleep_amount=0.01 || sleep_amount=1
+
+echo a >a && echo b >b || framework_failure_
+gzip a && gzip b || fail=1
+
+# Check that gzip etc. behave like cat if the output is a broken pipe.
+for trap_pipe in trap :; do
+  cat_status=$( (($trap_pipe '' PIPE
+                  sleep $sleep_amount
+                  cat <a.gz
+                  echo $? >&3) | : ) 3>&1)
+  test 1 -lt $cat_status && test $cat_status -lt 128 && cat_status=1
+
+  for cmd in 'gunzip' 'gunzip -q' 'gzip -d' 'gzip -dq' \
+             'zcat' 'zcmp - b.gz' 'zdiff - b.gz' 'zgrep a'; do
+    cmd_status=$( (($trap_pipe '' PIPE
+                    sleep $sleep_amount
+                    $cmd <a.gz
+                    echo $? >&3) | : ) 3>&1)
+    test 1 -lt $cmd_status && test $cmd_status -lt 128 && cmd_status=1
+    test $cat_status -eq $cmd_status || fail=1
+  done
+done
+
+Exit $fail
diff --git a/util.c b/util.c
index 642e620..71a937f 100644
--- a/util.c
+++ b/util.c
@@ -36,6 +36,10 @@
 #  define CHAR_BIT 8
 #endif
 
+#ifndef EPIPE
+# define EPIPE 0
+#endif
+
 static int write_buffer (int, voidp, unsigned int);
 
 /* ========================================================================
@@ -452,8 +456,10 @@ void read_error()
 
 void write_error()
 {
+  int exitcode = errno == EPIPE ? WARNING : ERROR;
+  if (! (exitcode == WARNING && quiet))
     fprintf (stderr, "\n%s: %s: %s\n", program_name, ofname, strerror (errno));
-    abort_gzip();
+  finish_up_gzip (exitcode);
 }
 
 /* ========================================================================
diff --git a/zgrep.in b/zgrep.in
index 7aeee6c..504cca1 100644
--- a/zgrep.in
+++ b/zgrep.in
@@ -253,9 +253,9 @@ do
   )
   r=$?
 
-  # Ignore gzip status 2, as it is just a warning.
+  # Ignore gzip status 2 or 141, as it is just a warning or broken pipe.
   # gzip status 1 is an error, like grep status 2.
-  test $gzip_status -eq 2 && gzip_status=0
+  { test $gzip_status -eq 2 || test $gzip_status -eq 141; } && gzip_status=0
   test $gzip_status -eq 1 && gzip_status=2
 
   # Use the more serious of the grep and gzip statuses.