diff options
| -rw-r--r-- | ChangeLog | 10 | ||||
| -rw-r--r-- | Src/builtin.c | 5 | ||||
| -rw-r--r-- | Src/exec.c | 6 | ||||
| -rw-r--r-- | Src/jobs.c | 75 | ||||
| -rw-r--r-- | Src/signals.c | 59 |
5 files changed, 115 insertions, 40 deletions
diff --git a/ChangeLog b/ChangeLog index 977441687..19e43c838 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2006-03-02 Peter Stephenson <p.w.stephenson@ntlworld.com> + + * 22317: Src/builtins.c, Src/exec.c: exiting the shell from + code forked from within a function doesn't maintain the + exit status. + + * 22277, 22281 plus tweaks: Src/exec.c, Src/jobs.c, Src/signals.c, + Test/C03traps.ztst: standardize behaviour of using wait builtin + with trapped signals. + 2006-03-02 Peter Stephenson <pws@csr.com> * unposted, but see 22307: configure.ac: turn all diff --git a/Src/builtin.c b/Src/builtin.c index 1b7e1935e..d51149ec6 100644 --- a/Src/builtin.c +++ b/Src/builtin.c @@ -4119,12 +4119,15 @@ bin_break(char *name, char **argv, UNUSED(Options ops), int func) } /*FALLTHROUGH*/ case BIN_EXIT: - if (locallevel) { + if (locallevel > forklevel) { /* * We don't exit directly from functions to allow tidying * up, in particular EXIT traps. We still need to perform * the usual interactive tests to see if we can exit at * all, however. + * + * If we are forked, we exit the shell at the function depth + * at which we became a subshell, hence the comparison. */ if (stopmsg || (zexit(0,2), !stopmsg)) { retflag = 1; diff --git a/Src/exec.c b/Src/exec.c index 58fc476bc..6c68c5c7d 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -3037,7 +3037,7 @@ getoutput(char *cmd, int qt) zclose(pipes[1]); retval = readoutput(pipes[0], qt); fdtable[pipes[0]] = FDT_UNUSED; - waitforpid(pid); /* unblocks */ + waitforpid(pid, 0); /* unblocks */ lastval = cmdoutval; return retval; } @@ -3190,7 +3190,7 @@ getoutputfile(char *cmd) close(fd); os = jobtab[thisjob].stat; - waitforpid(pid); + waitforpid(pid, 0); cmdoutval = 0; jobtab[thisjob].stat = os; return nam; @@ -3852,7 +3852,7 @@ doshfunc(char *name, Eprog prog, LinkList doshargs, int flags, int noreturnval) popheap(); if (exit_pending) { - if (locallevel) { + if (locallevel > forklevel) { /* Still functions to return: force them to do so. */ retflag = 1; breaks = loops; diff --git a/Src/jobs.c b/Src/jobs.c index 1e8ff8789..4f89d0f53 100644 --- a/Src/jobs.c +++ b/Src/jobs.c @@ -1116,11 +1116,16 @@ havefiles(void) } -/* wait for a particular process */ +/* + * Wait for a particular process. + * wait_cmd indicates this is from the interactive wait command, + * in which case the behaviour is a little different: the command + * itself can be interrupted by a trapped signal. + */ /**/ -void -waitforpid(pid_t pid) +int +waitforpid(pid_t pid, int wait_cmd) { int first = 1, q = queue_signal_level(); @@ -1133,18 +1138,30 @@ waitforpid(pid_t pid) else kill(pid, SIGCONT); - signal_suspend(SIGCHLD, SIGINT); + last_signal = -1; + signal_suspend(SIGCHLD, wait_cmd); + if (last_signal != SIGCHLD && wait_cmd) { + /* wait command interrupted, but no error: return */ + restore_queue_signals(q); + return 128 + last_signal; + } child_block(); } child_unblock(); restore_queue_signals(q); + + return 0; } -/* wait for a job to finish */ +/* + * Wait for a job to finish. + * wait_cmd indicates this is from the wait builtin; see + * wait_cmd in waitforpid(). + */ /**/ -static void -zwaitjob(int job, int sig) +static int +zwaitjob(int job, int wait_cmd) { int q = queue_signal_level(); Job jn = jobtab + job; @@ -1158,7 +1175,13 @@ zwaitjob(int job, int sig) while (!errflag && jn->stat && !(jn->stat & STAT_DONE) && !(interact && (jn->stat & STAT_STOPPED))) { - signal_suspend(SIGCHLD, sig); + signal_suspend(SIGCHLD, wait_cmd); + if (last_signal != SIGCHLD && wait_cmd) + { + /* builtin wait interrupted by trapped signal */ + restore_queue_signals(q); + return 128 + last_signal; + } /* Commenting this out makes ^C-ing a job started by a function stop the whole function again. But I guess it will stop something else from working properly, we have to find out @@ -1181,6 +1204,8 @@ zwaitjob(int job, int sig) } child_unblock(); restore_queue_signals(q); + + return 0; } /* wait for running job to finish */ @@ -1692,9 +1717,9 @@ bin_fg(char *name, char **argv, Options ops, int func) } else { /* Must be BIN_WAIT, so wait for all jobs */ for (job = 0; job <= maxjob; job++) if (job != thisjob && jobtab[job].stat) - zwaitjob(job, SIGINT); + retval = zwaitjob(job, 1); unqueue_signals(); - return 0; + return retval; } } @@ -1712,11 +1737,19 @@ bin_fg(char *name, char **argv, Options ops, int func) Job j; Process p; - if (findproc(pid, &j, &p, 0)) - waitforpid(pid); - else + if (findproc(pid, &j, &p, 0)) { + /* + * returns 0 for normal exit, else signal+128 + * in which case we should return that status. + */ + retval = waitforpid(pid, 1); + if (!retval) + retval = lastval2; + } else { zwarnnam(name, "pid %d is not a child of this shell", 0, pid); - retval = lastval2; + /* presumably lastval2 doesn't tell us a heck of a lot? */ + retval = 1; + } thisjob = ocj; continue; } @@ -1796,8 +1829,18 @@ bin_fg(char *name, char **argv, Options ops, int func) killjb(jobtab + job, SIGCONT); } if (func == BIN_WAIT) - zwaitjob(job, SIGINT); - if (func != BIN_BG) { + { + retval = zwaitjob(job, 1); + if (!retval) + retval = lastval2; + } + else if (func != BIN_BG) { + /* + * HERE: there used not to be an "else" above. How + * could it be right to wait for the foreground job + * when we've just been told to wait for another + * job (and done it)? + */ waitjobs(); retval = lastval2; } else if (ofunc == BIN_DISOWN) diff --git a/Src/signals.c b/Src/signals.c index 0b508f869..3abeab647 100644 --- a/Src/signals.c +++ b/Src/signals.c @@ -347,9 +347,38 @@ static int suspend_longjmp = 0; static signal_jmp_buf suspend_jmp_buf; #endif +#if defined(POSIX_SIGNALS) || defined(BSD_SIGNALS) +static void +signal_suspend_setup(sigset_t *set, int sig, int wait_cmd) +{ + if (isset(TRAPSASYNC)) { + sigemptyset(set); + } else { + sigfillset(set); + sigdelset(set, sig); +#ifdef POSIX_SIGNALS + sigdelset(set, SIGHUP); /* still don't know why we add this? */ +#endif + if (wait_cmd) + { + /* + * Using "wait" builtin. We should allow SIGINT and + * execute traps delivered to the shell. + */ + int sigtr; + sigdelset(set, SIGINT); + for (sigtr = 1; sigtr <= NSIG; sigtr++) { + if (sigtrapped[sigtr] & ~ZSIG_IGNORED) + sigdelset(set, sigtr); + } + } + } +} +#endif + /**/ int -signal_suspend(int sig, int sig2) +signal_suspend(int sig, int wait_cmd) { int ret; @@ -359,15 +388,7 @@ signal_suspend(int sig, int sig2) sigset_t oset; #endif /* BROKEN_POSIX_SIGSUSPEND */ - if (isset(TRAPSASYNC)) { - sigemptyset(&set); - } else { - sigfillset(&set); - sigdelset(&set, sig); - sigdelset(&set, SIGHUP); /* still don't know why we add this? */ - if (sig2) - sigdelset(&set, sig2); - } + signal_suspend_setup(&set, sig, wait_cmd); #ifdef BROKEN_POSIX_SIGSUSPEND sigprocmask(SIG_SETMASK, &set, &oset); pause(); @@ -379,15 +400,8 @@ signal_suspend(int sig, int sig2) # ifdef BSD_SIGNALS sigset_t set; - if (isset(TRAPSASYNC)) { - sigemptyset(&set); - } else { - sigfillset(&set); - sigdelset(&set, sig); - if (sig2) - sigdelset(&set, sig2); - ret = sigpause(set); - } + signal_suspend_setup(&set, sig, wait_cmd); + ret = sigpause(set); # else # ifdef SYSV_SIGNALS ret = sigpause(sig); @@ -397,7 +411,7 @@ signal_suspend(int sig, int sig2) * between the child_unblock() and pause() */ if (signal_setjmp(suspend_jmp_buf) == 0) { suspend_longjmp = 1; /* we want to signal_longjmp after catching signal */ - child_unblock(); /* do we need to unblock sig2 as well? */ + child_unblock(); /* do we need to do wait_cmd stuff as well? */ ret = pause(); } suspend_longjmp = 0; /* turn off using signal_longjmp since we are past * @@ -409,6 +423,10 @@ signal_suspend(int sig, int sig2) return ret; } +/* last signal we handled: race prone, or what? */ +/**/ +int last_signal; + /* the signal handler */ /**/ @@ -422,6 +440,7 @@ zhandler(int sig) signal_jmp_buf jump_to; #endif + last_signal = sig; signal_process(sig); sigfillset(&newmask); |