From: Peter Stephenson <p.stephenson@samsung.com>
To: <zsh-workers@zsh.org>
Subject: Re: [Bug] Exiting shell from function called by trap handler always produces status 0
Date: Tue, 9 Oct 2018 14:16:45 +0100 [thread overview]
Message-ID: <20181009131646eucas1p2eb07d8fa34e97de25db60d6a56c584a4~b8x2n0O1S2407324073eucas1p2z@eucas1p2.samsung.com> (raw)
In-Reply-To: <205ea5e4-4dc9-2ebf-9437-0822195e1a9a@inlv.org>
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
+
next prev parent reply other threads:[~2018-10-09 13:27 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20181009012624epcas1p44f2ae223f663713a980af4be735e5a3f@epcas1p4.samsung.com>
2018-10-08 13:02 ` 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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='20181009131646eucas1p2eb07d8fa34e97de25db60d6a56c584a4~b8x2n0O1S2407324073eucas1p2z@eucas1p2.samsung.com' \
--to=p.stephenson@samsung.com \
--cc=zsh-workers@zsh.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).