* [Bug] Exiting shell from function called by trap handler always produces status 0 @ 2018-10-08 13:02 ` Martijn Dekker 2018-10-09 8:49 ` Peter Stephenson 0 siblings, 1 reply; 11+ messages in thread From: Martijn Dekker @ 2018-10-08 13:02 UTC (permalink / raw) To: Zsh hackers list When a trap handler exits the shell using the 'exit' command within a function, the shell's exit status is zero even if another exit status was given as an argument to the 'exit' command. $ Src/zsh -c 'fn() { exit 13; }; trap "fn" EXIT' $ echo $? 0 (expected output: 13) I was able to reproduce this in zsh 5.4.1 and up, and not in 5.3.1 and earlier. Every other shell also exits with status 13. Looping through git commits starting from zsh-5.3.1, I found the commit that introduced this: d7110d8 (41012: Fix premature exit from nested function in EXIT trap). Thanks, - M. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Bug] Exiting shell from function called by trap handler always produces status 0 2018-10-08 13:02 ` [Bug] Exiting shell from function called by trap handler always produces status 0 Martijn Dekker @ 2018-10-09 8:49 ` Peter Stephenson 2018-10-09 9:58 ` Mikael Magnusson 2018-10-09 11:46 ` Martijn Dekker 0 siblings, 2 replies; 11+ messages in thread From: Peter Stephenson @ 2018-10-09 8:49 UTC (permalink / raw) To: zsh-workers On Mon, 2018-10-08 at 14:02 +0100, Martijn Dekker wrote: > When a trap handler exits the shell using the 'exit' command within a > function, the shell's exit status is zero even if another exit status > was given as an argument to the 'exit' command. > > $ Src/zsh -c 'fn() { exit 13; }; trap "fn" EXIT' > $ echo $? > 0 > (expected output: 13) diff --git a/Src/builtin.c b/Src/builtin.c index c5b319b..b81acdb 100644 --- a/Src/builtin.c +++ b/Src/builtin.c @@ -5709,7 +5709,13 @@ int shell_exiting; mod_export void zexit(int val, int from_where) { - /* Don't do anything recursively: see below */ + static int exit_val; + /* + * Don't do anything recursively: see below. + * Do, however, update exit status --- there's no nesting, + * a later value always overrides an earlier. + */ + exit_val = val; if (shell_exiting == -1) return; @@ -5757,7 +5763,7 @@ zexit(int val, int from_where) #endif } } - lastval = val; + lastval = exit_val; /* * Now we are committed to exiting any previous state * is irrelevant. Ensure trap can run. @@ -5771,9 +5777,9 @@ zexit(int val, int from_where) release_pgrp(); } if (mypid != getpid()) - _exit(val); + _exit(exit_val); else - exit(val); + exit(exit_val); } /* . (dot), source */ diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst index dce263f..3b5d4c6 100644 --- a/Test/C03traps.ztst +++ b/Test/C03traps.ztst @@ -863,6 +863,9 @@ F:Must be tested with a top-level script rather than source or function >a >b + $ZTST_testdir/../Src/zsh -fc 'fn() { exit 13; }; trap fn EXIT; exit' +13:Explicit exit in exit trap overrides status + %clean rm -f TRAPEXIT ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Bug] Exiting shell from function called by trap handler always produces status 0 2018-10-09 8:49 ` Peter Stephenson @ 2018-10-09 9:58 ` Mikael Magnusson 2018-10-09 11:46 ` Martijn Dekker 1 sibling, 0 replies; 11+ messages in thread From: Mikael Magnusson @ 2018-10-09 9:58 UTC (permalink / raw) To: Peter Stephenson; +Cc: zsh-workers On 10/9/18, Peter Stephenson <p.stephenson@samsung.com> wrote: > On Mon, 2018-10-08 at 14:02 +0100, Martijn Dekker wrote: >> When a trap handler exits the shell using the 'exit' command within a >> function, the shell's exit status is zero even if another exit status >> was given as an argument to the 'exit' command. >> >> $ Src/zsh -c 'fn() { exit 13; }; trap "fn" EXIT' >> $ echo $? >> 0 >> (expected output: 13) > > diff --git a/Src/builtin.c b/Src/builtin.c > index c5b319b..b81acdb 100644 > --- a/Src/builtin.c > +++ b/Src/builtin.c > @@ -5709,7 +5709,13 @@ int shell_exiting; > mod_export void > zexit(int val, int from_where) > { > - /* Don't do anything recursively: see below */ > + static int exit_val; > + /* > + * Don't do anything recursively: see below. > + * Do, however, update exit status --- there's no nesting, > + * a later value always overrides an earlier. > + */ > + exit_val = val; > if (shell_exiting == -1) > return; > > @@ -5757,7 +5763,7 @@ zexit(int val, int from_where) > #endif > } > } > - lastval = val; > + lastval = exit_val; > /* > * Now we are committed to exiting any previous state > * is irrelevant. Ensure trap can run. > @@ -5771,9 +5777,9 @@ zexit(int val, int from_where) > release_pgrp(); > } > if (mypid != getpid()) > - _exit(val); > + _exit(exit_val); > else > - exit(val); > + exit(exit_val); > } > > /* . (dot), source */ > diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst > index dce263f..3b5d4c6 100644 > --- a/Test/C03traps.ztst > +++ b/Test/C03traps.ztst > @@ -863,6 +863,9 @@ F:Must be tested with a top-level script rather than > source or function > >a > >b > > + $ZTST_testdir/../Src/zsh -fc 'fn() { exit 13; }; trap fn EXIT; exit' > +13:Explicit exit in exit trap overrides status > + > %clean > > rm -f TRAPEXIT We might as well make this test + $ZTST_testdir/../Src/zsh -fc 'fn() { exit $?+7; }; trap fn EXIT; exit 6' +13:Explicit exit in exit trap overrides status -- Mikael Magnusson ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Bug] Exiting shell from function called by trap handler always produces status 0 2018-10-09 8:49 ` Peter Stephenson 2018-10-09 9:58 ` Mikael Magnusson @ 2018-10-09 11:46 ` Martijn Dekker 2018-10-09 13:16 ` Peter Stephenson 1 sibling, 1 reply; 11+ messages in thread From: Martijn Dekker @ 2018-10-09 11:46 UTC (permalink / raw) To: zsh-workers Op 09-10-18 om 09:49 schreef Peter Stephenson: > On Mon, 2018-10-08 at 14:02 +0100, Martijn Dekker wrote: >> When a trap handler exits the shell using the 'exit' command within a >> function, the shell's exit status is zero even if another exit status >> was given as an argument to the 'exit' command. >> >> $ Src/zsh -c 'fn() { exit 13; }; trap "fn" EXIT' >> $ echo $? >> 0 >> (expected output: 13) > diff --git a/Src/builtin.c b/Src/builtin.c > index c5b319b..b81acdb 100644 [...] This still doesn't fix it. Output identical to above. > + $ZTST_testdir/../Src/zsh -fc 'fn() { exit 13; }; trap fn EXIT; exit' The extra '; exit' at the end makes it work correctly, but should not be necessary. Thanks, - M. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Bug] Exiting shell from function called by trap handler always produces status 0 2018-10-09 11:46 ` Martijn Dekker @ 2018-10-09 13:16 ` Peter Stephenson 2018-10-09 13:39 ` Daniel Shahaf 2018-10-09 20:09 ` Peter Stephenson 0 siblings, 2 replies; 11+ messages in thread From: Peter Stephenson @ 2018-10-09 13:16 UTC (permalink / raw) To: zsh-workers iOn Tue, 2018-10-09 at 12:46 +0100, Martijn Dekker wrote: > Op 09-10-18 om 09:49 schreef Peter Stephenson: > > On Mon, 2018-10-08 at 14:02 +0100, Martijn Dekker wrote: > > > > > > When a trap handler exits the shell using the 'exit' command within a > > > function, the shell's exit status is zero even if another exit status > > > was given as an argument to the 'exit' command. > > > > > > $ Src/zsh -c 'fn() { exit 13; }; trap "fn" EXIT' > > > $ echo $? > > > 0 > > > (expected output: 13) > > diff --git a/Src/builtin.c b/Src/builtin.c > > index c5b319b..b81acdb 100644 > [...] > > This still doesn't fix it. Output identical to above. Fair enough, it was me that added the explicit "exit" at the end, not you. Grumble. pws diff --git a/Src/builtin.c b/Src/builtin.c index ca3ef23..905edd9 100644 --- a/Src/builtin.c +++ b/Src/builtin.c @@ -5647,8 +5647,9 @@ bin_break(char *name, char **argv, UNUSED(Options ops), int func) if (stopmsg || (zexit(0,2), !stopmsg)) { retflag = 1; breaks = loops; - exit_pending = (num << 1) | 1; + exit_pending = 1; exit_level = locallevel; + exit_val = num; } } else zexit(num, 0); @@ -5698,6 +5699,42 @@ checkjobs(void) /**/ int shell_exiting; +/* + * Exit status if explicitly set by an exit command. + * This is complicated by the fact the exit command may be within + * a function whose state we need to unwind (exit_pending set + * and the exit will happen up the stack), or we may need to execute + * additional code such as a trap after we are committed to exiting + * (shell_exiting and the exit will happen down the stack). + * + * It's lucky this is all so obvious there is no possibility of any + * bugs. (C.f. the entire rest of the shell.) + */ +/**/ +int exit_val; + +/* + * Actually exit the shell, working out the status locally. + * This is exit_val if "exit" has explicitly been called in the shell, + * else lastval. + */ + +/**/ +void +realexit(void) +{ + exit(exit_val ? exit_val : lastval); +} + +/* As realexit(), but call _exit instead */ + +/**/ +void +_realexit(void) +{ + _exit(exit_val ? exit_val : lastval); +} + /* exit the shell. val is the return value of the shell. * * from_where is * 1 if zexit is called because of a signal @@ -5709,7 +5746,6 @@ int shell_exiting; mod_export void zexit(int val, int from_where) { - static int exit_val; /* * Don't do anything recursively: see below. * Do, however, update exit status --- there's no nesting, diff --git a/Src/exec.c b/Src/exec.c index 1d537af..c4a2740 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -738,7 +738,7 @@ execute(LinkList args, int flags, int defpath) if (!search_defpath(arg0, pbuf, PATH_MAX)) { if (commandnotfound(arg0, args) == 0) - _exit(lastval); + _realexit(); zerr("command not found: %s", arg0); _exit(127); } @@ -802,7 +802,7 @@ execute(LinkList args, int flags, int defpath) if (eno) zerr("%e: %s", eno, arg0); else if (commandnotfound(arg0, args) == 0) - _exit(lastval); + _realexit(); else zerr("command not found: %s", arg0); _exit((eno == EACCES || eno == ENOEXEC) ? 126 : 127); @@ -1012,6 +1012,7 @@ entersubsh(int flags, struct entersubsh_ret *retp) unsettrap(sig); monitor = isset(MONITOR); job_control_ok = monitor && (flags & ESUB_JOB_CONTROL) && isset(POSIXJOBS); + exit_val = 0; /* parent exit status is irrelevant */ if (flags & ESUB_NOMONITOR) opts[MONITOR] = 0; if (!isset(MONITOR)) { @@ -1535,9 +1536,9 @@ sublist_done: if (sigtrapped[SIGEXIT]) dotrap(SIGEXIT); if (mypid != getpid()) - _exit(lastval); + _realexit(); else - exit(lastval); + realexit(); } if (errreturn) { retflag = 1; @@ -2934,7 +2935,7 @@ execcmd_exec(Estate state, Execcmd_params eparams, /* autoload the builtin if necessary */ if (!(hn = resolvebuiltin(cmdarg, hn))) { if (forked) - _exit(lastval); + _realexit(); return; } if (type != WC_TYPESET) @@ -3115,7 +3116,7 @@ execcmd_exec(Estate state, Execcmd_params eparams, lastval = 1; errflag |= ERRFLAG_ERROR; if (forked) - _exit(lastval); + _realexit(); return; } } @@ -3210,7 +3211,7 @@ execcmd_exec(Estate state, Execcmd_params eparams, lastval = 1; errflag |= ERRFLAG_ERROR; if (forked) - _exit(lastval); + _realexit(); return; } else if (!nullcmd || !*nullcmd || opts[SHNULLCMD]) { if (!args) @@ -3230,7 +3231,7 @@ execcmd_exec(Estate state, Execcmd_params eparams, } else if ((cflags & BINF_PREFIX) && (cflags & BINF_COMMAND)) { lastval = 0; if (forked) - _exit(lastval); + _realexit(); return; } else { /* @@ -3242,7 +3243,7 @@ execcmd_exec(Estate state, Execcmd_params eparams, zerr("no match"); lastval = 1; if (forked) - _exit(lastval); + _realexit(); return; } cmdoutval = use_cmdoutval ? lastval : 0; @@ -3260,7 +3261,7 @@ execcmd_exec(Estate state, Execcmd_params eparams, fflush(xtrerr); } if (forked) - _exit(lastval); + _realexit(); return; } } else if (isset(RESTRICTED) && (cflags & BINF_EXEC) && do_exec) { @@ -3268,7 +3269,7 @@ execcmd_exec(Estate state, Execcmd_params eparams, (char *) getdata(firstnode(args))); lastval = 1; if (forked) - _exit(lastval); + _realexit(); return; } @@ -3304,7 +3305,7 @@ execcmd_exec(Estate state, Execcmd_params eparams, if (oautocont >= 0) opts[AUTOCONTINUE] = oautocont; if (forked) - _exit(lastval); + _realexit(); return; } break; @@ -3315,7 +3316,7 @@ execcmd_exec(Estate state, Execcmd_params eparams, /* autoload the builtin if necessary */ if (!(hn = resolvebuiltin(cmdarg, hn))) { if (forked) - _exit(lastval); + _realexit(); return; } break; @@ -3333,7 +3334,7 @@ execcmd_exec(Estate state, Execcmd_params eparams, if (oautocont >= 0) opts[AUTOCONTINUE] = oautocont; if (forked) - _exit(lastval); + _realexit(); return; } @@ -3412,7 +3413,7 @@ execcmd_exec(Estate state, Execcmd_params eparams, if (oautocont >= 0) opts[AUTOCONTINUE] = oautocont; if (forked) - _exit(lastval); + _realexit(); return; } } @@ -3442,7 +3443,7 @@ execcmd_exec(Estate state, Execcmd_params eparams, if (oautocont >= 0) opts[AUTOCONTINUE] = oautocont; if (forked) - _exit(lastval); + _realexit(); return; } @@ -4118,13 +4119,13 @@ execcmd_exec(Estate state, Execcmd_params eparams, if (do_exec) { if (subsh) - _exit(lastval); + _realexit(); /* If we are exec'ing a command, and we are not in a subshell, * * then check if we should save the history file. */ if (isset(RCS) && interact && !nohistsave) savehistfile(NULL, 1, HFILE_USE_OPTIONS); - exit(lastval); + realexit(); } if (restorelist) restore_params(restorelist, removelist); @@ -4215,7 +4216,7 @@ execcmd_exec(Estate state, Execcmd_params eparams, closem(FDT_UNUSED, 1); if (thisjob != -1) waitjobs(); - _exit(lastval); + _realexit(); } fixfds(save); @@ -4631,7 +4632,7 @@ getoutput(char *cmd, int qt) execode(prog, 0, 1, "cmdsubst"); cmdpop(); close(1); - _exit(lastval); + _realexit(); zerr("exit returned in child!!"); kill(getpid(), SIGKILL); return NULL; @@ -4825,7 +4826,7 @@ getoutputfile(char *cmd, char **eptr) execode(prog, 0, 1, "equalsubst"); cmdpop(); close(1); - _exit(lastval); + _realexit(); zerr("exit returned in child!!"); kill(getpid(), SIGKILL); return NULL; @@ -4938,7 +4939,7 @@ getproc(char *cmd, char **eptr) execode(prog, 0, 1, out ? "outsubst" : "insubst"); cmdpop(); zclose(out); - _exit(lastval); + _realexit(); return NULL; #endif /* HAVE_FIFOS and PATH_DEV_FD not defined */ } @@ -4986,7 +4987,7 @@ getpipe(char *cmd, int nullexec) cmdpush(CS_CMDSUBST); execode(prog, 0, 1, out ? "outsubst" : "insubst"); cmdpop(); - _exit(lastval); + _realexit(); return 0; } @@ -5927,7 +5928,7 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) * exit command was handled. */ stopmsg = 1; - zexit(exit_pending >> 1, 0); + zexit(exit_val, 0); } } diff --git a/Src/init.c b/Src/init.c index e9e6be9..838c2c2 100644 --- a/Src/init.c +++ b/Src/init.c @@ -157,7 +157,7 @@ loop(int toplevel, int justonce) * Handle that now. */ stopmsg = 1; - zexit(exit_pending >> 1, 0); + zexit(exit_val, 0); } if (tok == LEXERR && !lastval) lastval = 1; @@ -215,14 +215,14 @@ loop(int toplevel, int justonce) clearerr(stderr); } if (subsh) /* how'd we get this far in a subshell? */ - exit(lastval); + realexit(); if (((!interact || sourcelevel) && errflag) || retflag) break; if (isset(SINGLECOMMAND) && toplevel) { dont_queue_signals(); if (sigtrapped[SIGEXIT]) dotrap(SIGEXIT); - exit(lastval); + realexit(); } if (justonce) break; @@ -1358,7 +1358,7 @@ init_misc(char *cmd, char *zsh_name) bshin = fdopen(SHIN, "r"); execstring(cmd, 0, 1, "cmdarg"); stopmsg = 1; - zexit(lastval, 0); + zexit(exit_val ? exit_val : lastval, 0); } if (interact && isset(RCS)) diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst index eab01e5..bab0b0a 100644 --- a/Test/C03traps.ztst +++ b/Test/C03traps.ztst @@ -869,6 +869,10 @@ F:Must be tested with a top-level script rather than source or function $ZTST_testdir/../Src/zsh -fc 'fn() { exit $?+8; }; trap fn EXIT; exit 7' 15:Progated exit status through exit trap + $ZTST_testdir/../Src/zsh -fc 'fn() { exit 13; }; trap fn EXIT' +13:Explicit exit in exit trap overrides implicit exit status + %clean rm -f TRAPEXIT + ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Bug] Exiting shell from function called by trap handler always produces status 0 2018-10-09 13:16 ` Peter Stephenson @ 2018-10-09 13:39 ` Daniel Shahaf 2018-10-09 13:43 ` Peter Stephenson [not found] ` <1539092591.3286.12.camel@samsung.com> 2018-10-09 20:09 ` Peter Stephenson 1 sibling, 2 replies; 11+ messages in thread From: Daniel Shahaf @ 2018-10-09 13:39 UTC (permalink / raw) To: Peter Stephenson, zsh-workers Peter Stephenson wrote on Tue, 09 Oct 2018 14:16 +0100: > +++ b/Src/builtin.c > @@ -5698,6 +5699,42 @@ checkjobs(void) > +/* > + * Exit status if explicitly set by an exit command. > + * This is complicated by the fact the exit command may be within > + * a function whose state we need to unwind (exit_pending set > + * and the exit will happen up the stack), or we may need to execute > + * additional code such as a trap after we are committed to exiting > + * (shell_exiting and the exit will happen down the stack). > + * > + * It's lucky this is all so obvious there is no possibility of any > + * bugs. (C.f. the entire rest of the shell.) > + */ > +/**/ > +int exit_val; Shouldn't this variable be initialized? A function-local 'static' (below) is implicitly initialized to 0, but a non-static global is not implicitly initialized. > + > +/* > + * Actually exit the shell, working out the status locally. > + * This is exit_val if "exit" has explicitly been called in the shell, > + * else lastval. > + */ > + > @@ -5709,7 +5746,6 @@ int shell_exiting; > mod_export void > zexit(int val, int from_where) > { > - static int exit_val; > /* > * Don't do anything recursively: see below. > * Do, however, update exit status --- there's no nesting, ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Bug] Exiting shell from function called by trap handler always produces status 0 2018-10-09 13:39 ` Daniel Shahaf @ 2018-10-09 13:43 ` Peter Stephenson [not found] ` <1539092591.3286.12.camel@samsung.com> 1 sibling, 0 replies; 11+ messages in thread From: Peter Stephenson @ 2018-10-09 13:43 UTC (permalink / raw) To: Daniel Shahaf, zsh-workers On Tue, 2018-10-09 at 13:39 +0000, Daniel Shahaf wrote: > Peter Stephenson wrote on Tue, 09 Oct 2018 14:16 +0100: > A function-local 'static' (below) is implicitly initialized to 0, but > a non-static global is not implicitly initialized. If that's really the case then the entire shell (look around at other cases), and pretty much every other non-trivial programme I've ever written, is fundamentally broken. I'm not going to try to fix that now. I can't quote chapter and verse but in all interesting cases they're initialised to zero. pws ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <1539092591.3286.12.camel@samsung.com>]
* Re: [Bug] Exiting shell from function called by trap handler always produces status 0 [not found] ` <1539092591.3286.12.camel@samsung.com> @ 2018-10-09 13:50 ` Peter Stephenson 0 siblings, 0 replies; 11+ messages in thread From: Peter Stephenson @ 2018-10-09 13:50 UTC (permalink / raw) To: zsh-workers On Tue, 2018-10-09 at 14:43 +0100, Peter Stephenson wrote: > On Tue, 2018-10-09 at 13:39 +0000, Daniel Shahaf wrote: > > Peter Stephenson wrote on Tue, 09 Oct 2018 14:16 +0100: > > A function-local 'static' (below) is implicitly initialized to 0, but > > a non-static global is not implicitly initialized. > If that's really the case then the entire shell (look around at other > cases), and pretty much every other non-trivial programme I've ever > written, is fundamentally broken. I'm not going to try to fix that now. > > I can't quote chapter and verse but in all interesting cases they're > initialised to zero. Looking around at some discussions of this subject on websites... I wonder if you're getting confused between static and automatic *allocation* vs. global and static *linkage*? Linkage has no effect here: global variables are statically allocated. pws ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Bug] Exiting shell from function called by trap handler always produces status 0 2018-10-09 13:16 ` Peter Stephenson 2018-10-09 13:39 ` Daniel Shahaf @ 2018-10-09 20:09 ` Peter Stephenson 2018-10-10 8:56 ` Daniel Shahaf 1 sibling, 1 reply; 11+ messages in thread From: Peter Stephenson @ 2018-10-09 20:09 UTC (permalink / raw) To: zsh-workers Here's a slight improvement --- we can "exit 0" even if the last command status is non-zero. pws diff --git a/Src/builtin.c b/Src/builtin.c index e01e035cc..8dcdcc024 100644 --- a/Src/builtin.c +++ b/Src/builtin.c @@ -5723,7 +5723,7 @@ int exit_val; void realexit(void) { - exit(exit_val ? exit_val : lastval); + exit((shell_exiting || exit_pending) ? exit_val : lastval); } /* As realexit(), but call _exit instead */ @@ -5732,7 +5732,7 @@ realexit(void) void _realexit(void) { - _exit(exit_val ? exit_val : lastval); + _exit((shell_exiting || exit_pending) ? exit_val : lastval); } /* exit the shell. val is the return value of the shell. * ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Bug] Exiting shell from function called by trap handler always produces status 0 2018-10-09 20:09 ` Peter Stephenson @ 2018-10-10 8:56 ` Daniel Shahaf 2018-10-10 9:31 ` Peter Stephenson 0 siblings, 1 reply; 11+ messages in thread From: Daniel Shahaf @ 2018-10-10 8:56 UTC (permalink / raw) To: zsh-workers Peter Stephenson wrote on Tue, 09 Oct 2018 21:09 +0100: > Here's a slight improvement --- we can "exit 0" even if the last command > status is non-zero. Can we add a test for this? I tried to write one but couldn't nail down the behaviour that differed. FWIW: % Src/zsh -fc 'fn() { exit 0 }; trap fn EXIT; false' ; echo $? 1 % /usr/bin/zsh -fc 'fn() { exit 0 }; trap fn EXIT; false' ; echo $? 0 % ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Bug] Exiting shell from function called by trap handler always produces status 0 2018-10-10 8:56 ` Daniel Shahaf @ 2018-10-10 9:31 ` Peter Stephenson 0 siblings, 0 replies; 11+ messages in thread From: Peter Stephenson @ 2018-10-10 9:31 UTC (permalink / raw) To: zsh-workers On Wed, 2018-10-10 at 08:56 +0000, Daniel Shahaf wrote: > Peter Stephenson wrote on Tue, 09 Oct 2018 21:09 +0100: > > > > Here's a slight improvement --- we can "exit 0" even if the last command > > status is non-zero. > Can we add a test for this? I tried to write one but couldn't nail down > the behaviour that differed. > > FWIW: > > % Src/zsh -fc 'fn() { exit 0 }; trap fn EXIT; false' ; echo $? > 1 > % /usr/bin/zsh -fc 'fn() { exit 0 }; trap fn EXIT; false' ; echo $? > 0 > % I missed another case for implicit exit that was masking the effect. We go through zexit() at this point as it's a standard end-of-command-list case, though whether that's actually necessary (running additional tests mostly associated with interactive shells) I don't know. It's also a bit confusing that the exit trap has already happened in that point --- in execlist(), I think --- even though it's more sensibly associated in this case with the code inside zexit() which will also run it (and if it had done the exit status would have been overridden and we wouldn't have seen this). But I don't think there are any real world consequences for this with this change. I don't think an exit_pending can actually be picked up at that point, but it's easier and safer to be consistent. pws diff --git a/Src/builtin.c b/Src/builtin.c index e01e035..8dcdcc0 100644 --- a/Src/builtin.c +++ b/Src/builtin.c @@ -5723,7 +5723,7 @@ int exit_val; void realexit(void) { - exit(exit_val ? exit_val : lastval); + exit((shell_exiting || exit_pending) ? exit_val : lastval); } /* As realexit(), but call _exit instead */ @@ -5732,7 +5732,7 @@ realexit(void) void _realexit(void) { - _exit(exit_val ? exit_val : lastval); + _exit((shell_exiting || exit_pending) ? exit_val : lastval); } /* exit the shell. val is the return value of the shell. * diff --git a/Src/init.c b/Src/init.c index 838c2c2..cec9143 100644 --- a/Src/init.c +++ b/Src/init.c @@ -1358,7 +1358,7 @@ init_misc(char *cmd, char *zsh_name) bshin = fdopen(SHIN, "r"); execstring(cmd, 0, 1, "cmdarg"); stopmsg = 1; - zexit(exit_val ? exit_val : lastval, 0); + zexit((exit_pending || shell_exiting) ? exit_val : lastval, 0); } if (interact && isset(RCS)) diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst index bab0b0a..57daf8d 100644 --- a/Test/C03traps.ztst +++ b/Test/C03traps.ztst @@ -872,6 +872,9 @@ F:Must be tested with a top-level script rather than source or function $ZTST_testdir/../Src/zsh -fc 'fn() { exit 13; }; trap fn EXIT' 13:Explicit exit in exit trap overrides implicit exit status + $ZTST_testdir/../Src/zsh -fc 'fn() { exit 0; }; trap fn EXIT; false' +0:Explicit exit status 0 in exit trap overrides implicit non-zero status + %clean rm -f TRAPEXIT ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-10-10 9:31 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20181009012624epcas1p44f2ae223f663713a980af4be735e5a3f@epcas1p4.samsung.com>
2018-10-08 13:02 ` [Bug] Exiting shell from function called by trap handler always produces status 0 Martijn Dekker
2018-10-09 8:49 ` Peter Stephenson
2018-10-09 9:58 ` Mikael Magnusson
2018-10-09 11:46 ` Martijn Dekker
2018-10-09 13:16 ` Peter Stephenson
2018-10-09 13:39 ` Daniel Shahaf
2018-10-09 13:43 ` Peter Stephenson
[not found] ` <1539092591.3286.12.camel@samsung.com>
2018-10-09 13:50 ` Peter Stephenson
2018-10-09 20:09 ` Peter Stephenson
2018-10-10 8:56 ` Daniel Shahaf
2018-10-10 9:31 ` Peter Stephenson
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).