pass "xerrok" status across the execution call stack to more closely

match what both POSIX and ksh.1 already describe in regards to set
-e/errexit's behavior in determining when to exit from nonzero return
values.

specifically, the truth values tested as operands to `&&' and `||', as
well as the resulting compound expression itself, along with the truth
value resulting from a negated command (i.e. a pipeline prefixed `!'),
should not make the shell exit when -e is in effect.

issue reported by matthieu.
testing matthieu, naddy.
ok miod (earlier version), otto.
man page ok jmc.

9 files changed, 75 insertions, 56 deletions

diff --git a/bin/ksh/c_sh.c b/bin/ksh/c_sh.c
index ae5c3cd8352..e822e850570 100644
--- a/bin/ksh/c_sh.c
+++ b/bin/ksh/c_sh.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: c_sh.c,v 1.38 2008/07/23 16:34:38 jaredy Exp $	*/
+/*	$OpenBSD: c_sh.c,v 1.39 2009/01/29 23:27:26 jaredy Exp $	*/
 
 /*
  * built-in Bourne commands
@@ -709,7 +709,7 @@ c_times(char **wp)
  * time pipeline (really a statement, not a built-in command)
  */
 int
-timex(struct op *t, int f)
+timex(struct op *t, int f, volatile int *xerrok)
 {
 #define TF_NOARGS	BIT(0)
 #define TF_NOREAL	BIT(1)		/* don't report real time */
@@ -734,7 +734,7 @@ timex(struct op *t, int f)
 		 */
 		timerclear(&j_usrtime);
 		timerclear(&j_systime);
-		rv = execute(t->left, f | XTIME);
+		rv = execute(t->left, f | XTIME, xerrok);
 		if (t->left->type == TCOM)
 			tf |= t->left->str[0];
 		gettimeofday(&tv1, NULL);
diff --git a/bin/ksh/eval.c b/bin/ksh/eval.c
index 0af321366bb..e9b88066a22 100644
--- a/bin/ksh/eval.c
+++ b/bin/ksh/eval.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: eval.c,v 1.33 2007/08/02 11:05:54 fgsch Exp $	*/
+/*	$OpenBSD: eval.c,v 1.34 2009/01/29 23:27:26 jaredy Exp $	*/
 
 /*
  * Expansion - quoting, separation, substitution, globbing
@@ -868,7 +868,7 @@ comsub(Expand *xp, char *cp)
 			ksh_dup2(pv[1], 1, false);
 			close(pv[1]);
 		}
-		execute(t, XFORK|XXCOM|XPIPEO);
+		execute(t, XFORK|XXCOM|XPIPEO, NULL);
 		restfd(1, ofd1);
 		startlast();
 		xp->split = 1;	/* waitlast() */
diff --git a/bin/ksh/exec.c b/bin/ksh/exec.c
index 1eb65c02989..1c144fa9c19 100644
--- a/bin/ksh/exec.c
+++ b/bin/ksh/exec.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: exec.c,v 1.48 2007/09/05 19:02:01 otto Exp $	*/
+/*	$OpenBSD: exec.c,v 1.49 2009/01/29 23:27:26 jaredy Exp $	*/
 
 /*
  * execute command tree
@@ -13,7 +13,7 @@
 # define PS4_SUBSTITUTE(s)	substitute((s), 0)
 
 static int	comexec(struct op *, struct tbl *volatile, char **,
-		    int volatile);
+		    int volatile, volatile int *);
 static void	scriptexec(struct op *, char **);
 static int	call_builtin(struct tbl *, char **);
 static int	iosetup(struct ioword *, struct tbl *);
@@ -31,9 +31,9 @@ static void	dbteste_error(Test_env *, int, const char *);
  */
 int
 execute(struct op *volatile t,
-    volatile int flags)		/* if XEXEC don't fork */
+    volatile int flags, volatile int *xerrok)		/* if XEXEC don't fork */
 {
-	int i;
+	int i, dummy = 0;
 	volatile int rv = 0;
 	int pv[2];
 	char ** volatile ap;
@@ -44,6 +44,10 @@ execute(struct op *volatile t,
 	if (t == NULL)
 		return 0;
 
+	/* Caller doesn't care if XERROK should propagate. */
+	if (xerrok == NULL)
+		xerrok = &dummy;
+
 	/* Is this the end of a pipeline?  If so, we want to evaluate the
 	 * command arguments
 	bool eval_done = false;
@@ -53,7 +57,7 @@ execute(struct op *volatile t,
 	}
 	 */
 	if ((flags&XFORK) && !(flags&XEXEC) && t->type != TPIPE)
-		return exchild(t, flags & ~XTIME, -1); /* run in sub-process */
+		return exchild(t, flags & ~XTIME, xerrok, -1); /* run in sub-process */
 
 	newenv(E_EXEC);
 	if (trap)
@@ -111,11 +115,11 @@ execute(struct op *volatile t,
 
 	switch (t->type) {
 	case TCOM:
-		rv = comexec(t, tp, ap, flags);
+		rv = comexec(t, tp, ap, flags, xerrok);
 		break;
 
 	case TPAREN:
-		rv = execute(t->left, flags|XFORK);
+		rv = execute(t->left, flags|XFORK, xerrok);
 		break;
 
 	case TPIPE:
@@ -131,7 +135,7 @@ execute(struct op *volatile t,
 			 *    (: ; cat /etc/termcap) | sleep 1
 			 *  will hang forever).
 			 */
-			exchild(t->left, flags|XPIPEO|XCCLOSE, pv[0]);
+			exchild(t->left, flags|XPIPEO|XCCLOSE, NULL, pv[0]);
 			(void) ksh_dup2(pv[0], 0, false); /* stdin of next */
 			closepipe(pv);
 			flags |= XPIPEI;
@@ -140,17 +144,17 @@ execute(struct op *volatile t,
 		restfd(1, e->savefd[1]); /* stdout of last */
 		e->savefd[1] = 0; /* no need to re-restore this */
 		/* Let exchild() close 0 in parent, after fork, before wait */
-		i = exchild(t, flags|XPCLOSE, 0);
+		i = exchild(t, flags|XPCLOSE, xerrok, 0);
 		if (!(flags&XBGND) && !(flags&XXCOM))
 			rv = i;
 		break;
 
 	case TLIST:
 		while (t->type == TLIST) {
-			execute(t->left, flags & XERROK);
+			execute(t->left, flags & XERROK, NULL);
 			t = t->right;
 		}
-		rv = execute(t, flags & XERROK);
+		rv = execute(t, flags & XERROK, xerrok);
 		break;
 
 	case TCOPROC:
@@ -208,7 +212,7 @@ execute(struct op *volatile t,
 		 */
 		flags &= ~XEXEC;
 		exchild(t->left, flags|XBGND|XFORK|XCOPROC|XCCLOSE,
-		    coproc.readw);
+		    NULL, coproc.readw);
 		break;
 	    }
 
@@ -217,20 +221,24 @@ execute(struct op *volatile t,
 		 * forks again for async...  parent should optimize
 		 * this to "foo &"...
 		 */
-		rv = execute(t->left, (flags&~XEXEC)|XBGND|XFORK);
+		rv = execute(t->left, (flags&~XEXEC)|XBGND|XFORK, xerrok);
 		break;
 
 	case TOR:
 	case TAND:
-		rv = execute(t->left, XERROK);
-		if (t->right != NULL && (rv == 0) == (t->type == TAND))
-			rv = execute(t->right, flags & XERROK);
-		else
-			flags |= XERROK;
+		rv = execute(t->left, XERROK, xerrok);
+		if ((rv == 0) == (t->type == TAND))
+			rv = execute(t->right, XERROK, xerrok);
+		flags |= XERROK;
+		if (xerrok)
+			*xerrok = 1;
 		break;
 
 	case TBANG:
-		rv = !execute(t->right, XERROK);
+		rv = !execute(t->right, XERROK, xerrok);
+		flags |= XERROK;
+		if (xerrok)
+			*xerrok = 1;
 		break;
 
 	case TDBRACKET:
@@ -272,7 +280,7 @@ execute(struct op *volatile t,
 		if (t->type == TFOR) {
 			while (*ap != NULL) {
 				setstr(global(t->str), *ap++, KSH_UNWIND_ERROR);
-				rv = execute(t->left, flags & XERROK);
+				rv = execute(t->left, flags & XERROK, xerrok);
 			}
 		} else { /* TSELECT */
 			for (;;) {
@@ -282,7 +290,7 @@ execute(struct op *volatile t,
 				}
 				is_first = false;
 				setstr(global(t->str), cp, KSH_UNWIND_ERROR);
-				rv = execute(t->left, flags & XERROK);
+				rv = execute(t->left, flags & XERROK, xerrok);
 			}
 		}
 	    }
@@ -305,17 +313,17 @@ execute(struct op *volatile t,
 			}
 		}
 		rv = 0; /* in case of a continue */
-		while ((execute(t->left, XERROK) == 0) == (t->type == TWHILE))
-			rv = execute(t->right, flags & XERROK);
+		while ((execute(t->left, XERROK, NULL) == 0) == (t->type == TWHILE))
+			rv = execute(t->right, flags & XERROK, xerrok);
 		break;
 
 	case TIF:
 	case TELIF:
 		if (t->right == NULL)
 			break;	/* should be error */
-		rv = execute(t->left, XERROK) == 0 ?
-		    execute(t->right->left, flags & XERROK) :
-		    execute(t->right->right, flags & XERROK);
+		rv = execute(t->left, XERROK, NULL) == 0 ?
+		    execute(t->right->left, flags & XERROK, xerrok) :
+		    execute(t->right->right, flags & XERROK, xerrok);
 		break;
 
 	case TCASE:
@@ -327,11 +335,11 @@ execute(struct op *volatile t,
 				goto Found;
 		break;
 	  Found:
-		rv = execute(t->left, flags & XERROK);
+		rv = execute(t->left, flags & XERROK, xerrok);
 		break;
 
 	case TBRACE:
-		rv = execute(t->left, flags & XERROK);
+		rv = execute(t->left, flags & XERROK, xerrok);
 		break;
 
 	case TFUNCT:
@@ -342,7 +350,7 @@ execute(struct op *volatile t,
 		/* Clear XEXEC so nested execute() call doesn't exit
 		 * (allows "ls -l | time grep foo").
 		 */
-		rv = timex(t, flags & ~XEXEC);
+		rv = timex(t, flags & ~XEXEC, xerrok);
 		break;
 
 	case TEXEC:		/* an eval'd TCOM */
@@ -362,7 +370,8 @@ execute(struct op *volatile t,
 	quitenv(NULL);		/* restores IO */
 	if ((flags&XEXEC))
 		unwind(LEXIT);	/* exit child */
-	if (rv != 0 && !(flags & XERROK)) {
+	if (rv != 0 && !(flags & XERROK) &&
+	    (xerrok == NULL || !*xerrok)) {
 		trapsig(SIGERR_);
 		if (Flag(FERREXIT))
 			unwind(LERROR);
@@ -375,7 +384,8 @@ execute(struct op *volatile t,
  */
 
 static int
-comexec(struct op *t, struct tbl *volatile tp, char **ap, volatile int flags)
+comexec(struct op *t, struct tbl *volatile tp, char **ap, volatile int flags,
+    volatile int *xerrok)
 {
 	int i;
 	volatile int rv = 0;
@@ -586,7 +596,7 @@ comexec(struct op *t, struct tbl *volatile tp, char **ap, volatile int flags)
 		i = sigsetjmp(e->jbuf, 0);
 		if (i == 0) {
 			/* seems odd to pass XERROK here, but at&t ksh does */
-			exstat = execute(tp->val.t, flags & XERROK);
+			exstat = execute(tp->val.t, flags & XERROK, xerrok);
 			i = LRETURN;
 		}
 		kshname = old_kshname;
@@ -661,7 +671,7 @@ comexec(struct op *t, struct tbl *volatile tp, char **ap, volatile int flags)
 		texec.left = t;	/* for tprint */
 		texec.str = tp->val.s;
 		texec.args = ap;
-		rv = exchild(&texec, flags, -1);
+		rv = exchild(&texec, flags, xerrok, -1);
 		break;
 	}
   Leave:
diff --git a/bin/ksh/jobs.c b/bin/ksh/jobs.c
index 53f58b5e4d2..46f6c6dd72e 100644
--- a/bin/ksh/jobs.c
+++ b/bin/ksh/jobs.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: jobs.c,v 1.36 2007/09/06 19:57:47 otto Exp $	*/
+/*	$OpenBSD: jobs.c,v 1.37 2009/01/29 23:27:26 jaredy Exp $	*/
 
 /*
  * Process and job control
@@ -331,7 +331,7 @@ j_change(void)
 
 /* execute tree in child subprocess */
 int
-exchild(struct op *t, int flags,
+exchild(struct op *t, int flags, volatile int *xerrok,
     int close_fd)	/* used if XPCLOSE or XCCLOSE */
 {
 	static Proc	*last_proc;	/* for pipelines */
@@ -348,7 +348,7 @@ exchild(struct op *t, int flags,
 		/* Clear XFORK|XPCLOSE|XCCLOSE|XCOPROC|XPIPEO|XPIPEI|XXCOM|XBGND
 		 * (also done in another execute() below)
 		 */
-		return execute(t, flags & (XEXEC | XERROK));
+		return execute(t, flags & (XEXEC | XERROK), xerrok);
 
 	/* no SIGCHLD's while messing with job and process lists */
 	sigprocmask(SIG_BLOCK, &sm_sigchld, &omask);
@@ -478,7 +478,7 @@ exchild(struct op *t, int flags,
 		Flag(FTALKING) = 0;
 		tty_close();
 		cleartraps();
-		execute(t, (flags & XERROK) | XEXEC); /* no return */
+		execute(t, (flags & XERROK) | XEXEC, NULL); /* no return */
 		internal_errorf(0, "exchild: execute() returned");
 		unwind(LLEAVE);
 		/* NOTREACHED */
diff --git a/bin/ksh/ksh.1 b/bin/ksh/ksh.1
index c979fbe5fa8..a25235e9e19 100644
--- a/bin/ksh/ksh.1
+++ b/bin/ksh/ksh.1
@@ -1,8 +1,8 @@
-.\"	$OpenBSD: ksh.1,v 1.122 2008/05/17 23:31:52 sobrado Exp $
+.\"	$OpenBSD: ksh.1,v 1.123 2009/01/29 23:27:26 jaredy Exp $
 .\"
 .\"	Public Domain
 .\"
-.Dd $Mdocdate: May 17 2008 $
+.Dd $Mdocdate: January 29 2009 $
 .Dt KSH 1
 .Os
 .Sh NAME
@@ -3567,8 +3567,9 @@ explicitly tested by a shell construct such as
 .Ic until ,
 .Ic while ,
 .Ic && ,
+.Ic || ,
 or
-.Ic ||
+.Ic !\&
 statements.
 .It Fl f \*(Ba Ic noglob
 Do not expand file name patterns.
diff --git a/bin/ksh/main.c b/bin/ksh/main.c
index abacbfdd4fc..b33f7f0c978 100644
--- a/bin/ksh/main.c
+++ b/bin/ksh/main.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: main.c,v 1.44 2008/07/05 07:25:18 djm Exp $	*/
+/*	$OpenBSD: main.c,v 1.45 2009/01/29 23:27:26 jaredy Exp $	*/
 
 /*
  * startup, main loop, environments and error handling
@@ -566,7 +566,7 @@ shell(Source *volatile s, volatile int toplevel)
 		}
 
 		if (t && (!Flag(FNOEXEC) || (s->flags & SF_TTY)))
-			exstat = execute(t, 0);
+			exstat = execute(t, 0, NULL);
 
 		if (t != NULL && t->type != TEOF && interactive && really_exit)
 			really_exit = 0;
diff --git a/bin/ksh/proto.h b/bin/ksh/proto.h
index 95c0c12531a..07985e79577 100644
--- a/bin/ksh/proto.h
+++ b/bin/ksh/proto.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: proto.h,v 1.31 2009/01/17 22:06:44 millert Exp $	*/
+/*	$OpenBSD: proto.h,v 1.32 2009/01/29 23:27:26 jaredy Exp $	*/
 
 /*
  * prototypes for PD-KSH
@@ -44,7 +44,7 @@ int	c_set(char **);
 int	c_unset(char **);
 int	c_ulimit(char **);
 int	c_times(char **);
-int	timex(struct op *, int);
+int	timex(struct op *, int, volatile int *);
 void	timex_hook(struct op *, char ** volatile *);
 int	c_exec(char **);
 int	c_builtin(char **);
@@ -65,7 +65,7 @@ char	*debunk(char *, const char *, size_t);
 void	expand(char *, XPtrV *, int);
 int	glob_str(char *, XPtrV *, int);
 /* exec.c */
-int	execute(struct op * volatile, volatile int);
+int	execute(struct op * volatile, volatile int, volatile int *);
 int	shcomexec(char **);
 struct tbl * findfunc(const char *, unsigned int, int);
 int	define(const char *, struct op *);
@@ -134,7 +134,7 @@ struct temp *maketemp(Area *, Temp_type, struct temp **);
 void	j_init(int);
 void	j_exit(void);
 void	j_change(void);
-int	exchild(struct op *, int, int);
+int	exchild(struct op *, int, volatile int *, int);
 void	startlast(void);
 int	waitlast(void);
 int	waitfor(const char *, int *);
diff --git a/bin/ksh/sh.1 b/bin/ksh/sh.1
index f87f0b22b8f..95ab08d9d09 100644
--- a/bin/ksh/sh.1
+++ b/bin/ksh/sh.1
@@ -1,8 +1,8 @@
-.\"	$OpenBSD: sh.1,v 1.76 2008/03/21 12:51:19 millert Exp $
+.\"	$OpenBSD: sh.1,v 1.77 2009/01/29 23:27:26 jaredy Exp $
 .\"
 .\"	Public Domain
 .\"
-.Dd $Mdocdate: March 21 2008 $
+.Dd $Mdocdate: January 29 2009 $
 .Dt SH 1
 .Os
 .Sh NAME
@@ -2768,8 +2768,9 @@ explicitly tested by a shell construct such as
 .Ic until ,
 .Ic while ,
 .Ic && ,
+.Ic || ,
 or
-.Ic ||
+.Ic !\&
 statements.
 .It Fl f \*(Ba Ic noglob
 Do not expand file name patterns.
diff --git a/regress/bin/ksh/seterror.sh b/regress/bin/ksh/seterror.sh
index c7001e254c5..f3753249ca2 100644
--- a/regress/bin/ksh/seterror.sh
+++ b/regress/bin/ksh/seterror.sh
@@ -1,13 +1,20 @@
 #! /bin/sh
-# $OpenBSD: seterror.sh,v 1.1 2003/02/09 18:52:49 espie Exp $
+# $OpenBSD: seterror.sh,v 1.2 2009/01/29 23:27:26 jaredy Exp $
 
 # set -e is supposed to abort the script for errors that are not caught
 # otherwise.
 
 set -e
 
+if true; then false && false; fi
+if true; then if true; then false && false; fi; fi
+
 for i in 1 2 3
 do
-	false && true
+	true && false
+	false || false
 done
+
+! true | false
+
 true