From 3c74891fcd68d37c1629943f703ac70428e3ce9f Mon Sep 17 00:00:00 2001 From: Peter Stephenson Date: Fri, 13 Apr 2018 12:09:34 +0100 Subject: 42630: Improve process group handling in pipelines. If process group leader exits, allow a newly forked process to become process leader. If a foreground job, reattach the shell to the terminal until that happens. Unblock signals when reading output for command subsitution so that we can do this reattaching immediately. --- Src/exec.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) (limited to 'Src/exec.c') diff --git a/Src/exec.c b/Src/exec.c index 216057aa7..f6c768f37 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -4576,10 +4576,20 @@ readoutput(int in, int qt, int *readerror) char *buf, *ptr; int bsiz, c, cnt = 0; FILE *fin; + int q = queue_signal_level(); fin = fdopen(in, "r"); ret = newlinklist(); ptr = buf = (char *) hcalloc(bsiz = 64); + /* + * We need to be sensitive to SIGCHLD else we can be + * stuck forever with important processes unreaped. + * The case that triggered this was where the exiting + * process is group leader of the foreground process and we need + * to reclaim the terminal else ^C doesn't work. + */ + dont_queue_signals(); + child_unblock(); while ((c = fgetc(fin)) != EOF || errno == EINTR) { if (c == EOF) { errno = 0; @@ -4592,13 +4602,18 @@ readoutput(int in, int qt, int *readerror) cnt++; } if (++cnt >= bsiz) { - char *pp = (char *) hcalloc(bsiz *= 2); + char *pp; + queue_signals(); + pp = (char *) hcalloc(bsiz *= 2); + dont_queue_signals(); memcpy(pp, buf, cnt - 1); ptr = (buf = pp) + cnt - 1; } *ptr++ = c; } + child_block(); + restore_queue_signals(q); if (readerror) *readerror = ferror(fin) ? errno : 0; fclose(fin); -- cgit v1.2.3 From 9e2afb92987d7fd96a838c15b6641cc1b634a825 Mon Sep 17 00:00:00 2001 From: Peter Stephenson Date: Fri, 20 Apr 2018 10:33:10 +0100 Subject: 42684 (with extra comments): Fork early if in bg. In execcmd the case of running the last command in a pipeline asynchronously for the purpose of & and &! is easy to work out, and we can avoid side effects and unnecessary execution time in the parent shell by forking earlier. --- ChangeLog | 5 ++ Src/exec.c | 230 ++++++++++++++++++++++++++++++++++++------------------------- 2 files changed, 140 insertions(+), 95 deletions(-) (limited to 'Src/exec.c') diff --git a/ChangeLog b/ChangeLog index 41b993bfe..653798257 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2018-04-20 Peter Stephenson + + * 42684 (with extra comments): Src/exec.c: Fork earlier + when executing command if run in background. + 2018-04-19 Peter Stephenson * 42686: Src/signals.c: Fix 42630 only to change process groups diff --git a/Src/exec.c b/Src/exec.c index f6c768f37..1b622d56f 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -2707,6 +2707,78 @@ static void execcmd_getargs(LinkList preargs, LinkList args, int expand) } } +/**/ +static int +execcmd_fork(Estate state, int how, int type, Wordcode varspc, + LinkList *filelistp, char *text, int oautocont) +{ + pid_t pid; + int synch[2], flags; + char dummy; + struct timeval bgtime; + + child_block(); + + if (pipe(synch) < 0) { + zerr("pipe failed: %e", errno); + return -1; + } else if ((pid = zfork(&bgtime)) == -1) { + close(synch[0]); + close(synch[1]); + lastval = 1; + errflag |= ERRFLAG_ERROR; + return -1; + } + if (pid) { + close(synch[1]); + read_loop(synch[0], &dummy, 1); + close(synch[0]); + if (how & Z_ASYNC) { + lastpid = (zlong) pid; + } else if (!jobtab[thisjob].stty_in_env && varspc) { + /* search for STTY=... */ + Wordcode p = varspc; + wordcode ac; + + while (wc_code(ac = *p) == WC_ASSIGN) { + if (!strcmp(ecrawstr(state->prog, p + 1, NULL), "STTY")) { + jobtab[thisjob].stty_in_env = 1; + break; + } + p += (WC_ASSIGN_TYPE(ac) == WC_ASSIGN_SCALAR ? + 3 : WC_ASSIGN_NUM(ac) + 2); + } + } + addproc(pid, text, 0, &bgtime); + if (oautocont >= 0) + opts[AUTOCONTINUE] = oautocont; + pipecleanfilelist(jobtab[thisjob].filelist, 1); + return pid; + } + + /* pid == 0 */ + close(synch[0]); + flags = ((how & Z_ASYNC) ? ESUB_ASYNC : 0) | ESUB_PGRP; + if ((type != WC_SUBSH) && !(how & Z_ASYNC)) + flags |= ESUB_KEEPTRAP; + if (type == WC_SUBSH && !(how & Z_ASYNC)) + flags |= ESUB_JOB_CONTROL; + *filelistp = jobtab[thisjob].filelist; + entersubsh(flags); + close(synch[1]); + + if (sigtrapped[SIGINT] & ZSIG_IGNORED) + holdintr(); +#ifdef HAVE_NICE + /* Check if we should run background jobs at a lower priority. */ + if ((how & Z_ASYNC) && isset(BGNICE)) + if (nice(5) < 0) + zwarn("nice(5) failed: %e", errno); +#endif /* HAVE_NICE */ + + return 0; +} + /* * Execute a command at the lowest level of the hierarchy. */ @@ -3074,6 +3146,29 @@ execcmd_exec(Estate state, Execcmd_params eparams, esprefork = (magic_assign || (isset(MAGICEQUALSUBST) && type != WC_TYPESET)) ? PREFORK_TYPESET : 0; + if (how & Z_ASYNC) { + /* + * If running in the background, we don't need any of + * the rest of this functino to affect the state in the + * main shell, so fork immediately. + * + * In other cases we may need to process the command line + * a bit further before we make the decision. + */ + text = getjobtext(state->prog, eparams->beg); + switch (execcmd_fork(state, how, type, varspc, &filelist, + text, oautocont)) { + case -1: + goto fatal; + case 0: + break; + default: + return; + } + forked = 1; + } else + text = NULL; + if (args) { if (eparams->htok) prefork(args, esprefork, NULL); @@ -3226,11 +3321,9 @@ execcmd_exec(Estate state, Execcmd_params eparams, } /* Get the text associated with this command. */ - if ((how & Z_ASYNC) || + if (!text && (!sfcontext && (jobbing || (how & Z_TIMED)))) text = getjobtext(state->prog, eparams->beg); - else - text = NULL; /* * Set up special parameter $_ @@ -3378,7 +3471,8 @@ execcmd_exec(Estate state, Execcmd_params eparams, /************************************************************************** * Do we need to fork? We need to fork if: * - * 1) The command is supposed to run in the background. (or) * + * 1) The command is supposed to run in the background. This * + * case is now handled above (forked = 1 here). (or) * * 2) There is no `exec' flag, and either: * * a) This is a builtin or shell function with output piped somewhere. * * b) This is an external command and we can't do a `fake exec'. * @@ -3397,99 +3491,45 @@ execcmd_exec(Estate state, Execcmd_params eparams, * current shell. * **************************************************************************/ - if ((how & Z_ASYNC) || - (!do_exec && - (((is_builtin || is_shfunc) && output) || - (!is_cursh && (last1 != 1 || nsigtrapped || havefiles() || - fdtable_flocks))))) { - - pid_t pid; - int synch[2], flags; - char dummy; - struct timeval bgtime; - - child_block(); - - if (pipe(synch) < 0) { - zerr("pipe failed: %e", errno); - goto fatal; - } else if ((pid = zfork(&bgtime)) == -1) { - close(synch[0]); - close(synch[1]); - lastval = 1; - errflag |= ERRFLAG_ERROR; - goto fatal; - } - if (pid) { - - close(synch[1]); - read_loop(synch[0], &dummy, 1); - close(synch[0]); - if (how & Z_ASYNC) { - lastpid = (zlong) pid; - } else if (!jobtab[thisjob].stty_in_env && varspc) { - /* search for STTY=... */ - Wordcode p = varspc; - wordcode ac; - - while (wc_code(ac = *p) == WC_ASSIGN) { - if (!strcmp(ecrawstr(state->prog, p + 1, NULL), "STTY")) { - jobtab[thisjob].stty_in_env = 1; - break; - } - p += (WC_ASSIGN_TYPE(ac) == WC_ASSIGN_SCALAR ? - 3 : WC_ASSIGN_NUM(ac) + 2); - } + if (!forked) { + if (!do_exec && + (((is_builtin || is_shfunc) && output) || + (!is_cursh && (last1 != 1 || nsigtrapped || havefiles() || + fdtable_flocks)))) { + switch (execcmd_fork(state, how, type, varspc, &filelist, + text, oautocont)) { + case -1: + goto fatal; + case 0: + break; + default: + return; } - addproc(pid, text, 0, &bgtime); - if (oautocont >= 0) - opts[AUTOCONTINUE] = oautocont; - pipecleanfilelist(jobtab[thisjob].filelist, 1); - return; - } - /* pid == 0 */ - close(synch[0]); - flags = ((how & Z_ASYNC) ? ESUB_ASYNC : 0) | ESUB_PGRP; - if ((type != WC_SUBSH) && !(how & Z_ASYNC)) - flags |= ESUB_KEEPTRAP; - if (type == WC_SUBSH && !(how & Z_ASYNC)) - flags |= ESUB_JOB_CONTROL; - filelist = jobtab[thisjob].filelist; - entersubsh(flags); - close(synch[1]); - forked = 1; - if (sigtrapped[SIGINT] & ZSIG_IGNORED) - holdintr(); -#ifdef HAVE_NICE - /* Check if we should run background jobs at a lower priority. */ - if ((how & Z_ASYNC) && isset(BGNICE)) - if (nice(5) < 0) - zwarn("nice(5) failed: %e", errno); -#endif /* HAVE_NICE */ - - } else if (is_cursh) { - /* This is a current shell procedure that didn't need to fork. * - * This includes current shell procedures that are being exec'ed, * - * as well as null execs. */ - jobtab[thisjob].stat |= STAT_CURSH; - if (!jobtab[thisjob].procs) - jobtab[thisjob].stat |= STAT_NOPRINT; - if (is_builtin) - jobtab[thisjob].stat |= STAT_BUILTIN; - } else { - /* This is an exec (real or fake) for an external command. * - * Note that any form of exec means that the subshell is fake * - * (but we may be in a subshell already). */ - is_exec = 1; - /* - * If we are in a subshell environment anyway, say we're forked, - * even if we're actually not forked because we know the - * subshell is exiting. This ensures SHLVL reflects the current - * shell, and also optimises out any save/restore we'd need to - * do if we were returning to the main shell. - */ - if (type == WC_SUBSH) forked = 1; + } else if (is_cursh) { + /* This is a current shell procedure that didn't need to fork. * + * This includes current shell procedures that are being exec'ed, * + * as well as null execs. */ + jobtab[thisjob].stat |= STAT_CURSH; + if (!jobtab[thisjob].procs) + jobtab[thisjob].stat |= STAT_NOPRINT; + if (is_builtin) + jobtab[thisjob].stat |= STAT_BUILTIN; + } else { + /* This is an exec (real or fake) for an external command. * + * Note that any form of exec means that the subshell is fake * + * (but we may be in a subshell already). */ + is_exec = 1; + /* + * If we are in a subshell environment anyway, say we're forked, + * even if we're actually not forked because we know the + * subshell is exiting. This ensures SHLVL reflects the current + * shell, and also optimises out any save/restore we'd need to + * do if we were returning to the main shell. + */ + if (type == WC_SUBSH) + forked = 1; + } } if ((esglob = !(cflags & BINF_NOGLOB)) && args && eparams->htok) { -- cgit v1.2.3 From 95d861d0283e47c64980c844d66be44c6c4aad8a Mon Sep 17 00:00:00 2001 From: Peter Stephenson Date: Tue, 24 Apr 2018 10:57:03 +0100 Subject: 42708: fix for process substitution. Don't close associated file descriptors in the closem() tidy up function as they should remain visible to external processes. Override if about to exit. Unit test for the failing case: note this relies on the existence of /proc/self/fd or equivalent. --- ChangeLog | 7 +++++++ Src/Modules/clone.c | 2 +- Src/Modules/zpty.c | 2 +- Src/exec.c | 24 ++++++++++++++++-------- Test/D03procsubst.ztst | 7 +++++++ 5 files changed, 32 insertions(+), 10 deletions(-) (limited to 'Src/exec.c') diff --git a/ChangeLog b/ChangeLog index 632a35717..535cf9f00 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2018-04-24 Peter Stephenson + + * 42708: Src/exec.c, Src/Modules/clone.c, Src/Modules/zpty.c, + Test/D03procsubst.ztst: don't close file descriptors for + process substitution on closem() tidy up unless we are exiting + as they need to be visible to external processes. + 2018-04-23 Peter Stephenson * 42705: Src/signals.c: another fix for 42630 --- also check diff --git a/Src/Modules/clone.c b/Src/Modules/clone.c index 930429248..ef6275dcf 100644 --- a/Src/Modules/clone.c +++ b/Src/Modules/clone.c @@ -69,7 +69,7 @@ bin_clone(char *nam, char **args, UNUSED(Options ops), UNUSED(int func)) dup2(ttyfd,2); if (ttyfd > 2) close(ttyfd); - closem(0); + closem(FDT_UNUSED, 0); close(coprocin); close(coprocout); /* Acquire a controlling terminal */ diff --git a/Src/Modules/zpty.c b/Src/Modules/zpty.c index 1c93a1d02..2f83f7ce6 100644 --- a/Src/Modules/zpty.c +++ b/Src/Modules/zpty.c @@ -400,7 +400,7 @@ newptycmd(char *nam, char *pname, char **args, int echo, int nblock) dup2(slave, 1); dup2(slave, 2); - closem(0); + closem(FDT_UNUSED, 0); close(slave); close(master); close(coprocin); diff --git a/Src/exec.c b/Src/exec.c index 1b622d56f..ddd9ee0ad 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -698,7 +698,7 @@ execute(LinkList args, int flags, int defpath) * Note that we don't close fd's attached to process substitution * here, which should be visible to external processes. */ - closem(FDT_XTRACE); + closem(FDT_XTRACE, 0); #ifndef FD_CLOEXEC if (SHTTY != -1) { close(SHTTY); @@ -3943,7 +3943,7 @@ execcmd_exec(Estate state, Execcmd_params eparams, LinkList assigns = (LinkList)0; int postassigns = eparams->postassigns; if (forked) - closem(FDT_INTERNAL); + closem(FDT_INTERNAL, 0); if (postassigns) { Wordcode opc = state->pc; state->pc = eparams->assignspc; @@ -4129,7 +4129,7 @@ execcmd_exec(Estate state, Execcmd_params eparams, if (errflag) _exit(1); } - closem(FDT_INTERNAL); + closem(FDT_INTERNAL, 0); if (coprocin != -1) { zclose(coprocin); coprocin = -1; @@ -4191,7 +4191,7 @@ execcmd_exec(Estate state, Execcmd_params eparams, for (i = 0; i < 10; i++) if (fdtable[i] != FDT_UNUSED) close(i); - closem(FDT_UNUSED); + closem(FDT_UNUSED, 1); if (thisjob != -1) waitjobs(); _exit(lastval); @@ -4367,16 +4367,24 @@ fixfds(int *save) * * Close any that are marked as used if "how" is FDT_UNUSED, else * close any with the value "how". + * + * If "all" is zero, we'll skip cases where we need the file + * descriptor to be visible externally. */ /**/ mod_export void -closem(int how) +closem(int how, int all) { int i; for (i = 10; i <= max_zsh_fd; i++) if (fdtable[i] != FDT_UNUSED && + /* + * Process substitution needs to be visible to user; + * fd's are explicitly cleaned up by filelist handling. + */ + (all || fdtable[i] != FDT_PROC_SUBST) && (how == FDT_UNUSED || (fdtable[i] & FDT_TYPE_MASK) == how)) { if (i == SHTTY) SHTTY = -1; @@ -4859,7 +4867,7 @@ getproc(char *cmd, char **eptr) addproc(pid, NULL, 1, &bgtime); return pnam; } - closem(FDT_UNUSED); + closem(FDT_UNUSED, 0); fd = open(pnam, out ? O_WRONLY | O_NOCTTY : O_RDONLY | O_NOCTTY); if (fd == -1) { zerr("can't open %s: %e", pnam, errno); @@ -4898,7 +4906,7 @@ getproc(char *cmd, char **eptr) } entersubsh(ESUB_ASYNC|ESUB_PGRP); redup(pipes[out], out); - closem(FDT_UNUSED); /* this closes pipes[!out] as well */ + closem(FDT_UNUSED, 0); /* this closes pipes[!out] as well */ #endif /* PATH_DEV_FD */ cmdpush(CS_CMDSUBST); @@ -4948,7 +4956,7 @@ getpipe(char *cmd, int nullexec) } entersubsh(ESUB_PGRP); redup(pipes[out], out); - closem(FDT_UNUSED); /* this closes pipes[!out] as well */ + closem(FDT_UNUSED, 0); /* this closes pipes[!out] as well */ cmdpush(CS_CMDSUBST); execode(prog, 0, 1, out ? "outsubst" : "insubst"); cmdpop(); diff --git a/Test/D03procsubst.ztst b/Test/D03procsubst.ztst index ca8d56ff5..1ef55821b 100644 --- a/Test/D03procsubst.ztst +++ b/Test/D03procsubst.ztst @@ -149,3 +149,10 @@ fi 0:proc subst fd in forked subshell closed in parent (external command) >1 1 + + procfunc() { + cat <(cat "$@") + } + procfunc <(echo argument) +0:With /proc/self file descriptors must not be tidied up too early +>argument -- cgit v1.2.3 From 76b5b3a504f5a2d9062a82dec279c3d88d57e8ee Mon Sep 17 00:00:00 2001 From: Daniel Shahaf Date: Wed, 25 Apr 2018 15:31:36 +0000 Subject: unposted: Correct process substitution buffer size in the PATH_DEV_FD codepath. --- ChangeLog | 5 +++++ Src/exec.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) (limited to 'Src/exec.c') diff --git a/ChangeLog b/ChangeLog index cb5354e62..c641b21d8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2018-04-25 Daniel Shahaf + + * unposted: Src/exec.c: Correct process substitution buffer + size in the PATH_DEV_FD codepath. + 2018-04-25 Peter Stephenson * 42714: Test/W02jobs.ztst, Test/ztst.zsh: add basic fg and bg diff --git a/Src/exec.c b/Src/exec.c index ddd9ee0ad..ee55aac8c 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -4882,7 +4882,7 @@ getproc(char *cmd, char **eptr) zerr("process substitution %s cannot be used here", cmd); return NULL; } - pnam = hcalloc(strlen(PATH_DEV_FD) + 6); + pnam = zhalloc(strlen(PATH_DEV_FD) + 1 + DIGBUFSIZE); if (!(prog = parsecmd(cmd, eptr))) return NULL; if (mpipe(pipes) < 0) -- cgit v1.2.3 From f7519811e1bbe990ff1c3d499ffb70cfc2d034f8 Mon Sep 17 00:00:00 2001 From: Ricardo Giorni Date: Sun, 29 Apr 2018 12:05:39 -0700 Subject: 47201: fix 42355 for multiple backslashes --- ChangeLog | 4 ++++ Src/exec.c | 12 ++++++------ Test/A04redirect.ztst | 24 ++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 6 deletions(-) (limited to 'Src/exec.c') diff --git a/ChangeLog b/ChangeLog index 0bf5502b6..c2d9b1c7b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2018-04-29 Barton E. Schaefer + + * Ricardo Giorni: 47201: fix 42355 for multiple backslashes + 2018-04-26 Peter Stephenson * c.f. 42726: Test/W02jobs.ztst: Back off fg and bg tests as they diff --git a/Src/exec.c b/Src/exec.c index ee55aac8c..64cf5ae46 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -4435,16 +4435,16 @@ gethere(char **strp, int typ) bptr = buf + bsiz; bsiz *= 2; } - if (lexstop) + if (lexstop || c == '\n') break; - if (c == '\n') { - if (!qt && bptr > t && *(bptr - 1) == '\\') { - /* line continuation */ + if (!qt && c == '\\') { + *bptr++ = c; + c = hgetc(); + if (c == '\n') { bptr--; c = hgetc(); continue; - } else - break; + } } *bptr++ = c; c = hgetc(); diff --git a/Test/A04redirect.ztst b/Test/A04redirect.ztst index b5b65cf5d..1e17dddd4 100644 --- a/Test/A04redirect.ztst +++ b/Test/A04redirect.ztst @@ -174,6 +174,30 @@ >some stuff >to test >tab\stripping +>Last line + + heretest() { + print First line + cat <<-HERE + $foo\\ + $foo + some\\ \ + stuff + to\ + test \\ + more backslash craziness\\\\\\\\\ + wild + HERE + print Last line + } + heretest +0:No line continuation in here-document on escaped backslash +>First line +>bar\ +>bar +>some\ stuff +>to test \ +>more backslash craziness\\\\ wild >Last line # -- cgit v1.2.3 From faf0035e532cde45528806e7a05ad28a0ab7c0fb Mon Sep 17 00:00:00 2001 From: Peter Stephenson Date: Fri, 20 Apr 2018 11:17:06 +0100 Subject: unposted (branch fork_early): Fork early for pipelines. If executing a command not at the end of a pipeline, and not optimised in the calling code, we can fork before "prefork" substitutions, at the same point as for background commands. Move fork before our preliminary scan of arguments to find a command: in the cases of early fork we don't need this information before forking. Ensure we _exit if forked in execcmd_exec(). Rationalise use of forks and pipes. Ensure we _exit instead of returning from execcmd_exec() if we have forked. Before the optimisation code after the fork always ran to the check at the end, but that code is overkill for the logic between the early fork and the existing one. Remove old workaround to fork in caller of execcmd for current shell constructs as no longer needed with early fork below. Close input of newly created pipe on fork (destined for RHS of pipe which we never execute): this replaces a workaround from zsh-workers/32171, commit 9887fc3d7b. Set last1 on early fork as needed by some instances of shell constructs on LHS of pipeline to know they are exiting. --- ChangeLog | 8 ++++ Src/exec.c | 145 +++++++++++++++++++++++++++---------------------------------- 2 files changed, 71 insertions(+), 82 deletions(-) (limited to 'Src/exec.c') diff --git a/ChangeLog b/ChangeLog index f93d3d654..8cbcc80bc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2018-05-01 Peter Stephenson + + * from branch fork_early, c.f. 42702: Src/exec.c: Various changes + to make forking for a command work better. Move the fork even + earlier; add case of non-final pipeline elements; _exit when + forked; remove previous early fork in caller; replace fix for + leaked file descriptor in pipeline handling. + 2018-04-29 Oliver Kiddle * Matthew Martin: 42730: Completion/Unix/Command/_rmdir, diff --git a/Src/exec.c b/Src/exec.c index 64cf5ae46..08c4ea95a 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -1878,8 +1878,6 @@ static void execpline2(Estate state, wordcode pcode, int how, int input, int output, int last1) { - pid_t pid; - int pipes[2]; struct execcmd_params eparams; if (breaks || retflag) @@ -1900,65 +1898,21 @@ execpline2(Estate state, wordcode pcode, } if (WC_PIPE_TYPE(pcode) == WC_PIPE_END) { execcmd_analyse(state, &eparams); - execcmd_exec(state, &eparams, input, output, how, last1 ? 1 : 2); + execcmd_exec(state, &eparams, input, output, how, last1 ? 1 : 2, -1); } else { + int pipes[2]; int old_list_pipe = list_pipe; - int subsh_close = -1; - Wordcode next = state->pc + (*state->pc), start_pc; + Wordcode next = state->pc + (*state->pc); - start_pc = ++state->pc; + ++state->pc; execcmd_analyse(state, &eparams); if (mpipe(pipes) < 0) { /* FIXME */ } - /* if we are doing "foo | bar" where foo is a current * - * shell command, do foo in a subshell and do the * - * rest of the pipeline in the current shell. */ - if ((eparams.type >= WC_CURSH || !eparams.args) - && (how & Z_SYNC)) { - int synch[2]; - struct timeval bgtime; - - if (pipe(synch) < 0) { - zerr("pipe failed: %e", errno); - lastval = 1; - errflag |= ERRFLAG_ERROR; - return; - } else if ((pid = zfork(&bgtime)) == -1) { - close(synch[0]); - close(synch[1]); - lastval = 1; - errflag |= ERRFLAG_ERROR; - return; - } else if (pid) { - char dummy, *text; - - text = getjobtext(state->prog, start_pc); - addproc(pid, text, 0, &bgtime); - close(synch[1]); - read_loop(synch[0], &dummy, 1); - close(synch[0]); - } else { - zclose(pipes[0]); - close(synch[0]); - entersubsh(((how & Z_ASYNC) ? ESUB_ASYNC : 0) - | ESUB_PGRP | ESUB_KEEPTRAP); - close(synch[1]); - if (sigtrapped[SIGEXIT]) - { - unsettrap(SIGEXIT); - } - execcmd_exec(state, &eparams, input, pipes[1], how, 1); - _exit(lastval); - } - } else { - /* otherwise just do the pipeline normally. */ - addfilelist(NULL, pipes[0]); - subsh_close = pipes[0]; - execcmd_exec(state, &eparams, input, pipes[1], how, 0); - } + addfilelist(NULL, pipes[0]); + execcmd_exec(state, &eparams, input, pipes[1], how, 0, pipes[0]); zclose(pipes[1]); state->pc = next; @@ -1969,8 +1923,6 @@ execpline2(Estate state, wordcode pcode, execpline2(state, *state->pc++, how, pipes[0], output, last1); list_pipe = old_list_pipe; cmdpop(); - if (subsh_close != pipes[0]) - zclose(pipes[0]); } } @@ -2710,7 +2662,8 @@ static void execcmd_getargs(LinkList preargs, LinkList args, int expand) /**/ static int execcmd_fork(Estate state, int how, int type, Wordcode varspc, - LinkList *filelistp, char *text, int oautocont) + LinkList *filelistp, char *text, int oautocont, + int close_if_forked) { pid_t pid; int synch[2], flags; @@ -2766,6 +2719,7 @@ execcmd_fork(Estate state, int how, int type, Wordcode varspc, *filelistp = jobtab[thisjob].filelist; entersubsh(flags); close(synch[1]); + zclose(close_if_forked); if (sigtrapped[SIGINT] & ZSIG_IGNORED) holdintr(); @@ -2786,7 +2740,7 @@ execcmd_fork(Estate state, int how, int type, Wordcode varspc, /**/ static void execcmd_exec(Estate state, Execcmd_params eparams, - int input, int output, int how, int last1) + int input, int output, int how, int last1, int close_if_forked) { HashNode hn = NULL; LinkList filelist = NULL; @@ -2863,6 +2817,29 @@ execcmd_exec(Estate state, Execcmd_params eparams, pushnode(args, dupstring("fg")); } + if ((how & Z_ASYNC) || output) { + /* + * If running in the background, or not the last command in a + * pipeline, we don't need any of the rest of this function to + * affect the state in the main shell, so fork immediately. + * + * In other cases we may need to process the command line + * a bit further before we make the decision. + */ + text = getjobtext(state->prog, eparams->beg); + switch (execcmd_fork(state, how, type, varspc, &filelist, + text, oautocont, close_if_forked)) { + case -1: + goto fatal; + case 0: + break; + default: + return; + } + last1 = forked = 1; + } else + text = NULL; + /* Check if it's a builtin needing automatic MAGIC_EQUALS_SUBST * * handling. Things like typeset need this. We can't detect the * * command if it contains some tokens (e.g. x=ex; ${x}port), so this * @@ -2871,7 +2848,7 @@ execcmd_exec(Estate state, Execcmd_params eparams, if ((type == WC_SIMPLE || type == WC_TYPESET) && args) { /* * preargs contains args that have been expanded by prefork. - * Running execcmd_getargs() causes the any argument available + * Running execcmd_getargs() causes any argument available * in args to be exanded where necessary and transferred to * preargs. We call execcmd_getargs() every time we need to * analyse an argument not available in preargs, though there is @@ -2918,8 +2895,11 @@ execcmd_exec(Estate state, Execcmd_params eparams, is_builtin = 1; /* autoload the builtin if necessary */ - if (!(hn = resolvebuiltin(cmdarg, hn))) + if (!(hn = resolvebuiltin(cmdarg, hn))) { + if (forked) + _exit(lastval); return; + } if (type != WC_TYPESET) magic_assign = (hn->flags & BINF_MAGICEQUALS); break; @@ -3097,6 +3077,8 @@ execcmd_exec(Estate state, Execcmd_params eparams, zerr("unknown exec flag -%c", *cmdopt); lastval = 1; errflag |= ERRFLAG_ERROR; + if (forked) + _exit(lastval); return; } } @@ -3146,28 +3128,6 @@ execcmd_exec(Estate state, Execcmd_params eparams, esprefork = (magic_assign || (isset(MAGICEQUALSUBST) && type != WC_TYPESET)) ? PREFORK_TYPESET : 0; - if (how & Z_ASYNC) { - /* - * If running in the background, we don't need any of - * the rest of this functino to affect the state in the - * main shell, so fork immediately. - * - * In other cases we may need to process the command line - * a bit further before we make the decision. - */ - text = getjobtext(state->prog, eparams->beg); - switch (execcmd_fork(state, how, type, varspc, &filelist, - text, oautocont)) { - case -1: - goto fatal; - case 0: - break; - default: - return; - } - forked = 1; - } else - text = NULL; if (args) { if (eparams->htok) @@ -3212,6 +3172,8 @@ execcmd_exec(Estate state, Execcmd_params eparams, zerr("redirection with no command"); lastval = 1; errflag |= ERRFLAG_ERROR; + if (forked) + _exit(lastval); return; } else if (!nullcmd || !*nullcmd || opts[SHNULLCMD]) { if (!args) @@ -3230,6 +3192,8 @@ execcmd_exec(Estate state, Execcmd_params eparams, } } else if ((cflags & BINF_PREFIX) && (cflags & BINF_COMMAND)) { lastval = 0; + if (forked) + _exit(lastval); return; } else { /* @@ -3240,6 +3204,8 @@ execcmd_exec(Estate state, Execcmd_params eparams, if (badcshglob == 1) { zerr("no match"); lastval = 1; + if (forked) + _exit(lastval); return; } cmdoutval = use_cmdoutval ? lastval : 0; @@ -3253,12 +3219,16 @@ execcmd_exec(Estate state, Execcmd_params eparams, fputc('\n', xtrerr); fflush(xtrerr); } + if (forked) + _exit(lastval); return; } } else if (isset(RESTRICTED) && (cflags & BINF_EXEC) && do_exec) { zerrnam("exec", "%s: restricted", (char *) getdata(firstnode(args))); lastval = 1; + if (forked) + _exit(lastval); return; } @@ -3293,6 +3263,8 @@ execcmd_exec(Estate state, Execcmd_params eparams, lastval = 1; if (oautocont >= 0) opts[AUTOCONTINUE] = oautocont; + if (forked) + _exit(lastval); return; } break; @@ -3301,8 +3273,11 @@ execcmd_exec(Estate state, Execcmd_params eparams, is_builtin = 1; /* autoload the builtin if necessary */ - if (!(hn = resolvebuiltin(cmdarg, hn))) + if (!(hn = resolvebuiltin(cmdarg, hn))) { + if (forked) + _exit(lastval); return; + } break; } cflags &= ~BINF_BUILTIN & ~BINF_COMMAND; @@ -3317,6 +3292,8 @@ execcmd_exec(Estate state, Execcmd_params eparams, lastval = 1; if (oautocont >= 0) opts[AUTOCONTINUE] = oautocont; + if (forked) + _exit(lastval); return; } @@ -3394,6 +3371,8 @@ execcmd_exec(Estate state, Execcmd_params eparams, lastval = 1; if (oautocont >= 0) opts[AUTOCONTINUE] = oautocont; + if (forked) + _exit(lastval); return; } } @@ -3422,6 +3401,8 @@ execcmd_exec(Estate state, Execcmd_params eparams, lastval = 1; if (oautocont >= 0) opts[AUTOCONTINUE] = oautocont; + if (forked) + _exit(lastval); return; } @@ -3497,7 +3478,7 @@ execcmd_exec(Estate state, Execcmd_params eparams, (!is_cursh && (last1 != 1 || nsigtrapped || havefiles() || fdtable_flocks)))) { switch (execcmd_fork(state, how, type, varspc, &filelist, - text, oautocont)) { + text, oautocont, close_if_forked)) { case -1: goto fatal; case 0: -- cgit v1.2.3 From 805192311f3426b852026b03e8ca4b421189fcf1 Mon Sep 17 00:00:00 2001 From: dana Date: Sun, 10 Dec 2017 12:17:30 -0600 Subject: 42101 (tweaked): assigning shell status to array was broken --- ChangeLog | 7 +++++++ Src/exec.c | 8 ++++++-- Test/A06assign.ztst | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) (limited to 'Src/exec.c') diff --git a/ChangeLog b/ChangeLog index 325638c30..c05b9e06c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2018-05-11 Peter Stephenson + + * 42101 (tweaked): dana: Src/exec.c, Test/A06assign.ztst: + assigning shell status to array was broken. + + * Src/exec.c (execcmd_exec): + 2018-05-08 Peter Stephenson * 42752: Completion/Unx/Command/_git: better completion diff --git a/Src/exec.c b/Src/exec.c index 08c4ea95a..963b0a5c3 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -2750,7 +2750,7 @@ execcmd_exec(Estate state, Execcmd_params eparams, char *text; int save[10]; int fil, dfil, is_cursh, do_exec = 0, redir_err = 0, i; - int nullexec = 0, magic_assign = 0, forked = 0; + int nullexec = 0, magic_assign = 0, forked = 0, old_lastval; int is_shfunc = 0, is_builtin = 0, is_exec = 0, use_defpath = 0; /* Various flags to the command. */ int cflags = 0, orig_cflags = 0, checked = 0, oautocont = -1; @@ -2775,6 +2775,7 @@ execcmd_exec(Estate state, Execcmd_params eparams, * If assignment but no command get the status from variable * assignment. */ + old_lastval = lastval; if (!args && varspc) lastval = errflag ? errflag : cmdoutval; /* @@ -3209,8 +3210,11 @@ execcmd_exec(Estate state, Execcmd_params eparams, return; } cmdoutval = use_cmdoutval ? lastval : 0; - if (varspc) + if (varspc) { + /* Make sure $? is still correct for assignment */ + lastval = old_lastval; addvars(state, varspc, 0); + } if (errflag) lastval = 1; else diff --git a/Test/A06assign.ztst b/Test/A06assign.ztst index fd2b4177c..f89edb888 100644 --- a/Test/A06assign.ztst +++ b/Test/A06assign.ztst @@ -199,6 +199,41 @@ >a 1 2 3 >a 1 2 3 +# tests of array assignment using lastval ($?) + + true + array=( $? ) + print $array +0:Assign $? to array (true) +>0 + + false + array=( $? ) + print $array +0:Assign $? to array (false) +>1 + + true + typeset array=( $? ) + print $array +0:Assign $? to array with typeset (true) +>0 + + false + typeset array=( $? ) + print $array +0:Assign $? to array with typeset (false) +>1 + + array=( ) + true + array+=( $? ) + false + array+=( $? ) + print $array +0:Append $? to array (true+false) +>0 1 + # tests of var+=scalar s+=foo -- cgit v1.2.3 From 9408c4825b339c43299a39b46811fc6153740e01 Mon Sep 17 00:00:00 2001 From: Peter Stephenson Date: Fri, 15 Jun 2018 15:02:36 +0100 Subject: users/23472: Add $sysparams[procsubstpid] to zsh/system --- ChangeLog | 3 +++ Doc/Zsh/mod_system.yo | 5 +++++ Src/Modules/system.c | 4 ++++ Src/exec.c | 10 ++++++++++ Src/zsh.h | 1 + 5 files changed, 23 insertions(+) (limited to 'Src/exec.c') diff --git a/ChangeLog b/ChangeLog index 0a83ee93f..4c8ff2f41 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,8 @@ 2018-06-15 Peter Stephenson + * users/23472: Doc/Zsh/mod_system.yo, Src/Modules/system.c, + Src/exec.c, Src/zsh.h: Add $sysparams[procsubstpid]. + * 43008: Src/otpions.c: combine suggestion from Sebastain to silence warnings for double setgid/setuid with suggestion from Eitan to put setgid first. diff --git a/Doc/Zsh/mod_system.yo b/Doc/Zsh/mod_system.yo index 7f2009b43..a27bab47f 100644 --- a/Doc/Zsh/mod_system.yo +++ b/Doc/Zsh/mod_system.yo @@ -255,6 +255,11 @@ Returns the process ID of the parent of the current process, even in subshells. Compare tt($PPID), which returns the process ID of the parent of the main shell process. ) +item(tt(procsubstpid))( +Returns the process ID of the last process started for process +substitution, i.e. the tt(LPAR())var(...)tt(RPAR()) expansions. +) enditem() ) enditem() diff --git a/Src/Modules/system.c b/Src/Modules/system.c index 9fd4d2583..7a4f4ee13 100644 --- a/Src/Modules/system.c +++ b/Src/Modules/system.c @@ -772,6 +772,8 @@ fillpmsysparams(Param pm, const char *name) num = (int)getpid(); } else if (!strcmp(name, "ppid")) { num = (int)getppid(); + } else if (!strcmp(name, "procsubstpid")) { + num = (int)procsubstpid; } else { pm->u.str = dupstring(""); pm->node.flags |= PM_UNSET; @@ -805,6 +807,8 @@ scanpmsysparams(UNUSED(HashTable ht), ScanFunc func, int flags) func(&spm.node, flags); fillpmsysparams(&spm, "ppid"); func(&spm.node, flags); + fillpmsysparams(&spm, "procsubstpid"); + func(&spm.node, flags); } static struct mathfunc mftab[] = { diff --git a/Src/exec.c b/Src/exec.c index 963b0a5c3..d44527841 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -174,6 +174,11 @@ mod_export int zleactive; /**/ pid_t cmdoutpid; +/* pid of last process started by <(...), >(...) */ + +/**/ +mod_export pid_t procsubstpid; + /* exit status of process undergoing 'process substitution' */ /**/ @@ -4850,6 +4855,7 @@ getproc(char *cmd, char **eptr) return NULL; if (!out) addproc(pid, NULL, 1, &bgtime); + procsubstpid = pid; return pnam; } closem(FDT_UNUSED, 0); @@ -4887,6 +4893,7 @@ getproc(char *cmd, char **eptr) { addproc(pid, NULL, 1, &bgtime); } + procsubstpid = pid; return pnam; } entersubsh(ESUB_ASYNC|ESUB_PGRP); @@ -4937,6 +4944,7 @@ getpipe(char *cmd, int nullexec) } if (!nullexec) addproc(pid, NULL, 1, &bgtime); + procsubstpid = pid; return pipes[!out]; } entersubsh(ESUB_PGRP); @@ -6172,6 +6180,7 @@ execsave(void) es->cmdoutpid = cmdoutpid; es->cmdoutval = cmdoutval; es->use_cmdoutval = use_cmdoutval; + es->procsubstpid = procsubstpid; es->trap_return = trap_return; es->trap_state = trap_state; es->trapisfunc = trapisfunc; @@ -6207,6 +6216,7 @@ execrestore(void) cmdoutpid = en->cmdoutpid; cmdoutval = en->cmdoutval; use_cmdoutval = en->use_cmdoutval; + procsubstpid = en->procsubstpid; trap_return = en->trap_return; trap_state = en->trap_state; trapisfunc = en->trapisfunc; diff --git a/Src/zsh.h b/Src/zsh.h index 8535d48fb..8e7f20b2c 100644 --- a/Src/zsh.h +++ b/Src/zsh.h @@ -1095,6 +1095,7 @@ struct execstack { pid_t cmdoutpid; int cmdoutval; int use_cmdoutval; + pid_t procsubstpid; int trap_return; int trap_state; int trapisfunc; -- cgit v1.2.3 From 17178db1a4d8700c9e10862d0dd496f92c53f1cd Mon Sep 17 00:00:00 2001 From: Peter Stephenson Date: Mon, 9 Jul 2018 11:06:07 +0100 Subject: users/23531: Error exit/return fixes. Fix regression with trap on left hand side of pipe. Fix forced return from shell structure within nested function. Fix tests exiting too early. Add new test case. --- ChangeLog | 7 +++++++ Src/exec.c | 17 +++++++++++++++++ Test/C03traps.ztst | 17 +++++++++++++++++ 3 files changed, 41 insertions(+) (limited to 'Src/exec.c') diff --git a/ChangeLog b/ChangeLog index 002f608be..babd2321e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2018-07-09 Peter Stephenson + + * users/23531: Src/exec.c, Test/C03traps.ztst: Fix error + exit/return from within shell construct inside nested function, + and in left hand side of pipe (regression due to broken tests); + stop tests from exiting too early and add new test for first fix. + 2018-07-07 Matthew Martin * 43106: Completion/BSD/Command/_ldap: Add ldap completer. diff --git a/Src/exec.c b/Src/exec.c index d44527841..586402070 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -2728,6 +2728,11 @@ execcmd_fork(Estate state, int how, int type, Wordcode varspc, if (sigtrapped[SIGINT] & ZSIG_IGNORED) holdintr(); + /* + * EXIT traps shouldn't be called even if we forked to run + * shell code as this isn't the main shell. + */ + sigtrapped[SIGEXIT] = 0; #ifdef HAVE_NICE /* Check if we should run background jobs at a lower priority. */ if ((how & Z_ASYNC) && isset(BGNICE)) @@ -5792,7 +5797,19 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) undoshfunc: --funcdepth; if (retflag) { + /* + * This function is forced to return. + */ retflag = 0; + /* + * The calling function isn't necessarily forced to return, + * but it should be made sensitive to ERR_EXIT and + * ERR_RETURN as the assumptions we made at the end of + * constructs within this function no longer apply. If + * there are cases where this is not true, they need adding + * to C03traps.ztst. + */ + this_noerrexit = 0; breaks = funcsave->breaks; } freearray(pparams); diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst index f22962550..dce263f94 100644 --- a/Test/C03traps.ztst +++ b/Test/C03traps.ztst @@ -680,6 +680,22 @@ F:Must be tested with a top-level script rather than source or function >Better >In .zshenv + unsetopt errreturn + fn2() { + if true; then + false + fi + } + fn1() { + setopt localoptions errreturn + fn2 + print $? + } + fn1 + print fn1 done +0:ERR_RETURN caused by function returning false from within shell construct +>fn1 done + fn2() { if false; then print Bad @@ -741,6 +757,7 @@ F:Must be tested with a top-level script rather than source or function 0:ERR_EXIT not triggered by status 1 at end of { } >OK + unsetopt err_exit err_return (setopt err_exit for x in y; do false -- cgit v1.2.3 From 3e9fc32fc3d6ac4931f82d666fa4f2f52ac5f65b Mon Sep 17 00:00:00 2001 From: Peter Stephenson Date: Tue, 10 Jul 2018 14:42:02 +0100 Subject: 43156, 43157: Need to allow for extra space in gethere(). The point can now increment twice per iteration. --- ChangeLog | 5 +++++ Src/exec.c | 8 +++++--- 2 files changed, 10 insertions(+), 3 deletions(-) (limited to 'Src/exec.c') diff --git a/ChangeLog b/ChangeLog index babd2321e..6e17484aa 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2018-07-10 Peter Stephenson + + * 43156, 43157: Src/exec.c: need to allow for possible + update of pointer by two in gethere(). + 2018-07-09 Peter Stephenson * users/23531: Src/exec.c, Test/C03traps.ztst: Fix error diff --git a/Src/exec.c b/Src/exec.c index 586402070..615a5086f 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -4418,7 +4418,9 @@ gethere(char **strp, int typ) while ((c = hgetc()) == '\t' && strip) ; for (;;) { - if (bptr == buf + bsiz) { + if (bptr >= buf + bsiz - 2) { + ptrdiff_t toff = t - buf; + ptrdiff_t bptroff = bptr - buf; char *newbuf = realloc(buf, 2 * bsiz); if (!newbuf) { /* out of memory */ @@ -4426,8 +4428,8 @@ gethere(char **strp, int typ) return NULL; } buf = newbuf; - t = buf + bsiz - (bptr - t); - bptr = buf + bsiz; + t = buf + toff; + bptr = buf + bptroff; bsiz *= 2; } if (lexstop || c == '\n') -- cgit v1.2.3