Is it expected that the following command produces no output? ( exit 130 ) | { sleep 1; echo hello } Roman.
> 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;
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
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.
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
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.
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