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

[Assaf Gordon]

tags 34488 fixed
close 34488
stop

Hello,

The original request of "sort --limit" resulted in
an improved "env" with options new options,
which was included in the recent version 8.31.

I'm therefor closing this item.

-assaf

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

[Pádraig Brady]

On 03/03/19 16:59, Pádraig Brady wrote:
> A summary of the all signal options in my local set now is:
> 
>   --block-signal[=SIG]block delivery of SIG signal(s) to COMMAND
>   --unblock-signal[=SIG]  unblock delivery of SIG signal(s) to COMMAND
>   --default-signal[=SIG]  reset handling of SIG signal(s) to the default
>   --ignore-signal[=SIG]   set handling of SIG signals(s) to do nothing
>   --list-signal-handling  list non default signal handling to stderr

I think we might be also able to remove --unblock-signal,
and just have --default-signal.

I.E. I don't think one would want to set default signal handling,
but leave a signal blocked?  Also I don't think one would
want to unblock a signal, but leave it ignored?
At least not and the level of env just exec()ing a program.
Anyway if those weird cases are needed, they can be achieved
by combining the options.

The attached does that, leaving us with:

--block-signal[=SIG]block delivery of SIG signal(s) to COMMAND
--default-signal[=SIG]  reset handling of SIG signal(s) to the default
--ignore-signal[=SIG]   set handling of SIG signals(s) to do nothing
--list-signal-handling  list non default signal handling to stderr

cheers,
Pádraig.
From 7183477b7a5d9531a43b48beee8906ea7ae6fc06 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= 
Date: Sun, 3 Mar 2019 17:40:34 -0800
Subject: [PATCH] env: have --default-signal imply --unblock-signal

Remove --unblock as i's not needed in isolation
---
 doc/coreutils.texi | 12 +---
 src/env.c  |  9 +
 2 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 0e4ed16..8112034 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -17247,10 +17247,11 @@ env --chdir=/build FOO=bar timeout 5 true
 @end example
 
 @item --default-signal[=@var{sig}]
-Reset signal @var{sig} to its default signal handler. Without @var{sig} all
-known signals are reset to their defaults. Multiple signals can be
-comma-separated. The following command runs @command{seq} with SIGINT and
-SIGPIPE set to their default (which is to terminate the program):
+Unblock and reset signal @var{sig} to its default signal handler.
+Without @var{sig} all known signals are reset to their defaults.
+Multiple signals can be comma-separated. The following command runs
+@command{seq} with SIGINT and SIGPIPE set to their default
+(which is to terminate the program):
 
 @example
 env --default-signal=PIPE,INT seq 1000 | head -n1
@@ -17302,9 +17303,6 @@ env --default-signal=INT,PIPE --ignore-signal=INT
 @item --block-signal[=@var{sig}]
 Block signal(s) @var{sig} from being delivered.
 
-@item --unblock-signal[=@var{sig}]
-Unblock signal(s) @var{sig}.
-
 @item --list-blocked-signals
 List blocked (masked) signals and exit.
 If no signals are blocked, nothing is printed.
diff --git a/src/env.c b/src/env.c
index 9a910da..aa0cfbc 100644
--- a/src/env.c
+++ b/src/env.c
@@ -82,7 +82,6 @@ enum
   DEFAULT_SIGNAL_OPTION = CHAR_MAX + 1,
   IGNORE_SIGNAL_OPTION,
   BLOCK_SIGNAL_OPTION,
-  UNBLOCK_SIGNAL_OPTION,
   LIST_SIGNAL_HANDLING_OPTION,
 };
 
@@ -95,7 +94,6 @@ static struct option const longopts[] =
   {"default-signal", optional_argument, NULL, DEFAULT_SIGNAL_OPTION},
   {"ignore-signal",  optional_argument, NULL, IGNORE_SIGNAL_OPTION},
   {"block-signal",   optional_argument, NULL, BLOCK_SIGNAL_OPTION},
-  {"unblock-signal", optional_argument, NULL, UNBLOCK_SIGNAL_OPTION},
   {"list-signal-handling", no_argument, NULL,  LIST_SIGNAL_HANDLING_OPTION},
   {"debug", no_argument, NULL, 'v'},
   {"split-string", required_argument, NULL, 'S'},
@@ -136,9 +134,6 @@ Set each NAME to VALUE in the environment and run COMMAND.\n\
   --block-signal[=SIG]block delivery of SIG signal(s) to COMMAND\n\
 "), stdout);
   fputs (_("\
-  --unblock-signal[=SIG]  unblock delivery of SIG signal(s) to COMMAND\n\
-"), stdout);
-  fputs (_("\
   --default-signal[=SIG]  reset handling of SIG signal(s) to the default\n\
 "), stdout);
   fputs (_("\
@@ -823,6 +818,7 @@ main (int argc, char **argv)
   break;
 case DEFAULT_SIGNAL_OPTION:
   parse_signal_action_params (optarg, true);
+  parse_block_signal_params (optarg, false);
   break;
 case IGNORE_SIGNAL_OPTION:
   parse_signal_action_params (optarg, false);
@@ -830,9 +826,6 @@ main (int argc, char **argv)
 case BLOCK_SIGNAL_OPTION:
   parse_block_signal_params (optarg, true);
   break;
-case UNBLOCK_SIGNAL_OPTION:
-  parse_block_signal_params (optarg, false);
-  break;
 case LIST_SIGNAL_HANDLING_OPTION:
   report_signal_handling = true;
   break;
-- 
2.9.3

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

[Pádraig Brady]

On 03/03/19 16:07, Pádraig Brady wrote:
> On 03/03/19 15:36, Pádraig Brady wrote:
>> On 25/02/19 22:35, Pádraig Brady wrote:
>>> Thanks for doing all that.
>>> I've attached a few changes:
>>>
>>> - spelling fixes
>>> - usage() clarified/reordered
>>> - ensure sigset_t are initialized
>>> - Don't setprocmask() unless specified
>>> - Simplified SETMASK_SIGNAL_OPTION handling
>>> - The test missed `env` as a prerequisite
>>> - The test was slow/spun cpu, so used sleep instead of seq
>>> - Used $SHELL in case sh didn't support trap
>>>
>>> I see that the last signal that `kill -l` lists is not included.
>>> I think we should be processing SIGNUM_BOUND also?
>>
>> An additional patch attached to replace --list-signal-actions
>> with --list-ignored-signals.  This is simpler, and more symmetric
>> with the other options. Also the extra output was moot I think
>> since handlers are reset upon exec
> 
> I've locally adjusted the NEWS/texi for that patch also.
> I've a local patch to include SIGNUM_BOUND which I won't spam the list with.
> 
> Attached is a patch to remove the -p and --set-mask options,
> which I've a light preference for.
> I'll think some more about it.

Attached is a patch to combine the two --list options
to a single --list-signal-handling option.
This also doesn't exit, and so is more useful as
users can add --list-signal to their env commands
to diagnose signal handling.  One can easily use `true`
as the command to get the original list only functionality.

A summary of the all signal options in my local set now is:

  --block-signal[=SIG]block delivery of SIG signal(s) to COMMAND
  --unblock-signal[=SIG]  unblock delivery of SIG signal(s) to COMMAND
  --default-signal[=SIG]  reset handling of SIG signal(s) to the default
  --ignore-signal[=SIG]   set handling of SIG signals(s) to do nothing
  --list-signal-handling  list non default signal handling to stderr

cheers,
Pádraig
From 715254df092d6e3dfac7bdfd72f7307dce21dabd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= 
Date: Sun, 3 Mar 2019 16:50:27 -0800
Subject: [PATCH] env: add only a single --list-signal-handling option

This also doesn't exit, which is more useful as
it directly confirms any inherited handling
in combination with any signal settings we may have set.
One can easily use `true` as the command to get
the original list only functionality
---
 NEWS   |  3 ++-
 doc/coreutils.texi |  5 ++---
 src/env.c  | 54 +++---
 3 files changed, 23 insertions(+), 39 deletions(-)

diff --git a/NEWS b/NEWS
index 33f1ef2..6fbc8f1 100644
--- a/NEWS
+++ b/NEWS
@@ -90,7 +90,8 @@ GNU coreutils NEWS-*- outline -*-
   env now supports '--{block,unblock,setmask}-signal[=SIG]' to block/unblock
   signal delivery before executing a program.
 
-  env now supports '--list-blocked-signals', and '--list-ignored-signals'.
+  env now supports '--list-signal-handling' to identify signal handling
+  before executing a program.
 
 ** New commands
 
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 3c220ea..0e4ed16 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -17309,9 +17309,8 @@ Unblock signal(s) @var{sig}.
 List blocked (masked) signals and exit.
 If no signals are blocked, nothing is printed.
 
-@item --list-ignored-signals
-List ignored signals and exit.
-If no signals are ignored, nothing is printed.
+@item --list-signal-handling
+List blocked or ignored signals to stderr, before executing a command.
 
 @item -v
 @itemx --debug
diff --git a/src/env.c b/src/env.c
index 7bc3c7b..9a910da 100644
--- a/src/env.c
+++ b/src/env.c
@@ -70,6 +70,9 @@ static sigset_t unblock_signals;
 /* Whether signal mask adjustment requested.  */
 static bool sig_mask_changed;
 
+/* Whether to list non default handling.  */
+static bool report_signal_handling;
+
 static char const shortopts[] = "+C:ipS:u:v0 \t";
 
 /* For long options that have no equivalent short option, use a
@@ -80,8 +83,7 @@ enum
   IGNORE_SIGNAL_OPTION,
   BLOCK_SIGNAL_OPTION,
   UNBLOCK_SIGNAL_OPTION,
-  LIST_BLOCKED_SIGNALS_OPTION,
-  LIST_IGNORED_SIGNALS_OPTION,
+  LIST_SIGNAL_HANDLING_OPTION,
 };
 
 static struct option const longopts[] =
@@ -94,8 +96,7 @@ static struct option const longopts[] =
   {"ignore-signal",  optional_argument, NULL, IGNORE_SIGNAL_OPTION},
   {"block-signal",   optional_argument, NULL, BLOCK_SIGNAL_OPTION},
   {"unblock-signal", optional_argument, NULL, UNBLOCK_SIGNAL_OPTION},
-  {"list-blocked-signals", no_argument, NULL, LIST_BLOCKED_SIGNALS_OPTION},
-  {"list-ignored-signals", no_argument, NULL,  LIST_IGNORED_SIGNALS_OPTION},
+  {"list-signal-handling", no_argument, NULL,  LIST_SIGNAL_HANDLING_OPTION},
   {"debug", no_argument, NULL, 'v'},
   {"split-string", required_argument, NULL, 'S'},
   {GETOPT_HELP_OPTION_DECL},
@@ -144,10 +145,7 @@ Set each NAME to VALUE in the environment and run COMMAND.\n\
   --ignore-signal[=SIG]   set 

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

[Pádraig Brady]

On 03/03/19 15:36, Pádraig Brady wrote:
> On 25/02/19 22:35, Pádraig Brady wrote:
>> Thanks for doing all that.
>> I've attached a few changes:
>>
>> - spelling fixes
>> - usage() clarified/reordered
>> - ensure sigset_t are initialized
>> - Don't setprocmask() unless specified
>> - Simplified SETMASK_SIGNAL_OPTION handling
>> - The test missed `env` as a prerequisite
>> - The test was slow/spun cpu, so used sleep instead of seq
>> - Used $SHELL in case sh didn't support trap
>>
>> I see that the last signal that `kill -l` lists is not included.
>> I think we should be processing SIGNUM_BOUND also?
> 
> An additional patch attached to replace --list-signal-actions
> with --list-ignored-signals.  This is simpler, and more symmetric
> with the other options. Also the extra output was moot I think
> since handlers are reset upon exec

I've locally adjusted the NEWS/texi for that patch also.
I've a local patch to include SIGNUM_BOUND which I won't spam the list with.

Attached is a patch to remove the -p and --set-mask options,
which I've a light preference for.
I'll think some more about it.

cheers,
Pádraig

>From ae8d2ab5d9a380051a1ae1d315832e98f40ff456 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= 
Date: Sun, 3 Mar 2019 16:02:55 -0800
Subject: [PATCH] env: remove shortcut signal handling options

* src/env.c: Remove -p and --setmask-signal
---
 doc/coreutils.texi   |  9 -
 src/env.c| 16 
 tests/misc/env-signal-handler.sh | 10 --
 3 files changed, 35 deletions(-)

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 4dea101..3c220ea 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -17305,10 +17305,6 @@ Block signal(s) @var{sig} from being delivered.
 @item --unblock-signal[=@var{sig}]
 Unblock signal(s) @var{sig}.
 
-@item --setmask-signal[=@var{sig}]
-Set list of blocked signals to @var{sig}. All other signals will be unblocked.
-
-
 @item --list-blocked-signals
 List blocked (masked) signals and exit.
 If no signals are blocked, nothing is printed.
@@ -17317,11 +17313,6 @@ If no signals are blocked, nothing is printed.
 List ignored signals and exit.
 If no signals are ignored, nothing is printed.
 
-
-@item -p
-Equivalent to @option{--default-signal=PIPE} - sets SIGPIPE to its default
-behavior (terminate a program upon @samp{SIGPIPE}).
-
 @item -v
 @itemx --debug
 @opindex -v
diff --git a/src/env.c b/src/env.c
index 90821c3..7bc3c7b 100644
--- a/src/env.c
+++ b/src/env.c
@@ -80,7 +80,6 @@ enum
   IGNORE_SIGNAL_OPTION,
   BLOCK_SIGNAL_OPTION,
   UNBLOCK_SIGNAL_OPTION,
-  SETMASK_SIGNAL_OPTION,
   LIST_BLOCKED_SIGNALS_OPTION,
   LIST_IGNORED_SIGNALS_OPTION,
 };
@@ -95,7 +94,6 @@ static struct option const longopts[] =
   {"ignore-signal",  optional_argument, NULL, IGNORE_SIGNAL_OPTION},
   {"block-signal",   optional_argument, NULL, BLOCK_SIGNAL_OPTION},
   {"unblock-signal", optional_argument, NULL, UNBLOCK_SIGNAL_OPTION},
-  {"setmask-signal", optional_argument, NULL, SETMASK_SIGNAL_OPTION},
   {"list-blocked-signals", no_argument, NULL, LIST_BLOCKED_SIGNALS_OPTION},
   {"list-ignored-signals", no_argument, NULL,  LIST_IGNORED_SIGNALS_OPTION},
   {"debug", no_argument, NULL, 'v'},
@@ -140,9 +138,6 @@ Set each NAME to VALUE in the environment and run COMMAND.\n\
   --unblock-signal[=SIG]  unblock delivery of SIG signal(s) to COMMAND\n\
 "), stdout);
   fputs (_("\
-  --setmask-signal[=SIG]  set full mask with SIG signal(s) blocked\n\
-"), stdout);
-  fputs (_("\
   --default-signal[=SIG]  reset handling of SIG signal(s) to the default\n\
 "), stdout);
   fputs (_("\
@@ -155,9 +150,6 @@ Set each NAME to VALUE in the environment and run COMMAND.\n\
   --list-blocked-signals  list blocked (masked) signals and exit\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);
@@ -845,10 +837,6 @@ main (int argc, char **argv)
 case '0':
   opt_nul_terminate_output = true;
   break;
-case 'p':
-  /* alias to --default-signal=PIPE */
-  signals[SIGPIPE] = DEFAULT;
-  break;
 case DEFAULT_SIGNAL_OPTION:
   parse_signal_action_params (optarg, true);
   break;
@@ -861,10 +849,6 @@ main (int argc, char **argv)
 case UNBLOCK_SIGNAL_OPTION:
   parse_block_signal_params (optarg, false);
   break;
-case SETMASK_SIGNAL_OPTION:
-  parse_block_signal_params (NULL, false);
-  parse_block_signal_params (optarg, true);
-  break;
 case LIST_BLOCKED_SIGNALS_OPTION:
   list_blocked_signals ();
   break;
diff --git a/tests/misc/env-signal-handler.sh b/tests/misc/env-signal-handler.sh
index 4dd90c2..5e6b57e 100755
--- 

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

[Pádraig Brady]

On 25/02/19 22:35, Pádraig Brady wrote:
> Thanks for doing all that.
> I've attached a few changes:
> 
> - spelling fixes
> - usage() clarified/reordered
> - ensure sigset_t are initialized
> - Don't setprocmask() unless specified
> - Simplified SETMASK_SIGNAL_OPTION handling
> - The test missed `env` as a prerequisite
> - The test was slow/spun cpu, so used sleep instead of seq
> - Used $SHELL in case sh didn't support trap
> 
> I see that the last signal that `kill -l` lists is not included.
> I think we should be processing SIGNUM_BOUND also?

An additional patch attached to replace --list-signal-actions
with --list-ignored-signals.  This is simpler, and more symmetric
with the other options. Also the extra output was moot I think
since handlers are reset upon exec. For completeness the transitions are:

upon fork
  default -> default
  handled -> handled
  ignored -> ignored
  pending -> cleared
  blocked -> blocked

upon exec
  default -> default
  handled -> default
  ignored -> ignored
  pending -> pending
  blocked -> blocked

shell has additional rules:
  can't unignore
  handled -> default in subshell

I'll squash this into yours, before commit.

cheers,
Pádraig
>From a8e531b15a2e85ecf7d18526bfbb7990537621b3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= 
Date: Sun, 3 Mar 2019 15:29:59 -0800
Subject: [PATCH] env: use --list-ignored-signals instead of
 --list-signals-action

This is more symmetric and anyway signal handlers are
reset upon exec(), so the more complicated reporting is moot.
---
 src/env.c | 60 ++--
 1 file changed, 26 insertions(+), 34 deletions(-)

diff --git a/src/env.c b/src/env.c
index 1acfc11..74043b1 100644
--- a/src/env.c
+++ b/src/env.c
@@ -81,8 +81,8 @@ enum
   BLOCK_SIGNAL_OPTION,
   UNBLOCK_SIGNAL_OPTION,
   SETMASK_SIGNAL_OPTION,
-  LIST_SIGNAL_ACTIONS_OPTION,
   LIST_BLOCKED_SIGNALS_OPTION,
+  LIST_IGNORED_SIGNALS_OPTION,
 };
 
 static struct option const longopts[] =
@@ -93,11 +93,11 @@ static struct option const longopts[] =
   {"chdir", required_argument, NULL, 'C'},
   {"default-signal", optional_argument, NULL, DEFAULT_SIGNAL_OPTION},
   {"ignore-signal",  optional_argument, NULL, IGNORE_SIGNAL_OPTION},
-  {"list-signal-actions", no_argument, NULL,  LIST_SIGNAL_ACTIONS_OPTION},
   {"block-signal",   optional_argument, NULL, BLOCK_SIGNAL_OPTION},
   {"unblock-signal", optional_argument, NULL, UNBLOCK_SIGNAL_OPTION},
   {"setmask-signal", optional_argument, NULL, SETMASK_SIGNAL_OPTION},
   {"list-blocked-signals", no_argument, NULL, LIST_BLOCKED_SIGNALS_OPTION},
+  {"list-ignored-signals", no_argument, NULL,  LIST_IGNORED_SIGNALS_OPTION},
   {"debug", no_argument, NULL, 'v'},
   {"split-string", required_argument, NULL, 'S'},
   {GETOPT_HELP_OPTION_DECL},
@@ -149,7 +149,7 @@ Set each NAME to VALUE in the environment and run COMMAND.\n\
   --ignore-signal[=SIG]   set handling of SIG signals(s) to do nothing\n\
 "), stdout);
   fputs (_("\
-  --list-signal-actions   list signals with non-default actions and exit\n\
+  --list-ignored-signals  list ignored signals and exit\n\
 "), stdout);
   fputs (_("\
   --list-blocked-signals  list blocked (masked) signals and exit\n\
@@ -680,34 +680,6 @@ reset_signal_handlers (void)
 }
 }
 
-static void
-list_signal_actions (void)
-{
-  char signame[SIG2STR_MAX];
-  const char* action;
-
-  for (int i = 1; i < SIGNUM_BOUND; i++)
-{
-  struct sigaction act;
-  if (sigaction (i, NULL, ))
-continue;
-
-  if (act.sa_flags & SA_SIGINFO)
-action = "sig-action";
-  else if (act.sa_handler == SIG_DFL)
-continue; /* no need to print defaults.  */
-  else if (act.sa_handler == SIG_IGN)
-action = "ignore";
-  else
-action = "sig-handler";
-
-  sig2str (i, signame);
-  printf ("%-10s (%2d): %s\n", signame, i, action);
-}
-
-  exit (EXIT_SUCCESS);
-}
-
 
 static void
 parse_block_signal_params (const char* optarg, bool block)
@@ -819,6 +791,26 @@ list_blocked_signals (void)
   exit (EXIT_SUCCESS);
 }
 
+static void
+list_ignored_signals (void)
+{
+  char signame[SIG2STR_MAX];
+
+  for (int i = 1; i < SIGNUM_BOUND; i++)
+{
+  struct sigaction act;
+  if (sigaction (i, NULL, ))
+continue;
+
+  if (act.sa_handler != SIG_IGN)
+continue;
+
+  sig2str (i, signame);
+  printf ("%-10s (%2d)\n", signame, i);
+}
+
+  exit (EXIT_SUCCESS);
+}
 
 int
 main (int argc, char **argv)
@@ -863,9 +855,6 @@ main (int argc, char **argv)
 case IGNORE_SIGNAL_OPTION:
   parse_signal_action_params (optarg, false);
   break;
-case LIST_SIGNAL_ACTIONS_OPTION:
-  list_signal_actions ();
-  break;
 case BLOCK_SIGNAL_OPTION:
   parse_block_signal_params (optarg, true);
   break;
@@ -879,6 +868,9 @@ main (int argc, char **argv)
 case LIST_BLOCKED_SIGNALS_OPTION:
   

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

[Pádraig Brady]

Thanks for doing all that.
I've attached a few changes:

- spelling fixes
- usage() clarified/reordered
- ensure sigset_t are initialized
- Don't setprocmask() unless specified
- Simplified SETMASK_SIGNAL_OPTION handling
- The test missed `env` as a prerequisite
- The test was slow/spun cpu, so used sleep instead of seq
- Used $SHELL in case sh didn't support trap

I see that the last signal that `kill -l` lists is not included.
I think we should be processing SIGNUM_BOUND also?

cheers,
Pádraig
>From f54e67f2a9dcc4db287c31969e99899582f53a88 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= 
Date: Mon, 25 Feb 2019 22:30:09 -0800
Subject: [PATCH] env: misc signal handling fixes

spelling fixes
usage() clarified/reordered
ensure sigset_t are initialized
Don't setprocmask() unless specified
Simplified SETMASK_SIGNAL_OPTION handling
test missed `env` as a prerequisite
test was slow/spun cpu, so used sleep instead of seq
Used $SHELL in case sh didn't support trap
---
 NEWS |  4 ++--
 doc/coreutils.texi   |  6 +++---
 man/env.x|  4 ++--
 src/env.c| 46 +++-
 tests/misc/env-signal-handler.sh | 33 
 5 files changed, 47 insertions(+), 46 deletions(-)

diff --git a/NEWS b/NEWS
index c310d1f..e090c72 100644
--- a/NEWS
+++ b/NEWS
@@ -67,10 +67,10 @@ GNU coreutils NEWS-*- outline -*-
   test now supports the '-N FILE' unary operator (like e.g. bash) to check
   whether FILE exists and has been modified since it was last read.
 
-  env now supports '--default-singal[=SIG]' and '--ignore-signal[=SIG]'
+  env now supports '--default-signal[=SIG]' and '--ignore-signal[=SIG]'
   options to set signal handlers before executing a program.
 
-  env now supports '--{block,unblock,setmask}-singal[=SIG]' to block/unblock
+  env now supports '--{block,unblock,setmask}-signal[=SIG]' to block/unblock
   signal delivery before executing a program.
 
   env now supports '--list-signal-actions' and '--list-blocked-signals'
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 5199b83..30a5990 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -17274,7 +17274,7 @@ Most operating systems do not allow ignoring @samp{SIGKILL}, @samp{SIGSTOP}
 
 Multiple (and contradictory) @option{--default-signal=SIG} and
 @option{--ignore-signal=SIG} options are processed left-to-right,
-with the latter taking precedence. In the follwing example, @samp{SIGPIPE} is
+with the latter taking precedence. In the following example, @samp{SIGPIPE} is
 set to default while @samp{SIGINT} is ignored:
 
 @example
@@ -17282,10 +17282,10 @@ env --default-signal=INT,PIPE --ignore-signal=INT
 @end example
 
 @item --block-signal[=@var{sig}]
-Block signal @var{sig} from being delivered.
+Block signal(s) @var{sig} from being delivered.
 
 @item --unblock-signal[=@var{sig}]
-Unblock signal @var{sig}.
+Unblock signal(s) @var{sig}.
 
 @item --setmask-signal[=@var{sig}]
 Set list of blocked signals to @var{sig}. All other signals will be unblocked.
diff --git a/man/env.x b/man/env.x
index b787fe3..b2eb371 100644
--- a/man/env.x
+++ b/man/env.x
@@ -38,7 +38,7 @@ parameter the script will likely fail with:
 .PP
 See the full documentation for more details.
 .PP
-.SS "\-\-default-signal[=SIG]" to 'untrap' a singal
+.SS "\-\-default-signal[=SIG]" to 'untrap' a signal
 This option allows setting a signal handler to its default
 action. This is useful to reset a signal after setting it
 to 'ignore' using the shell's trap command.
@@ -87,7 +87,7 @@ Multiple (and contradictory)
 and
 .B \-\-ignore\-signal=SIG
 options are processed left-to-right, with the latter taking precedence.
-In the follwing example, SIGPIPE is set to default while SIGINT is ignored:
+In the following example, SIGPIPE is set to default while SIGINT is ignored:
 .RS
 .nf
 env \-\-default\-signal=INT,PIPE \-\-ignore\-signal=INT
diff --git a/src/env.c b/src/env.c
index 4385620..1acfc11 100644
--- a/src/env.c
+++ b/src/env.c
@@ -67,6 +67,8 @@ static sigset_t block_signals;
 /* Set of signals to unblock.  */
 static sigset_t unblock_signals;
 
+/* Whether signal mask adjustment requested.  */
+static bool sig_mask_changed;
 
 static char const shortopts[] = "+C:ipS:u:v0 \t";
 
@@ -125,35 +127,32 @@ Set each NAME to VALUE in the environment and run COMMAND.\n\
   -u, --unset=NAME remove variable from the environment\n\
 "), stdout);
   fputs (_("\
-  --block-signal[=SIG]block signal SIG.\n\
+  -C, --chdir=DIR  change working directory to DIR\n\
 "), stdout);
   fputs (_("\
-  --unblock-signal[=SIG]  unblock signal SIG.\n\
+  -S, --split-string=S  process and split S into separate arguments;\n\
+used to pass multiple arguments on shebang lines\n\
 "), stdout);
   fputs (_("\
-  --setmask-signal[=SIG]  set blocked signal(s) mask to SIG.\n\
+  

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

[Assaf Gordon]

Hello,

Thanks for all comments.

On 2019-02-24 11:33 a.m., Paul Eggert wrote:
Thanks for doing all that. Although Pádraig is not enthusiastic about a 
shortcut like -p, I'm a bit warmer to it, as it's an important special 
case to fix a wart in POSIX. No big deal either way.


For now I kept "-p", can be removed later of course.
The first patch includes Pádraig's recent suggestions (slightly modified).


The documentation should mention that SIGCHLD is special [...]


The documentation should say what happens if mutually-contradictory 
options are specified, [...]


The documentation should echo this suggestion in 
: 


I've added those, and I welcome all improvements suggestion to 
grammar/phrasing/etc.


> There should be options --block-signal[=SIG], --unblock-signal[=SIG],
> and --setmask-signal[=SIG] that affect the signal mask, which is also
> inherited by the child. These can be implemented via pthread_sigmask.

The second patch adds these new options (separated to ease review).
As for documentation - I'm not sure what to add beyond the basic
option description. When should these be used?

A third small patch adds "env ---list-signal-actions" and
"env --list-blocked-signals" - to ease diagnostics.
Might be worth adding for completeness (e.g., for users who
need to somehow know if SIGPIPE is being ignored by the shell
or not):

$ ( trap '' PIPE && src/env --list-signal-actions )
PIPE   (13): ignore

Comments very welcomed,
 - assaf



>From 02cba657e2f63c05f859daf18a7d1032fdc32c6f Mon Sep 17 00:00:00 2001
From: Assaf Gordon 
Date: Fri, 15 Feb 2019 12:31:48 -0700
Subject: [PATCH 1/3] env: new options
 -p/--default-signal=SIG/--ignore-signal=SIG

New options to set signal handlers to default (SIG_DFL) or ignore
(SIG_IGN) This is useful to overcome POSIX limitation that shell must
not override inherited signal state, e.g. the second 'trap' here is
a no-op:

   trap '' PIPE && sh -c 'trap - PIPE ; seq inf | head -n1'

Instead use:

   trap '' PIPE && sh -c 'env -p seq inf | head -n1'

Similarly, the following will prevent CTRL-C from terminating the
program:

   env --ignore-signal=INT seq inf > /dev/null

See https://bugs.gnu.org/34488#8 .

* NEWS: Mention new options.
* doc/coreutils.texi (env invocation): Document new options.
* man/env.x: Add example of --default-signal=SIG usage.
* src/env.c (signals): New global variable.
(shortopts,longopts): Add new options.
(usage): Print new options.
(parse_signal_params): Parse comma-separated list of signals, store in
signals variable.
(reset_signal_handlers): Set each signal to SIG_DFL/SIG_IGN.
(main): Process new options.
* src/local.mk (src_env_SOURCES): Add operand2sig.c.
* tests/misc/env-signal-handler.sh: New test.
* tests/local.mk (all_tests): Add new test.
---
 NEWS |   3 +
 doc/coreutils.texi   |  58 
 man/env.x|  69 ++
 src/env.c| 138 +++-
 src/local.mk |   1 +
 tests/local.mk   |   1 +
 tests/misc/env-signal-handler.sh | 146 +++
 7 files changed, 415 insertions(+), 1 deletion(-)
 create mode 100755 tests/misc/env-signal-handler.sh

diff --git a/NEWS b/NEWS
index e73cb52b8..ddbbaf138 100644
--- a/NEWS
+++ b/NEWS
@@ -81,6 +81,9 @@ GNU coreutils NEWS-*- outline -*-
   test now supports the '-N FILE' unary operator (like e.g. bash) to check
   whether FILE exists and has been modified since it was last read.
 
+  env now supports '--default-singal[=SIG]' and '--ignore-signal[=SIG]'
+  options to set signal handlers before executing a program.
+
 ** New commands
 
   basenc is added to complement existing base64,base32 commands,
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index eb1848882..c2c202b28 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -17246,6 +17246,64 @@ chroot /chroot env --chdir=/srv true
 env --chdir=/build FOO=bar timeout 5 true
 @end example
 
+@item --default-signal[=@var{sig}]
+Reset signal @var{sig} to its default signal handler. Without @var{sig} all
+known signals are reset to their defaults. Multiple signals can be
+comma-separated. The following command runs @command{seq} with SIGINT and
+SIGPIPE set to their default (which is to terminate the program):
+
+@example
+env --default-signal=PIPE,INT seq 1000 | head -n1
+@end example
+
+In the following example:
+
+@example
+trap '' PIPE && sh -c 'trap - PIPE ; seq inf | head -n1'
+@end example
+
+The first trap command sets SIGPIPE to ignore.  The second trap command
+ostensibly sets it back to its  default, but POSIX mandates that the shell
+must not change inherited state of the signal - so it is a no-op.
+
+Using @option{--default-signal=PIPE} (or its shortcut @option{-p}) can be
+used to force the signal to  its  default 

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

[Eric Blake]

On 2/23/19 11:32 PM, Pádraig Brady wrote:

 You HAVE to use some other intermediate program if you want to override
 an inherited ignored SIGPIPE in sh into an inherited default-behavior
 SIGPIPE in sort.
>>>
>>> Should we also propose to POSIX to allow trap to specify default?
>>
>> That's what "trap - PIPE" is already supposed to do, except that POSIX
>> has the odd requirement that a signal that was inherited ignored cannot
>> be reset to default.
>>
>>> Maybe `trap 0 PIPE` or similar?
>>
>> Alas, bash has already defined that to mean the same as 'trap - EXIT PIPE'.
> 
> Fair enough, but do we agree that it would be good
> to have functionality in the shell with some similar syntax
> that resets the handler to system default?

Worth asking on the bash list to see if Chet has any interest in such an
extension (POSIX is reluctant to specify something that doesn't have
existing implementation practice).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

[積丹尼 Dan Jacobson]

Thanks for working on this guys, hopefully users will one day no longer
be faced with (yes, seemingly) random errors.
$ ls -l | sed 2!d
drwxr-xr-x 14 jidanni jidanni  4096 2016-12-24  AndroidMisc
$ ls -l | sed 2q
total 157780
drwxr-xr-x 14 jidanni jidanni  4096 2016-12-24  AndroidMisc
ls: write error: Broken pipe

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

[Paul Eggert]

Thanks for doing all that. Although Pádraig is not enthusiastic about a shortcut 
like -p, I'm a bit warmer to it, as it's an important special case to fix a wart 
in POSIX. No big deal either way.


The documentation should mention that SIGCHLD is special, in that 
--ignore-signal=CHLD might have no effect (POSIX says it's unspecified).


The documentation should say what happens if mutually-contradictory options are 
specified, e.g., '--default-signal=INT --ignore-signal=INT'. Presumably they are 
processed left-to-right, as is traditional for 'env' with contradictory operands.


There should be options --block-signal[=SIG], --unblock-signal[=SIG], and 
--setmask-signal[=SIG] that affect the signal mask, which is also inherited by 
the child. These can be implemented via pthread_sigmask.


The documentation should echo this suggestion in 
: "many 
existing applications wrongly assume that they start with certain signals set to 
the default action and/or unblocked Therefore, it is best not to block or 
ignore signals across execs without explicit reason to do so, and especially not 
to block signals across execs of arbitrary (not closely cooperating) programs."

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

[Pádraig Brady]

On 18/02/19 01:46, Assaf Gordon wrote:
> Hello,
> 
> Thanks for all comments (on and off list).
> Attached an updated patch with documentation.
> 
> The supported options are:
> 
>   --default-signal[=SIG]  reset signal SIG to its default signal handler.
>   without SIG, all known signals are included.
>   multiple signals can be comma-separated.
>   --ignore-signal[=SIG]   set signal SIG to be IGNORED.
>   without SIG, all known signals are included.
>   multiple signals can be comma-separated.
>   -p  same as --default-signal=PIPE
> 
> (lower-case "-p" as to not conflict with BSD, but of course can be
> changed to another letter).
> 
> The new 'env-signal-handler.sh' test passes on GNU/linux, non-gnu/linux
> (alpine), and Free/Open/Net BSD.
> 
> Comments very welcomed,

Attached is a patch to be squashed in to:

  Process the same signals as kill -l
  Output whether action failed or not
  Don't run sigaction(..set..) if get failed

I'm not that enthusiastic about the -p
since it treats PIPE specially here,
and it's not special from env's point of view.

cheers,
Pádraig
>From 9d873110e29a6f6fb5c4932164e8ce5ad6ad0668 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= 
Date: Sat, 23 Feb 2019 22:11:08 -0800
Subject: [PATCH] env: --default fixes

Process the same signals as kill -l
Output whether action failed or not
Don't run sigaction(..set..) if get failed
---
 src/env.c | 48 +++-
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/src/env.c b/src/env.c
index 6c20ac4..a601538 100644
--- a/src/env.c
+++ b/src/env.c
@@ -572,13 +572,13 @@ parse_signal_params (const char* optarg, bool set_default)
   char *opt_sig;
   char *optarg_writable;
 
-  if (!optarg)
+  if (! optarg)
 {
   /* without an argument, reset all signals.
  Some signals cannot be set to ignore or default (e.g., SIGKILL,
  SIGSTOP on most OSes, and SIGCONT on AIX.) - so ignore errors.  */
   for (int i = 0 ; i < SIGNUM_BOUND; ++i)
-if (sig2str (i, signame) == 0)
+if (sig2str (i + 1, signame) == 0)
   signals[i] = set_default ? DEFAULT_NOERR : IGNORE_NOERR;
   return;
 }
@@ -603,38 +603,44 @@ parse_signal_params (const char* optarg, bool set_default)
 static void
 reset_signal_handlers (void)
 {
-  for (int i=0; i

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

[Pádraig Brady]

On 18/02/19 08:02, Eric Blake wrote:
> On 2/17/19 8:20 PM, Pádraig Brady wrote:
>> On 15/02/19 07:20, Eric Blake wrote:
>>> Except that POSIX has the nasty requirement 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  | sort -n | sed 5q | wc -l')
>>> 5
>>> sort: write failed: 'standard output': Broken pipe
>>> sort: write error
>>
>>> You HAVE to use some other intermediate program if you want to override
>>> an inherited ignored SIGPIPE in sh into an inherited default-behavior
>>> SIGPIPE in sort.
>>
>> Should we also propose to POSIX to allow trap to specify default?
> 
> That's what "trap - PIPE" is already supposed to do, except that POSIX
> has the odd requirement that a signal that was inherited ignored cannot
> be reset to default.
> 
>> Maybe `trap 0 PIPE` or similar?
> 
> Alas, bash has already defined that to mean the same as 'trap - EXIT PIPE'.

Fair enough, but do we agree that it would be good
to have functionality in the shell with some similar syntax
that resets the handler to system default?

cheers,
Pádraig

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

[Eric Blake]

On 2/17/19 8:20 PM, Pádraig Brady wrote:
> On 15/02/19 07:20, Eric Blake wrote:
>> Except that POSIX has the nasty requirement 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  | sort -n | sed 5q | wc -l')
>> 5
>> sort: write failed: 'standard output': Broken pipe
>> sort: write error
> 
>> You HAVE to use some other intermediate program if you want to override
>> an inherited ignored SIGPIPE in sh into an inherited default-behavior
>> SIGPIPE in sort.
> 
> Should we also propose to POSIX to allow trap to specify default?

That's what "trap - PIPE" is already supposed to do, except that POSIX
has the odd requirement that a signal that was inherited ignored cannot
be reset to default.

> Maybe `trap 0 PIPE` or similar?

Alas, bash has already defined that to mean the same as 'trap - EXIT PIPE'.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

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

[Assaf Gordon]

Hello,

Thanks for all comments (on and off list).
Attached an updated patch with documentation.

The supported options are:

 --default-signal[=SIG]  reset signal SIG to its default signal handler.
 without SIG, all known signals are included.
 multiple signals can be comma-separated.
 --ignore-signal[=SIG]   set signal SIG to be IGNORED.
 without SIG, all known signals are included.
 multiple signals can be comma-separated.
 -p  same as --default-signal=PIPE

(lower-case "-p" as to not conflict with BSD, but of course can be
changed to another letter).

The new 'env-signal-handler.sh' test passes on GNU/linux, non-gnu/linux
(alpine), and Free/Open/Net BSD.

Comments very welcomed,
 - assaf

>From 3542f1762c9f14e2275fe5e61d5d7f6275b420a9 Mon Sep 17 00:00:00 2001
From: Assaf Gordon 
Date: Fri, 15 Feb 2019 12:31:48 -0700
Subject: [PATCH] env: new options -p/--default-signal=SIG/--ignore-signal=SIG

New options to set signal handlers to default (SIG_DFL) or ignore
(SIG_IGN) This is useful to overcome POSIX limitation that shell must
not override inherited signal state, e.g. the second 'trap' here is
a no-op:

   trap '' PIPE && sh -c 'trap - PIPE ; seq inf | head -n1'

Instead use:

   trap '' PIPE && sh -c 'env -p seq inf | head -n1'

Similarly, the following will prevent CTRL-C from terminating the
program:

   env --ignore-signal=INT seq inf > /dev/null

See https://bugs.gnu.org/34488#8 .

* NEWS: Mention new options.
* doc/coreutils.texi (env invocation): Document new options.
* man/env.x: Add example of --default-signal=SIG usage.
* src/env.c (signals): New global variable.
(shortopts,longopts): Add new options.
(usage): Print new options.
(parse_signal_params): Parse comma-separated list of signals, store in
signals variable.
(reset_signal_handlers): Set each signal to SIG_DFL/SIG_IGN.
(main): Process new options.
* src/local.mk (src_env_SOURCES): Add operand2sig.c.
* tests/misc/env-signal-handler.sh: New test.
* tests/local.mk (all_tests): Add new test.
---
 NEWS |   3 +
 doc/coreutils.texi   |  43 
 man/env.x|  35 ++
 src/env.c| 127 +-
 src/local.mk |   1 +
 tests/local.mk   |   1 +
 tests/misc/env-signal-handler.sh | 146 +++
 7 files changed, 355 insertions(+), 1 deletion(-)
 create mode 100755 tests/misc/env-signal-handler.sh

diff --git a/NEWS b/NEWS
index fdde47593..5a8e8a3de 100644
--- a/NEWS
+++ b/NEWS
@@ -67,6 +67,9 @@ GNU coreutils NEWS-*- outline -*-
   test now supports the '-N FILE' unary operator (like e.g. bash) to check
   whether FILE exists and has been modified since it was last read.
 
+  env now supports '--default-singal[=SIG]' and '--ignore-signal[=SIG]'
+  options to set signal handlers before executing a program.
+
 ** New commands
 
   basenc is added to complement existing base64,base32 commands,
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index be35de490..57b209e07 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -17227,6 +17227,49 @@ chroot /chroot env --chdir=/srv true
 env --chdir=/build FOO=bar timeout 5 true
 @end example
 
+@item --default-signal[=@var{sig}]
+Reset signal @var{sig} to its default signal handler. Without @var{sig} all
+known signals are reset to their defaults. Multiple signals can be
+comma-separated. The following command runs @command{seq} with SIGINT and
+SIGPIPE set to their default (which is to terminate the program):
+
+@example
+env --default-signal=PIPE,INT seq 1000 | head -n1
+@end example
+
+In the following example:
+
+@example
+trap '' PIPE && sh -c 'trap - PIPE ; seq inf | head -n1'
+@end example
+
+The first trap command sets SIGPIPE to ignore.  The second trap command
+ostensibly sets it back to its  default, but POSIX mandates that the shell
+must not change inherited state of the signal - so it is a no-op.
+
+Using @option{--default-signal=PIPE} (or its shortcut @option{-p}) can be
+used to force the signal to  its  default behavior:
+
+@example
+trap '' PIPE && sh -c "env -p seq inf | head -n1'
+@end example
+
+
+@item --ignore-signal[=@var{sig}]
+Ignore signal @var{sig} when running a program. Without @var{sig} all
+known signals are set to ignore. Multiple signals can be
+comma-separated. The following command runs @command{seq} with SIGINT set
+to be ignored - pressing @kbd{Ctrl-C} will not terminate it:
+
+@example
+env --ignore-signal=INT seq inf > /dev/null
+@end example
+
+
+@item -p
+Equivalent to @option{--default-signal=PIPE} - sets SIGPIPE to its default
+behavior (terminate a program upon SIGPIPE).
+
 @item -v
 @itemx --debug
 @opindex -v
diff --git a/man/env.x b/man/env.x
index 8eea79655..5e0ef975e 100644
--- a/man/env.x
+++ b/man/env.x
@@ 

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

[Pádraig Brady]

On 15/02/19 07:20, Eric Blake wrote:
> Except that POSIX has the nasty requirement 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  | sort -n | sed 5q | wc -l')
> 5
> sort: write failed: 'standard output': Broken pipe
> sort: write error

> You HAVE to use some other intermediate program if you want to override
> an inherited ignored SIGPIPE in sh into an inherited default-behavior
> SIGPIPE in sort.

Should we also propose to POSIX to allow trap to specify default?
Maybe `trap 0 PIPE` or similar?

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

[Pádraig Brady]

On 15/02/19 14:11, Eric Blake wrote:
> On 2/15/19 3:40 PM, Assaf Gordon wrote:
>> 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  | 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.
> 
> Would we also want an easy way to ignore signals? That's a bit less of
> an issue, since you can use 'trap "" $SIG' in the shell; but having the
> symmetry in env may be nice (especially since using 'trap' is asymmetric
> in that you can't force the shell to un-ignore an inherited ignored signal).

I agree that ignore would be nice to add also.

>> Usage is:
>> env --default-signal=PIPE
>> env -P ##shortcut to reset SIGPIPE

BSD has -P for different reasons, so I would avoid that conflict

thanks a lot for working on this guys.
Pádraig

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

[Pádraig Brady]

On 16/02/19 23:24, 積丹尼 Dan Jacobson wrote:
> (I recall I heard about 50 years ago when pipe buffers first came to
> Unix they were supposed to be invisible to the user...)

Right. A lot of folks don't understand/handle them fully though:
https://www.pixelbeat.org/programming/sigpipe_handling.html

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

[Pádraig Brady]

On 17/02/19 13:00, Paul Eggert wrote:
> You make good points about nohup. Still, it's too bad that we'd have to add a 
> new command for such a trivial thing.
> 
> Perhaps it'd be better to overload 'env' instead, as you proposed earlier. 
> After 
> all, env is already being used for another little environmental thing (namely 
> changing directories), and if we find future needs for yet other little 
> things 
> (running with a different umask, say) we can put them under 'env' as well.

+1

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

[Paul Eggert]

You make good points about nohup. Still, it's too bad that we'd have to add a 
new command for such a trivial thing.


Perhaps it'd be better to overload 'env' instead, as you proposed earlier. After 
all, env is already being used for another little environmental thing (namely 
changing directories), and if we find future needs for yet other little things 
(running with a different umask, say) we can put them under 'env' as well.

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

[Assaf Gordon]

Hello,

On 2019-02-17 1:12 p.m., Paul Eggert wrote:

Assaf Gordon wrote:

I don't mind either way (env feature or new program).


This should be a new feature of 'nohup' not 'env', as 'nohup' is already 
about signal handling.  I don't see a need for a new program.


With 'nohup' I don't think there will be an easy (or at least intuitive
way) to 'untrap' SIGPIPE without affecting the output: STDOUT will be 
redirected to 'nohup.out' automatically (unless we add more options like 
"--no-redirect").


Example:

env -C /foo/bar PROGRAM## only change directory
env --default-signal=PIPE PROGRAM  ## only untrap SIGPIPE
env -i PROGRAM ## only empty environment

but

nohup --default-signal=PIPE PROGRAM

Will untrap SIGPIPE *and* SIGHUP *and* redirect stdout to a file.
So we'll need to add:

  nohup --no-redirect-stdout --default-signal=PIPE PROGRAM

Also,
nohup's manual pages warns:
   "NOTE:  your  shell  may  have  its own version of nohup, which
usually supersedes the version described here.  Please refer to
your shell's documentation for details about the options it
supports."

And if there is a built-in "nohup", it will confuse users who want to
use our new feature (and then more support questions, and we have to
explain how to use "env nohup" or "\nohup".

What do you think?

-assaf

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

[Paul Eggert]

Assaf Gordon wrote:

I don't mind either way (env feature or new program).


This should be a new feature of 'nohup' not 'env', as 'nohup' is already about 
signal handling.  I don't see a need for a new program.

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

[Assaf Gordon]

On 2019-02-16 4:56 p.m., Bernhard Voelker wrote:

On 2/15/19 10:40 PM, Assaf Gordon wrote:

$ seq  | env --default-signal PIPE sort -n | sed 5q | wc -l



  src/env.c| 90 +++-


That's quite a lot of new code.

What about a new program ... quick shot (and maybe an unlucky name): 'trap' ?



I don't mind either way (env feature or new program).

"trap" will get mixed-up with the shell's built-in command.
How about "untrap" (because the goal is to undo the 'trap' command),
and also there's no "untrap" executable name in debian, so no name 
conflicts?


will send an updated patch later today.

-assaf

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

[積丹尼 Dan Jacobson]

(I recall I heard about 50 years ago when pipe buffers first came to
Unix they were supposed to be invisible to the user...)

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

[Bernhard Voelker]

On 2/15/19 10:40 PM, Assaf Gordon wrote:
>> $ seq  | 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.

>  src/env.c| 90 
> +++-

That's quite a lot of new code.

What about a new program ... quick shot (and maybe an unlucky name): 'trap' ?

Have a nice day,
Berny

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

[Paul Eggert]

On 2/15/19 7:49 AM, 積丹尼 Dan Jacobson wrote:

Well the user thinks "hey I cut short 5 lines, 45 lines, 495 lines, and
then one I got a job at a big company and cut short 4995 lines and got
this error message just when the boss was looking."


I'm afraid I'll have to disagree on this one. This is a generic problem 
for any program that writes to standard output, and is not peculiar to 
'sort'. The 'sort' documentation is not the right place to document this 
issue, just as it's not the right place to document what happens when 
you type control-C to interrupt a sort.

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

[Eric Blake]

On 2/15/19 3:40 PM, Assaf Gordon wrote:
> 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  | 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.

Would we also want an easy way to ignore signals? That's a bit less of
an issue, since you can use 'trap "" $SIG' in the shell; but having the
symmetry in env may be nice (especially since using 'trap' is asymmetric
in that you can't force the shell to un-ignore an inherited ignored signal).

> 
> Usage is:
>     env --default-signal=PIPE
>     env -P ##shortcut to reset SIGPIPE

Nice, since that's probably the most common use case for it.

>     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

Also nice.


>  
> -static char const shortopts[] = "+C:iS:u:v0 \t";
> +/* if true, at least one signal handler should be reset.  */
> +static bool reset_signals ;

Extra space.

> +
> +/* if element [SIGNUM] is true, the signal handler's should be reset
> +   to its defaut. */

default

> +static bool signal_handlers[SIGNUM_BOUND];
> +
> +
> +static char const shortopts[] = "+C:iPS:u:v0 \t";

In the patch subject, you mentioned -D as a synonym to --default-signal,
but it's missing here.  (-P as a synonym to --default-signal=PIPE is
fine, though)

>  
>  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'},

Wait, you're making -P a synonym to --default-signal, even though -P
takes no options but --default-signal does? And the fact that it is
optional_argument means that you can't have a short option -D (that is,
'--default-signal FOO' is NOT the same as '--default-signal=FOO', but
treats FOO as a program name rather than a signal list).  optional
arguments can be odd; I'd consider using required_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);

Is blind usage() the best, or should we quote the unrecognized signal
name via error() to call attention to which part of the command line failed?

> +
> +  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 +{
> +  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, ))
> +  die (EXIT_CANCELED, errno, _("sigaction1(sig=%d) failed"), i);
> +
> +act.sa_handler = SIG_DFL;
> +if (sigaction (i, , NULL))
> +  die (EXIT_CANCELED, errno, _("sigaction2(sig=%d) failed"), i);

Why do you have to call sigaction() twice? Is it to make sure the rest
of  is set up correctly?  But what else in  needs setup if you
are going to set things to SIG_DFL?

> +
> +
> +}
> +}

Why 2 blank lines?

> +
>  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;

Hmm - you made it optional so that '--default-signal' and
'--default-signal=PIPE' can be synonyms.  If so, the help text in
usage() should definitely call out "--default-signal[=LIST]" to make it
obvious that LIST is an optional argument.


> +++ 

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

[Assaf Gordon]

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  | 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 
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 
 #include 
 #include 
+#include 
+#include 
 
 #include 
 #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; ihttps://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
+# -
+# 

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

[積丹尼 Dan Jacobson]

> "EB" == Eric Blake  writes:

>> And no fair saying "just save the output" (could be big) "into a file
>> first, and do head(1) or sed(1) on that."

EB> If you have an app that exits noisily on write failures to an early-exit
EB> pipe, your solutions are to quit ignoring SIGPIPE, or to change the
EB> pipeline to consume all of the output instead of exiting early and
EB> causing write failure.

OK, it seems that indeed it would then be best to document:

 If you want to make sure no such errors occur on large output,
 instead of
 $ sort | head
 use
 $ sort > file; head file

as an _easy to remember_ alternative amongst the many.

And OK put this in some general place, and have the sort, etc. pages
link to it. Thanks.

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

[積丹尼 Dan Jacobson]

> "AG" == Assaf Gordon  writes:
AG> The errors are not "random" - they happen because you explicitly
AG> cut short the output of a program.

Well the user thinks "hey I cut short 5 lines, 45 lines, 495 lines, and
then one I got a job at a big company and cut short 4995 lines and got
this error message just when the boss was looking."

So be sure to mention it. Thanks.

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

[Assaf Gordon]

severity 34488 wishlist
retitle 34488 doc: sort: expand on "broken pipe" (SIGPIPE) behavior
stop

Hello,

On 2019-02-15 7:43 a.m., 積丹尼 Dan Jacobson wrote:

Things start out cheery, but quickly get ugly,

$ for i in 9 99 999  9; do seq $i|sort -n|sed 5q|wc -l; done
5
5
5
5
sort: write failed: 'standard output': Broken pipe
sort: write error
5
sort: write failed: 'standard output': Broken pipe
sort: write error

Therefore, kindly add a sort --limit=n,


I don't think this is wise, as "head -n5" does exactly that in much more
generic way.


and/or on (info "(coreutils) sort invocation")
admit the problem, and give some workarounds, lest
our scripts occasionally spew error messages seemingly randomly,
just when the boss is looking.


Just to clarify: why do you think this a "problem" ?

This is the intended behavior of most proper programs:
Upon receiving SIGPIPE they should terminal with an error,
unless SIGPIPE is explicitly ignored.
The errors are not "random" - they happen because you explicitly
cut short the output of a program.

It is an important indication about how your pipe works,
and sort is not to blame, e.g.:

$ seq 10 | head -n1
1
seq: write error: Broken pipe

$ seq 100| cat | head -n1
1
cat: write error: Broken pipe
seq: write error: Broken pipe

This is a good indication that the entire output was not consumed,
and is very useful and important in some cases, e.g. when a program
crashes before consuming all input.

Here's a contrived example:

   $ seq 100 | sort -S 200 -T /foo/bar
   sort: cannot create temporary file in '/foo/bar': No such file or 
directory

   seq: write error: Broken pipe

I force "sort" to fail (limiting it's memory usage and pointing it to
non-existing temporarily directory).
It is then good to know that seq's output was cut short and not consumed.

If you know in advance you will trim the output of a program,
either hide the stderr with "2>/dev/null",
or use the shell's "trap PIPE" mechanism.


And no fair saying "just save the output" (could be big) "into a file
first, and do head(1) or sed(1) on that."


If you want to consume all input and just print the first 5 lines,
you can use "sed -n 1,5p" instead of "sed 5q" - no need for a temporary
file.


I'm marking this as a documentation "wishlist" item,
and patches are always welcomed.

regards,
 - assaf

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

[Eric Blake]

On 2/15/19 8:43 AM, 積丹尼 Dan Jacobson wrote:
> Things start out cheery, but quickly get ugly,
> 
> $ for i in 9 99 999  9; do seq $i|sort -n|sed 5q|wc -l; done
> 5
> 5
> 5
> 5
> sort: write failed: 'standard output': Broken pipe
> sort: write error
> 5
> sort: write failed: 'standard output': Broken pipe
> sort: write error

Your code is demonstrating what happens when sed ends processing without
consuming all of sort's output, to the point that sort is producing more
output than what stdio can buffer, and thus with enough pending output
will trigger a situation of SIGPIPE - but when SIGPIPE is ignored, that
instead turns into an EPIPE failure to write(), and sort treats all
write() failures as worthy of error reporting.  If you want sort to die
silently, then don't ignore SIGPIPE, so that sort won't see EPIPE and
thus won't be noisy.

$ (trap '' PIPE; seq  | sort -n | sed 5q | wc -l)
5
sort: write failed: 'standard output': Broken pipe
sort: write error
$ (trap - PIPE; seq  | sort -n | sed 5q | wc -l)
5

Except that POSIX has the nasty requirement 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  | sort -n | sed 5q | wc -l')
5
sort: write failed: 'standard output': Broken pipe
sort: write error

And unfortunately, there are several common cases of badly-behaved
environment setups that leave SIGPIPE ignored in the child (for example,
a quick google search found this:
https://blog.nelhage.com/2010/02/a-very-subtle-bug/)

You HAVE to use some other intermediate program if you want to override
an inherited ignored SIGPIPE in sh into an inherited default-behavior
SIGPIPE in sort.

Perhaps coreutils should teach 'env' a command-line option to forcefully
reset SIGPIPE back to default behavior (or add a new coreutil that does
the same idea), as a way to work around POSIX' requirement on sh.  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  | env --default-signal PIPE sort -n | sed 5q | wc -l
5

and avoid the EPIPE failures because sort is forced to start with
SIGPIPE handling rather than ignored.

> 
> Therefore, kindly add a sort --limit=n,

Not scalable.  The problem you encountered is NOT the fault of sort, but
is common to ALL utilities which dutifully report write() failures on
EPIPE errors, which in turn happens when SIGPIPE is ignored.  Adding an
option to every such utility that produces outtput is not a good use of
time.  If you want to change things universally, perhaps you should
petition POSIX to change the requirements to allow applications the
option to silently exit without an error message on EPIPE failures to
write(), instead of the current wording that all write() errors must be
diagnosed.

> and/or on (info "(coreutils) sort invocation")
> admit the problem, and give some workarounds, lest
> our scripts occasionally spew error messages seemingly randomly,
> just when the boss is looking.

The problem is not in sort, but in the fact that your environment is
ignoring SIGPIPE. Documenting something in sort doesn't scale, but
perhaps the documentation could mention SIGPIPE considerations in a more
global chapter covering all of the coreutils.

> 
> And no fair saying "just save the output" (could be big) "into a file
> first, and do head(1) or sed(1) on that."
> 

If you have an app that exits noisily on write failures to an early-exit
pipe, your solutions are to quit ignoring SIGPIPE, or to change the
pipeline to consume all of the output instead of exiting early and
causing write failure.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

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

[積丹尼 Dan Jacobson]

Things start out cheery, but quickly get ugly,

$ for i in 9 99 999  9; do seq $i|sort -n|sed 5q|wc -l; done
5
5
5
5
sort: write failed: 'standard output': Broken pipe
sort: write error
5
sort: write failed: 'standard output': Broken pipe
sort: write error

Therefore, kindly add a sort --limit=n,
and/or on (info "(coreutils) sort invocation")
admit the problem, and give some workarounds, lest
our scripts occasionally spew error messages seemingly randomly,
just when the boss is looking.

And no fair saying "just save the output" (could be big) "into a file
first, and do head(1) or sed(1) on that."