Subject: bug#34488: Add sort --limit, or document workarounds for sort|head error messages

From: Assaf Gordon <[email-masked]>
Date: Fri, 15 Feb 2019 14:40:07 -0700

Helo,

On 2019-02-15 8:20 a.m., Eric Blake wrote:
> On 2/15/19 8:43 AM, 積丹尼 Dan Jacobson wrote:
>> sort: write failed: 'standard output': Broken pipe
>> sort: write error
[...]
> Perhaps coreutils should teach 'env' a command-line option to forcefully
> reset SIGPIPE back to default behavior [...]   If we
> did that, then even if your sh is started with SIGPIPE ignored (so that
> the shell itself can't restore default behavior), you could do this
> theoretical invocation:
> 
> $ seq 9999 | env --default-signal PIPE sort -n | sed 5q | wc -l
> 5

That is a nice idea, I could've used it myself couple of times.

Attached a suggested patch.
If this seems like a good direction, I'll complete it with NEWS/docs/etc.

Usage is:
     env --default-signal=PIPE
     env -P     ##shortcut to reset SIGPIPE
     env --default-signal=PIPE,INT,FOO

This also works nicely with the recent 'env -S' option,
so a script like so can always start with default SIGPIPE handler:

     #!/usr/bin/env -S -P sh
     seq inf | head -n1

comments welcomed,
  - assaf

>From d65ddf38cd5cf60ba6fc4f1bf60f7324a3e6bebd Mon Sep 17 00:00:00 2001
From: Assaf Gordon <[email-masked]>
Date: Fri, 15 Feb 2019 12:31:48 -0700
Subject: [PATCH] env: new option -D/--default-signal=SIG [FIXME]

See https://bugs.gnu.org/34488#8 .
---
 src/env.c                        | 90 +++++++++++++++++++++++++++++++++++++++-
 src/local.mk                     |  1 +
 tests/local.mk                   |  1 +
 tests/misc/env-signal-handler.sh | 68 ++++++++++++++++++++++++++++++
 4 files changed, 159 insertions(+), 1 deletion(-)
 create mode 100755 tests/misc/env-signal-handler.sh

diff --git a/src/env.c b/src/env.c
index 3a1a3869e..ebda91589 100644
--- a/src/env.c
+++ b/src/env.c
@@ -21,12 +21,16 @@
 #include <sys/types.h>
 #include <getopt.h>
 #include <c-ctype.h>
+#include <signal.h>
+#include <string.h>
 
 #include <assert.h>
 #include "system.h"
 #include "die.h"
 #include "error.h"
+#include "operand2sig.h"
 #include "quote.h"
+#include "sig2str.h"
 
 /* The official name of this program (e.g., no 'g' prefix).  */
 #define PROGRAM_NAME "env"
@@ -48,7 +52,15 @@ static bool dev_debug;
 static char *varname;
 static size_t vnlen;
 
-static char const shortopts[] = "+C:iS:u:v0 \t";
+/* if true, at least one signal handler should be reset.  */
+static bool reset_signals ;
+
+/* if element [SIGNUM] is true, the signal handler's should be reset
+   to its defaut. */
+static bool signal_handlers[SIGNUM_BOUND];
+
+
+static char const shortopts[] = "+C:iPS:u:v0 \t";
 
 static struct option const longopts[] =
 {
@@ -56,6 +68,7 @@ static struct option const longopts[] =
   {"null", no_argument, NULL, '0'},
   {"unset", required_argument, NULL, 'u'},
   {"chdir", required_argument, NULL, 'C'},
+  {"default-signal", optional_argument, NULL, 'P'},
   {"debug", no_argument, NULL, 'v'},
   {"split-string", required_argument, NULL, 'S'},
   {GETOPT_HELP_OPTION_DECL},
@@ -88,8 +101,17 @@ Set each NAME to VALUE in the environment and run COMMAND.\n\
   -C, --chdir=DIR      change working directory to DIR\n\
 "), stdout);
       fputs (_("\
+      --default-signal=SIG  reset signal SIG to its default signal handler.\n\
+                            multiple signals can be comma-separated.\n\
+"), stdout);
+      fputs (_("\
   -S, --split-string=S  process and split S into separate arguments;\n\
                         used to pass multiple arguments on shebang lines\n\
+"), stdout);
+      fputs (_("\
+  -P                   same as --default-signal=PIPE\n\
+"), stdout);
+      fputs (_("\
   -v, --debug          print verbose information for each processing step\n\
 "), stdout);
       fputs (HELP_OPTION_DESCRIPTION, stdout);
@@ -525,6 +547,63 @@ parse_split_string (const char* str, int /*out*/ *orig_optind,
   *orig_optind = 0; /* tell getopt to restart from first argument */
 }
 
+static void
+parse_signal_params (const char* optarg)
+{
+  char signame[SIG2STR_MAX];
+  char *opt_sig;
+  char *optarg_writable = xstrdup (optarg);
+
+  opt_sig = strtok (optarg_writable, ",");
+  while (opt_sig)
+    {
+      int signum = operand2sig (opt_sig, signame);
+      if (signum < 0)
+        usage (EXIT_FAILURE);
+
+      signal_handlers[signum] = true;
+
+      opt_sig = strtok (NULL, ",");
+    }
+
+  free (optarg_writable);
+}
+
+static void
+reset_signal_handlers (void)
+{
+
+  if (!reset_signals)
+    return;
+
+  if (dev_debug)
+      devmsg ("Resetting signal handlers:\n");
+
+  for (int i=0; i<SIGNUM_BOUND; ++i)
+    {
+      struct sigaction act;
+
+      if (!signal_handlers[i])
+        continue;
+
+      if (dev_debug)
+        {
+          char signame[SIG2STR_MAX];
+          sig2str (i, signame);
+          devmsg ("   %s (%d)\n", signame, i);
+        }
+
+        if (sigaction (i, NULL, &act))
+          die (EXIT_CANCELED, errno, _("sigaction1(sig=%d) failed"), i);
+
+        act.sa_handler = SIG_DFL;
+        if (sigaction (i, &act, NULL))
+          die (EXIT_CANCELED, errno, _("sigaction2(sig=%d) failed"), i);
+
+
+    }
+}
+
 int
 main (int argc, char **argv)
 {
@@ -558,6 +637,13 @@ main (int argc, char **argv)
         case '0':
           opt_nul_terminate_output = true;
           break;
+        case 'P':
+          reset_signals = true;
+          if (optarg)
+            parse_signal_params (optarg);
+          else
+            signal_handlers[SIGPIPE] = true;
+          break;
         case 'C':
           newdir = optarg;
           break;
@@ -633,6 +719,8 @@ main (int argc, char **argv)
       return EXIT_SUCCESS;
     }
 
+  reset_signal_handlers ();
+
   if (newdir)
     {
       devmsg ("chdir:    %s\n", quoteaf (newdir));
diff --git a/src/local.mk b/src/local.mk
index ba219d530..a69d40521 100644
--- a/src/local.mk
+++ b/src/local.mk
@@ -356,6 +356,7 @@ src_coreutils_SOURCES = src/coreutils.c
 
 src_cp_SOURCES = src/cp.c $(copy_sources) $(selinux_sources)
 src_dir_SOURCES = src/ls.c src/ls-dir.c
+src_env_SOURCES = src/env.c src/operand2sig.c
 src_vdir_SOURCES = src/ls.c src/ls-vdir.c
 src_id_SOURCES = src/id.c src/group-list.c
 src_groups_SOURCES = src/groups.c src/group-list.c
diff --git a/tests/local.mk b/tests/local.mk
index 4751886b2..a699844d2 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -239,6 +239,7 @@ all_tests =					\
   tests/fmt/goal-option.sh			\
   tests/misc/echo.sh				\
   tests/misc/env.sh				\
+  tests/misc/env-signal-handler.sh		\
   tests/misc/ptx.pl				\
   tests/misc/test.pl				\
   tests/misc/seq.pl				\
diff --git a/tests/misc/env-signal-handler.sh b/tests/misc/env-signal-handler.sh
new file mode 100755
index 000000000..ef6313db0
--- /dev/null
+++ b/tests/misc/env-signal-handler.sh
@@ -0,0 +1,68 @@
+#!/bin/sh
+# Test env --default-signal=PIPE feature.
+
+# Copyright (C) 2019 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/>.
+
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ seq
+trap_sigpipe_or_skip_
+
+# Paraphrasing http://bugs.gnu.org/34488#8:
+# POSIX requires that sh started with an inherited ignored SIGPIPE must
+# silently ignore all attempts from within the shell to restore SIGPIPE
+# handling to child processes of the shell:
+#
+#    $ (trap '' PIPE; bash -c 'trap - PIPE; seq inf | head -n1')
+#    1
+#    seq: write error: Broken pipe
+#
+# With 'env --default-signal=PIPE', the signal handler can be reset to its
+# default.
+
+# Baseline Test
+# -------------
+# Ensure this results in a "broken pipe" error (the first 'trap'
+# sets SIGPIPE to ignore, and the second 'trap' becomes a no-op instead
+# of resetting SIGPIPE to its default). Upon a SIGPIPE 'seq' will not be
+# terminated, instead its write(2) call will return an error.
+(trap '' PIPE; sh -c 'trap - PIPE; seq 999999 2>err1t | head -n1 > out1')
+
+# The exact broken pipe message depends on the operating system, just ensure
+# there was a 'write error' message in stderr:
+sed 's/^\(seq: write error:\) .*/\1/' err1t > err1 || framework_failure_
+
+printf "1\n" > exp-out || framework_failure_
+printf "seq: write error:\n" > exp-err1 || framework_failure_
+
+compare exp-out out1 || framework_failure_
+compare exp-err1 err1 || framework_failure_
+
+
+# env test
+# --------
+# With env resetting the signal handler to its defaults, there should be no
+# error message (because the default SIGPIPE action is to terminate the
+# 'seq' program):
+(trap '' PIPE;
+ env --default-signal=PIPE \
+    sh -c 'trap - PIPE; seq 999999 2>err2 | head -n1 > out2')
+
+compare exp-out out2 || fail=1
+compare /dev/null err2 || fail=1
+
+
+
+Exit $fail
-- 
2.11.0