Merge of 22277, 22281 plus tweaks: standardize behaviour of using wait builtin with trapped signals.

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;

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)

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);