zsh-workers
 help / color / Atom feed
* Exit code 130 kills pipeline
@ 2020-06-17 15:45 Roman Perepelitsa
  2020-06-17 16:58 ` Peter Stephenson
  2020-06-17 17:42 ` Mikael Magnusson
  0 siblings, 2 replies; 7+ messages in thread
From: Roman Perepelitsa @ 2020-06-17 15:45 UTC (permalink / raw)
  To: Zsh hackers list

Is it expected that the following command produces no output?

  ( exit 130 ) | { sleep 1; echo hello }

Roman.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Exit code 130 kills pipeline
  2020-06-17 15:45 Exit code 130 kills pipeline Roman Perepelitsa
@ 2020-06-17 16:58 ` Peter Stephenson
  2020-06-18  5:49   ` Roman Perepelitsa
  2020-06-17 17:42 ` Mikael Magnusson
  1 sibling, 1 reply; 7+ messages in thread
From: Peter Stephenson @ 2020-06-17 16:58 UTC (permalink / raw)
  To: Zsh hackers list

> On 17 June 2020 at 16:45 Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote:
> Is it expected that the following command produces no output?
> 
>   ( exit 130 ) | { sleep 1; echo hello }

There's some slightly dodgy use of flags when testing for signals.

I'm not sure "or"ing in the 128 is really needed at all but to, keep
this minimal, all we need to do is test separately whether the process
actually got a signal.  If it didn't we've no business looking for
SIGINT or SIGQUIT.

pws

diff --git a/Src/jobs.c b/Src/jobs.c
index 8353f1152..0d4993554 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -442,7 +442,7 @@ update_job(Job jn)
     Process pn;
     int job;
     int val = 0, status = 0;
-    int somestopped = 0, inforeground = 0;
+    int somestopped = 0, inforeground = 0, signalled = 0;
 
     for (pn = jn->auxprocs; pn; pn = pn->next) {
 #ifdef WIFCONTINUED
@@ -464,12 +464,15 @@ update_job(Job jn)
 	    return;                        /* so no need to update job table entry         */
 	if (WIFSTOPPED(pn->status))        /* some processes are stopped                   */
 	    somestopped = 1;               /* so job is not done, but entry needs updating */
-	if (!pn->next)                     /* last job in pipeline determines exit status  */
+	if (!pn->next) {
+	    /* last job in pipeline determines exit status  */
 	    val = (WIFSIGNALED(pn->status) ?
 		   0200 | WTERMSIG(pn->status) :
 		   (WIFSTOPPED(pn->status) ?
 		    0200 | WEXITSTATUS(pn->status) :
 		    WEXITSTATUS(pn->status)));
+	    signalled = WIFSIGNALED(pn->status);
+	}
 	if (pn->pid == jn->gleader)        /* if this process is process group leader      */
 	    status = pn->status;
     }
@@ -564,7 +567,7 @@ update_job(Job jn)
 		}
 		/* If we have `foo|while true; (( x++ )); done', and hit
 		 * ^C, we have to stop the loop, too. */
-		if ((val & 0200) && inforeground == 1 &&
+		if (signalled && inforeground == 1 &&
 		    ((val & ~0200) == SIGINT || (val & ~0200) == SIGQUIT)) {
 		    if (!errbrk_saved) {
 			errbrk_saved = 1;
@@ -581,7 +584,7 @@ update_job(Job jn)
 		adjustwinsize(0);
 	    }
 	}
-    } else if (list_pipe && (val & 0200) && inforeground == 1 &&
+    } else if (list_pipe && signalled && inforeground == 1 &&
 	       ((val & ~0200) == SIGINT || (val & ~0200) == SIGQUIT)) {
 	if (!errbrk_saved) {
 	    errbrk_saved = 1;

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Exit code 130 kills pipeline
  2020-06-17 15:45 Exit code 130 kills pipeline Roman Perepelitsa
  2020-06-17 16:58 ` Peter Stephenson
@ 2020-06-17 17:42 ` Mikael Magnusson
  2020-06-17 17:46   ` Roman Perepelitsa
  1 sibling, 1 reply; 7+ messages in thread
From: Mikael Magnusson @ 2020-06-17 17:42 UTC (permalink / raw)
  To: Roman Perepelitsa; +Cc: Zsh hackers list

On 6/17/20, Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote:
> Is it expected that the following command produces no output?
>
>   ( exit 130 ) | { sleep 1; echo hello }

No?
% ( exit 130 ) | { sleep 1; echo hello }
hello

-- 
Mikael Magnusson

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Exit code 130 kills pipeline
  2020-06-17 17:42 ` Mikael Magnusson
@ 2020-06-17 17:46   ` Roman Perepelitsa
  2020-06-17 20:34     ` Mikael Magnusson
  0 siblings, 1 reply; 7+ messages in thread
From: Roman Perepelitsa @ 2020-06-17 17:46 UTC (permalink / raw)
  To: Mikael Magnusson; +Cc: Zsh hackers list

On Wed, Jun 17, 2020 at 7:42 PM Mikael Magnusson <mikachu@gmail.com> wrote:
>
> On 6/17/20, Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote:
> > Is it expected that the following command produces no output?
> >
> >   ( exit 130 ) | { sleep 1; echo hello }
>
> No?

Good point. I should've specified the environment.

% docker run -it --rm zshusers/zsh:5.8 \
  zsh -c '( exit 130 ) | { sleep 1; echo hello }'

(no output)

Roman.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Exit code 130 kills pipeline
  2020-06-17 17:46   ` Roman Perepelitsa
@ 2020-06-17 20:34     ` Mikael Magnusson
  0 siblings, 0 replies; 7+ messages in thread
From: Mikael Magnusson @ 2020-06-17 20:34 UTC (permalink / raw)
  To: Roman Perepelitsa; +Cc: Zsh hackers list

On 6/17/20, Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote:
> On Wed, Jun 17, 2020 at 7:42 PM Mikael Magnusson <mikachu@gmail.com> wrote:
>>
>> On 6/17/20, Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote:
>> > Is it expected that the following command produces no output?
>> >
>> >   ( exit 130 ) | { sleep 1; echo hello }
>>
>> No?
>
> Good point. I should've specified the environment.
>
> % docker run -it --rm zshusers/zsh:5.8 \
>   zsh -c '( exit 130 ) | { sleep 1; echo hello }'
>
> (no output)
>
> Roman.

I get no output from
zsh -c '( exit 130 ) | { sleep 1; echo hello }'
but i do get output from
zsh -ic '( exit 130 ) | { sleep 1; echo hello }'
and again no output from
zsh -fic '( exit 130 ) | { sleep 1; echo hello }'

so it's probably something weird :).

-- 
Mikael Magnusson

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Exit code 130 kills pipeline
  2020-06-17 16:58 ` Peter Stephenson
@ 2020-06-18  5:49   ` Roman Perepelitsa
  2020-06-18 10:49     ` Peter Stephenson
  0 siblings, 1 reply; 7+ messages in thread
From: Roman Perepelitsa @ 2020-06-18  5:49 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

On Wed, Jun 17, 2020 at 6:59 PM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> diff --git a/Src/jobs.c b/Src/jobs.c
> index 8353f1152..0d4993554 100644
> --- a/Src/jobs.c
> +++ b/Src/jobs.c
> [...]

Thanks, Peter. I confirm that this patch fixes the issue I was having.

Roman.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Exit code 130 kills pipeline
  2020-06-18  5:49   ` Roman Perepelitsa
@ 2020-06-18 10:49     ` Peter Stephenson
  0 siblings, 0 replies; 7+ messages in thread
From: Peter Stephenson @ 2020-06-18 10:49 UTC (permalink / raw)
  To: Zsh hackers list

Not a direct part of this bug, but while I'm looking I might at least
mention it, though I suspect it's very minor in practice.

Just looking at the code that sets the return value:

	    /* last job in pipeline determines exit status  */
	    val = (WIFSIGNALED(pn->status) ?
		   0200 | WTERMSIG(pn->status) :
		   (WIFSTOPPED(pn->status) ?
		    0200 | WEXITSTATUS(pn->status) :
		    WEXITSTATUS(pn->status)));
	    signalled = WIFSIGNALED(pn->status);

The only case where the bottom bits are the signal is WIFSIGNALED();
that's the fix that's just gone in, so no problem with that one.

However, I notice the Linux manual page says WEXITSTATUS() is only
useful if WIFEXITED() is true for the status.  So the remaining
two cases could possibly do with a bit of surgery.  I'm guessing
WIFSTOPPED() usually reports a status of 128 + 0, and if the
process is stopped (not exited) I'm not sure if the user gets
to see the status anyway.  So this may be a non-issue.

pws

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-17 15:45 Exit code 130 kills pipeline Roman Perepelitsa
2020-06-17 16:58 ` Peter Stephenson
2020-06-18  5:49   ` Roman Perepelitsa
2020-06-18 10:49     ` Peter Stephenson
2020-06-17 17:42 ` Mikael Magnusson
2020-06-17 17:46   ` Roman Perepelitsa
2020-06-17 20:34     ` Mikael Magnusson

zsh-workers

Archives are clonable: git clone --mirror http://inbox.vuxu.org/zsh-workers

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.zsh.workers


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git