bug#41634: 'timeout' returning 124 and 133

2020-06-07 Thread Pádraig Brady

On 06/06/2020 00:37, Bernhard Voelker wrote:

On 2020-06-01 10:01, Jonny Grant wrote:

My mistake missing that. But could the 137 be listed explicitly?
ie.
"It may be  necessary  to  use the KILL (9) signal, since this signal cannot be 
caught, in which case the exit status is
137 (128+9) rather than 124."


thanks for the suggestion.

I think this could still be improved:
there is still the open question what happens when the KILL signal
is sent either to timeout(1) or to COMMAND.
The attached patch tries to clarify.


Thanks for improving that Bernhard.

It's generally easier to scan tables of values, than blocks of text.
How about we add the exit code table to the man page also?
I've adjusted your patch to do that (along with some other tweaks) in the 
attached.

cheers,
Pádraig
>From 267e0ce456dda4d468da4a2d002eaf226b35c5d8 Mon Sep 17 00:00:00 2001
From: Bernhard Voelker 
Date: Sun, 7 Jun 2020 12:58:14 +0100
Subject: [PATCH] doc: timeout: improve documentation of the exit status

* doc/coreutils.texi (timeout invocation): Document that the exit
status is 137 when the KILL signal is used, regardless of whether that
signal is sent to COMMAND or timeout.
* src/timeout.c (usage): Likewise. Also split out and expand
on the possible exit status values to a separate table.

Discussed at https://bugs.gnu.org/41634
---
 doc/coreutils.texi |  9 +++--
 src/timeout.c  | 21 +++--
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index f0684b1c5..3432fb294 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -18139,14 +18139,19 @@ which should be especially considered when specifying sub-second timeouts.
 Exit status:
 
 @display
-124 if @var{command} times out
+124 if @var{command} times out, and @option{--preserve-status} is not specified
 125 if @command{timeout} itself fails
 126 if @var{command} is found but cannot be invoked
 127 if @var{command} cannot be found
-137 if @var{command} is sent the KILL(9) signal (128+9)
+137 if @var{command} or @command{timeout} is sent the KILL(9) signal (128+9)
 the exit status of @var{command} otherwise
 @end display
 
+In case of the @samp{KILL(9)} signal, @command{timeout} returns with
+exit status 137, regardless of whether that signal is sent to @var{command}
+or to @command{timeout} itself, i.e., these cases cannot be distinguished.
+In the latter case, the @var{command} process may still be alive after
+@command{timeout} has forcefully been terminated.
 
 @node Process control
 @chapter Process control
diff --git a/src/timeout.c b/src/timeout.c
index 14ae88da5..abe00c729 100644
--- a/src/timeout.c
+++ b/src/timeout.c
@@ -269,12 +269,21 @@ DURATION is a floating point number with an optional suffix:\n\
 'd' for days.\nA duration of 0 disables the associated timeout.\n"), stdout);
 
   fputs (_("\n\
-If the command times out, and --preserve-status is not set, then exit with\n\
-status 124.  Otherwise, exit with the status of COMMAND.  If no signal\n\
-is specified, send the TERM signal upon timeout.  The TERM signal kills\n\
-any process that does not block or catch that signal.  It may be necessary\n\
-to use the KILL (9) signal, since this signal cannot be caught, in which\n\
-case the exit status is 128+9 rather than 124.\n"), stdout);
+Upon timeout send the TERM signal to COMMAND, if no other SIGNAL specified.\n\
+The TERM signal kills any process that does not block or catch that signal.\n\
+It may be necessary to use the KILL signal, since this signal can't be caught.\
+\n"), stdout);
+
+  fputs (_("\n\
+EXIT status:\n\
+  124  if COMMAND times out, and --preserve-status is not specified\n\
+  125  if the timeout command itself fails\n\
+  126  if COMMAND is found but cannot be invoked\n\
+  127  if COMMAND cannot be found\n\
+  137  if COMMAND (or timeout itself) is sent the KILL (9) signal (128+9)\n\
+  -the exit status of COMMAND otherwise\n\
+"), stdout);
+
   emit_ancillary_info (PROGRAM_NAME);
 }
   exit (status);
-- 
2.26.2



bug#41634: 'timeout' returning 124 and 133

2020-06-07 Thread Bernhard Voelker
On 2020-06-06 23:49, Jonny Grant wrote:
> Great patch.

thanks, Padraig meanwhile further improved it.

> How about writing signal 9 by the name, ie $ kill -s KILL
> likewise $ kill -s TERM

Where do you mean?

Have a nice day,
Berny





bug#41634: 'timeout' returning 124 and 133

2020-06-07 Thread Bernhard Voelker
Hi Padraig,

On 2020-06-07 14:13, Pádraig Brady wrote:
> How about we add the exit code table to the man page also?
> I've adjusted your patch to do that (along with some other tweaks) in the 
> attached.

nice, especially also:

> -124 if @var{command} times out
> +124 if @var{command} times out, and @option{--preserve-status} is not 
> specified

Minor, grammar-related question:

> +Upon timeout send the TERM signal to COMMAND, if no other SIGNAL 
> specified.\n\
___^
s/timeout/&,/ ?

I stumbled upon this sentence a couple of times - shouldn't there be a comma?
The word 'timeout' could be a regular noun, a verb, or in this special case
the name of the tool, so I had to read the first half several times.
But maybe it's only me as a non-native English speaker - commas are much
cheaper over here. ;-)

Thanks & have a nice day,
Berny





bug#41634: 'timeout' returning 124 and 133

2020-06-07 Thread Pádraig Brady

On 07/06/2020 13:38, Bernhard Voelker wrote:

Minor, grammar-related question:


+Upon timeout send the TERM signal to COMMAND, if no other SIGNAL specified.\n\

___^
s/timeout/&,/ ?

I stumbled upon this sentence a couple of times - shouldn't there be a comma?
The word 'timeout' could be a regular noun, a verb, or in this special case
the name of the tool, so I had to read the first half several times.
But maybe it's only me as a non-native English speaker - commas are much
cheaper over here. ;-)


When writing --help we tend to be space limited,
so I tend to write with punctuation mainly as disambiguation.
But indeed it is better to add the comma there, and we have space.

cheers,
Pádraig





bug#41634: 'timeout' returning 124 and 133

2020-06-07 Thread Bernhard Voelker
On 2020-06-07 15:34, Pádraig Brady wrote:
> [...] it is better to add the comma there [...]

thanks, pushed with that change:
https://git.savannah.gnu.org/cgit/coreutils.git/commit/?id=189776ff3b

Marking this as done.

Have a nice day,
Berny





bug#41634: 'timeout' returning 124 and 133

2020-06-07 Thread Jonny Grant




On 07/06/2020 13:39, Bernhard Voelker wrote:

On 2020-06-06 23:49, Jonny Grant wrote:

Great patch.


thanks, Padraig meanwhile further improved it.


How about writing signal 9 by the name, ie $ kill -s KILL
likewise $ kill -s TERM


Where do you mean?

Have a nice day,
Berny



Hi Berny

Sorry I was meaning to give an example of a shell command to send KILL, but 
maybe it's not necessary.

BTW, I saw the patch was applied. Great it's improved


I saw this new line is clearer:
"Upon timeout, send the TERM signal to COMMAND, if no other SIGNAL specified."

However, I thought even clearer is this variation :-
"Upon timeout, if no SIGNAL specified by --signal, send the TERM signal to 
COMMAND."

May I ask, do these texinfo changes also go into the man page?

This is the man page 8.32, and it doesn't match the html manual
https://www.man7.org/linux/man-pages/man1/timeout.1.html

I'm looking at the generated html manual:
https://www.gnu.org/software/coreutils/manual/coreutils.html#timeout-invocation

I don't know if these html pages can be updated to show the coreutil version on 
them at all at the top oand bottom?

Could an example be given on the man page and manual?

===
EXAMPLE

The command below gives an example to demonstrate the use of this, sending HUP  signal after 5 seconds, and sending the 
KILL signal after 10 seconds if 'ls' has not finished.

$ timeout -k 10s -s HUP 5s ls
===


My last question

There is -k, it would be clearer if it was possible to specify -t or --timeout,
"$ timeout -k 11s 6s ls"   This always looks ambiguous to me, but the 11s is the KILL, and the 6s is the regular TERM 
signal.


Would you consider supporting a -t ?
So then we could write
"$ timeout -t 6s -k 11s ls"

or even
"$ timeout --timeout=6s --kill-after=11s ls"


Cheers
Jonny