From 02ea8ff2a4f8d592ef57493188c2648b7b8c5772 Mon Sep 17 00:00:00 2001 From: Paul Ackersviller Date: Sun, 11 Nov 2007 21:20:37 +0000 Subject: Merge of 22277, 22281 plus tweaks: standardize behaviour of using wait builtin with trapped signals. --- Src/exec.c | 4 +-- Src/jobs.c | 92 +++++++++++++++++++++++++++++++++++++++++++---------------- Src/signals.c | 59 +++++++++++++++++++++++++------------- 3 files changed, 109 insertions(+), 46 deletions(-) (limited to 'Src') diff --git a/Src/exec.c b/Src/exec.c index c69730f88..61d21be90 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -2896,7 +2896,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; } @@ -3025,7 +3025,7 @@ getoutputfile(char *cmd) close(fd); os = jobtab[thisjob].stat; - waitforpid(pid); + waitforpid(pid, 0); cmdoutval = 0; jobtab[thisjob].stat = os; return nam; diff --git a/Src/jobs.c b/Src/jobs.c index ec11c747b..2da44a4e8 100644 --- a/Src/jobs.c +++ b/Src/jobs.c @@ -940,10 +940,10 @@ printjob(Job jn, int lng, int synch) if ((lng & 4) || (interact && job == thisjob && jn->pwd && strcmp(jn->pwd, pwd))) { - fprintf(shout, "(pwd %s: ", (lng & 4) ? "" : "now"); - fprintdir(((lng & 4) && jn->pwd) ? jn->pwd : pwd, shout); - fprintf(shout, ")\n"); - fflush(shout); + fprintf(fout, "(pwd %s: ", (lng & 4) ? "" : "now"); + fprintdir(((lng & 4) && jn->pwd) ? jn->pwd : pwd, fout); + fprintf(fout, ")\n"); + fflush(fout); } /* delete job if done */ @@ -1111,11 +1111,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(); @@ -1128,18 +1133,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; @@ -1153,7 +1170,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 @@ -1176,6 +1199,8 @@ zwaitjob(int job, int sig) } child_unblock(); restore_queue_signals(q); + + return 0; } /* wait for running job to finish */ @@ -1687,9 +1712,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; } } @@ -1707,11 +1732,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; } @@ -1780,11 +1813,12 @@ bin_fg(char *name, char **argv, Options ops, int func) printjob(jobtab + job, (stopped) ? -1 : lng, 1); if (func != BIN_BG) { /* fg or wait */ if (jobtab[job].pwd && strcmp(jobtab[job].pwd, pwd)) { - fprintf(shout, "(pwd : "); - fprintdir(jobtab[job].pwd, shout); - fprintf(shout, ")\n"); + FILE *fout = (func == BIN_JOBS) ? stdout : shout; + fprintf(fout, "(pwd : "); + fprintdir(jobtab[job].pwd, fout); + fprintf(fout, ")\n"); + fflush(fout); } - fflush(shout); if (func != BIN_WAIT) { /* fg */ thisjob = job; if ((jobtab[job].stat & STAT_SUPERJOB) && @@ -1803,8 +1837,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 a6c60b604..7885210cf 100644 --- a/Src/signals.c +++ b/Src/signals.c @@ -338,9 +338,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; @@ -350,15 +379,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(); @@ -370,15 +391,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); @@ -388,7 +402,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 * @@ -400,6 +414,10 @@ signal_suspend(int sig, int sig2) return ret; } +/* last signal we handled: race prone, or what? */ +/**/ +int last_signal; + /* the signal handler */ /**/ @@ -413,6 +431,7 @@ zhandler(int sig) signal_jmp_buf jump_to; #endif + last_signal = sig; signal_process(sig); sigfillset(&newmask); -- cgit 1.4.1