22317: exit status from code backgrounded in function

22277, 22281, tweaks: standardize behaviour of wait builtin
with trapped signals

4 files changed, 105 insertions, 40 deletions

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