Subject: bin/3662: /bin/sh knows too much about wait()
To: None <gnats-bugs@gnats.netbsd.org>
From: David Holland <dholland@bordeaux.eecs.harvard.edu>
List: netbsd-bugs
Date: 05/22/1997 18:27:49
>Number:         3662
>Category:       bin
>Synopsis:       /bin/sh knows too much about wait()
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bin-bug-people (Utility Bug People)
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Thu May 22 15:35:01 1997
>Last-Modified:
>Originator:     dholland@eecs.harvard.edu (David Holland)
>Organization:
VINO OS project 
>Release:        NetBSD-1.2 and -current of 5/22/1997
>Environment:

	Discovered under VINO, actually.

System: NetBSD bordeaux 1.2C NetBSD 1.2C (BORDEAUX.PROF) #28: Sat Mar 1 16:28:54 EST 1997 abrown@bordeaux:/usr/src/sys/arch/i386/compile/BORDEAUX.PROF i386


>Description:

	/bin/sh does not use the WIFEXITED, WIFSIGNALED, etc. macros from
	<sys/wait.h>, but instead assumes it knows the exact bit patterns 
	generated by the kernel.

	This (1) is not correct, and (2) breaks if the bit patterns change,
	such as for support of more than 256 signals, exit codes larger than
	256, or other matters.

>How-To-Repeat:
	N/A.

>Fix:

This patch is against src/bin/sh/jobs.c. 
Please let me know if you find any mistakes in it.

Index: jobs.c
===================================================================
RCS file: /home/vino/repo/src/utils/bsd/bin/sh/jobs.c,v
retrieving revision 1.1.1.1
diff -u -3 -r1.1.1.1 jobs.c
--- jobs.c	1997/04/10 18:32:45	1.1.1.1
+++ jobs.c	1997/05/22 22:07:22
@@ -241,7 +241,7 @@
 	INTOFF;
 	killpg(jp->ps[0].pid, SIGCONT);
 	for (ps = jp->ps, i = jp->nprocs ; --i >= 0 ; ps++) {
-		if ((ps->status & 0377) == 0177) {
+		if (WIFSTOPPED(ps->status)) {
 			ps->status = -1;
 			jp->state = 0;
 		}
@@ -304,19 +304,21 @@
 			s[0] = '\0';
 			if (ps->status == -1) {
 				/* don't print anything */
-			} else if ((ps->status & 0xFF) == 0) {
-				fmtstr(s, 64, "Exit %d", ps->status >> 8);
+			} else if (WIFEXITED(ps->status)) {
+				fmtstr(s, 64, "Exit %d", 
+				       WEXITSTATUS(ps->status));
 			} else {
-				i = ps->status;
 #if JOBS
-				if ((i & 0xFF) == 0177)
-					i >>= 8;
+				if (WIFSTOPPED(ps->status)) 
+					i = WSTOPSIG(ps->status);
+				else /* WIFSIGNALED(ps->status) */
 #endif
+					i = WTERMSIG(ps->status);
 				if ((i & 0x7F) < NSIG && sys_siglist[i & 0x7F])
 					scopy(sys_siglist[i & 0x7F], s);
 				else
 					fmtstr(s, 64, "Signal %d", i & 0x7F);
-				if (i & 0x80)
+				if (WCOREDUMP(ps->status))
 					strcat(s, " (core dumped)");
 			}
 			out1str(s);
@@ -372,7 +374,7 @@
 	char **argv;
 {
 	struct job *job;
-	int status;
+	int status, retval;
 	struct job *jp;
 
 	if (argc > 1) {
@@ -384,17 +386,19 @@
 		if (job != NULL) {
 			if (job->state) {
 				status = job->ps[job->nprocs - 1].status;
-				if ((status & 0xFF) == 0)
-					status = status >> 8 & 0xFF;
+				if (WIFEXITED(status))
+					retval = WEXITSTATUS(status);
 #if JOBS
-				else if ((status & 0xFF) == 0177)
-					status = (status >> 8 & 0x7F) + 128;
+				else if (WIFSTOPPED(status))
+					retval = WSTOPSIG(status) + 128;
 #endif
-				else
-					status = (status & 0x7F) + 128;
+				else {
+					/* XXX: limits number of signals */
+					retval = WTERMSIG(status) + 128;
+				}
 				if (! iflag)
 					freejob(job);
-				return status;
+				return retval;
 			}
 		} else {
 			for (jp = jobtab ; ; jp++) {
@@ -713,18 +717,18 @@
 #endif
 	status = jp->ps[jp->nprocs - 1].status;
 	/* convert to 8 bits */
-	if ((status & 0xFF) == 0)
-		st = status >> 8 & 0xFF;
+	if (WIFEXITED(status))
+		st = WEXITSTATUS(status);
 #if JOBS
-	else if ((status & 0xFF) == 0177)
-		st = (status >> 8 & 0x7F) + 128;
+	else if (WIFSTOPPED(status))
+		st = WSTOPSIG(status) + 128;
 #endif
 	else
-		st = (status & 0x7F) + 128;
+		st = WTERMSIG(status) + 128;
 	if (! JOBS || jp->state == JOBDONE)
 		freejob(jp);
 	CLEAR_PENDING_INT;
-	if ((status & 0x7F) == SIGINT)
+	if (WIFSIGNALED(status) && WTERMSIG(status) == SIGINT)
 		kill(getpid(), SIGINT);
 	INTON;
 	return st;
@@ -749,6 +753,7 @@
 	int done;
 	int stopped;
 	int core;
+	int sig;
 
 	TRACE(("dowait(%d) called\n", block));
 	do {
@@ -767,13 +772,13 @@
 				if (sp->pid == -1)
 					continue;
 				if (sp->pid == pid) {
-					TRACE(("Changin status of proc %d from 0x%x to 0x%x\n", pid, sp->status, status));
+					TRACE(("Changing status of proc %d from 0x%x to 0x%x\n", pid, sp->status, status));
 					sp->status = status;
 					thisjob = jp;
 				}
 				if (sp->status == -1)
 					stopped = 0;
-				else if ((sp->status & 0377) == 0177)
+				else if (WIFSTOPPED(sp->status))
 					done = 0;
 			}
 			if (stopped) {		/* stopped or done */
@@ -791,29 +796,32 @@
 	}
 	INTON;
 	if (! rootshell || ! iflag || (job && thisjob == job)) {
+		core = WCOREDUMP(status);
 #if JOBS
-		if ((status & 0xFF) == 0177)
-			status >>= 8;
+		if (WIFSTOPPED(status)) sig = WSTOPSIG(status);
+		else
 #endif
-		core = status & 0x80;
-		status &= 0x7F;
-		if (status != 0 && status != SIGINT && status != SIGPIPE) {
+		if (WIFEXITED(status)) sig = 0;
+		else sig = WTERMSIG(status);
+
+		if (sig != 0 && sig != SIGINT && sig != SIGPIPE) {
 			if (thisjob != job)
 				outfmt(out2, "%d: ", pid);
 #if JOBS
-			if (status == SIGTSTP && rootshell && iflag)
+			if (sig == SIGTSTP && rootshell && iflag)
 				outfmt(out2, "%%%d ", job - jobtab + 1);
 #endif
-			if (status < NSIG && sys_siglist[status])
-				out2str(sys_siglist[status]);
+			if (sig < NSIG && sys_siglist[sig])
+				out2str(sys_siglist[sig]);
 			else
-				outfmt(out2, "Signal %d", status);
+				outfmt(out2, "Signal %d", sig);
 			if (core)
 				out2str(" - core dumped");
 			out2c('\n');
 			flushout(&errout);
 		} else {
-			TRACE(("Not printing status: status=%d\n", status));
+			TRACE(("Not printing status: status=%d, sig=%d\n", 
+			       status, sig));
 		}
 	} else {
 		TRACE(("Not printing status, rootshell=%d, job=0x%x\n", rootshell, job));

>Audit-Trail:
>Unformatted: