From 551ff842721d6ca83727dbe6cd40178f46cc8201 Mon Sep 17 00:00:00 2001 From: Peter Stephenson Date: Sun, 16 Sep 2018 19:13:38 +0100 Subject: 43464: Another attachtty() fix. If list_pipe_job triggered more than once we need to know the most recent process group leader, so record that both if the attach happened in the main shell on in entersubsh(). Also don't pass back proocess group for ESUB_ASYNC subshells. --- Src/exec.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'Src/exec.c') diff --git a/Src/exec.c b/Src/exec.c index b9af9ea63..a667b078d 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -1036,7 +1036,7 @@ entersubsh(int flags, struct entersubsh_ret *retp) if (!(flags & ESUB_ASYNC)) attachtty(jobtab[thisjob].gleader); } - if (retp) { + if (retp && !(flags & ESUB_ASYNC)) { retp->gleader = jobtab[list_pipe_job].gleader; retp->list_pipe_job = list_pipe_job; } @@ -1058,12 +1058,13 @@ entersubsh(int flags, struct entersubsh_ret *retp) !jobtab[list_pipe_job].gleader) jobtab[list_pipe_job].gleader = jobtab[thisjob].gleader; setpgrp(0L, jobtab[thisjob].gleader); - if (!(flags & ESUB_ASYNC)) + if (!(flags & ESUB_ASYNC)) { attachtty(jobtab[thisjob].gleader); - if (retp) { - retp->gleader = jobtab[thisjob].gleader; - if (list_pipe_job != thisjob) - retp->list_pipe_job = list_pipe_job; + if (retp) { + retp->gleader = jobtab[thisjob].gleader; + if (list_pipe_job != thisjob) + retp->list_pipe_job = list_pipe_job; + } } } } -- cgit v1.2.3 From db67cc2f4b5b1282cb2be719ebb6bfd5a52f9169 Mon Sep 17 00:00:00 2001 From: Daniel Shahaf Date: Tue, 18 Sep 2018 18:57:12 +0000 Subject: 43489: Add error checking on a new write() call. Silences a compiler warning (write(2) is declared with warn_unused_result in current Debian Unstable). --- ChangeLog | 4 ++++ Src/exec.c | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) (limited to 'Src/exec.c') diff --git a/ChangeLog b/ChangeLog index 0841cb325..b5c41edb5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2018-09-25 Daniel Shahaf + + * 43489: Src/exec.c: Add error checking on a new write() call. + 2018-09-25 Peter Stephenson * 43543: Src/jobs.c: Improvements to 43535: attempt to keep diff --git a/Src/exec.c b/Src/exec.c index a667b078d..1d537af24 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -2745,7 +2745,10 @@ execcmd_fork(Estate state, int how, int type, Wordcode varspc, flags |= ESUB_JOB_CONTROL; *filelistp = jobtab[thisjob].filelist; entersubsh(flags, &esret); - write(synch[1], &esret, sizeof(esret)); + if (write_loop(synch[1], (const void *) &esret, sizeof(esret)) != sizeof(esret)) { + zerr("Failed to send entersubsh_ret report: %e", errno); + return -1; + } close(synch[1]); zclose(close_if_forked); -- cgit v1.2.3 From bbccbe0c85887bfc15c57a0c5eb97e59f7cb9fa7 Mon Sep 17 00:00:00 2001 From: Peter Stephenson Date: Tue, 9 Oct 2018 14:38:26 +0100 Subject: 43660: extend 43653 when final exit is implicit. Combine logic for case after committed to exit (shell_exiting) with case where exit occurred in a function we nee to unwind (exit_pending). Add sarky note for future generations to be confused at. --- Src/builtin.c | 40 ++++++++++++++++++++++++++++++++++++++-- Src/exec.c | 49 +++++++++++++++++++++++++------------------------ Src/init.c | 8 ++++---- Test/C03traps.ztst | 4 ++++ 4 files changed, 71 insertions(+), 30 deletions(-) (limited to 'Src/exec.c') diff --git a/Src/builtin.c b/Src/builtin.c index ca3ef23be..e01e035cc 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 1d537af24..c4a2740c0 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 e9e6be9b4..838c2c2d1 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 eab01e5c1..bab0b0a0c 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 + -- cgit v1.2.3 From d1095bdf744c190c7e8ff126ba02caea8f63880d Mon Sep 17 00:00:00 2001 From: Kamil Dudka Date: Wed, 7 Nov 2018 14:04:54 +0100 Subject: 43723: file descriptor could leak on fork error --- ChangeLog | 2 ++ Src/exec.c | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) (limited to 'Src/exec.c') diff --git a/ChangeLog b/ChangeLog index ed44bcf26..009c33e30 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,7 @@ 2018-11-09 Peter Stephenson + * 43792: Kamil: Src/exec.c: file descriptor could leak on fork error. + * 43793: Kamil: Src/Zle/computil.c: could overrun buffer by 1 byte. * 43791: Kamil: Src/Modules/clone.c: file descriptor was closed twice. diff --git a/Src/exec.c b/Src/exec.c index c4a2740c0..042ba065a 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -4805,7 +4805,8 @@ getoutputfile(char *cmd, char **eptr) } if ((cmdoutpid = pid = zfork(NULL)) == -1) { - /* fork or open error */ + /* fork error */ + close(fd); child_unblock(); return nam; } else if (pid) { -- cgit v1.2.3