Sourcing an empty files does not reset exit status

Sourcing an empty files does not reset exit status

[Gioele Barabucci]

Hello,

a bug has been filed in the Debian BTS about dash not resetting the exit 
status after sourcing an empty file with the dot command. [1]

The following test echoes "OK" with bash and "fail" with dash

     #!/bin/sh

     echo > ./empty
     false

     . ./empty && echo "OK" || echo "fail"

A similar bug in dash has been discussed and addressed in 2011 [2], but 
it looks like the solution has been only partial.

The version of dash I tested is the current git master branch, commit 
2e58422.

[1] https://bugs.debian.org/777262
[2] http://article.gmane.org/gmane.comp.shells.dash/531

Regards,

--
Gioele Barabucci <gioele@svario.it>

Re: Sourcing an empty files does not reset exit status

[Harald van Dijk]

[-- Attachment #1: Type: text/plain, Size: 1991 bytes --]

On 17/11/2015 03:18, Gioele Barabucci wrote:
> Hello,
>
> a bug has been filed in the Debian BTS about dash not resetting the exit
> status after sourcing an empty file with the dot command. [1]
>
> The following test echoes "OK" with bash and "fail" with dash
>
>      #!/bin/sh
>
>      echo > ./empty
>      false
>
>      . ./empty && echo "OK" || echo "fail"
 >
> A similar bug in dash has been discussed and addressed in 2011 [2], but
> it looks like the solution has been only partial.
>
> The version of dash I tested is the current git master branch, commit
> 2e58422.
>
> [1] https://bugs.debian.org/777262
> [2] http://article.gmane.org/gmane.comp.shells.dash/531

The bug described there was about empty files. While the fix has been 
applied and does make dash handle empty files properly, your test 
doesn't use an empty file, it uses a file containing a single blank 
line. Unfortunately, the single blank line gets parsed by dash as a null 
command, null commands don't (and shouldn't) reset the exit status, and 
the fix you link to doesn't handle this because it sees a command has 
been executed and saves the exit status after executing that command as 
the exit status to be used by ".".

I think the easiest way to fix this is to prevent null commands from 
affecting status in cmdloop, as attached.

An alternative could be to change the outer if condition to exclude
n == NULL, but I didn't do that because the change of job_warning and 
clearing of numeof make sense to me even for null commands. Besides, 
when debug tracing is enabled, null commands have a visible effect that 
should remain.

Note that this fixes the problem with . but the same problem can be 
present in other locations. For example,

     false
     eval "
     " && echo OK || echo Fail

used to print Fail, and needed the same modification in the evalstring 
function to make that print OK (included in the attached patch). There 
may be other similar bugs lurking.

Cheers,
Harald van Dijk

[-- Attachment #2: dash-dot-status.patch --]
[-- Type: text/plain, Size: 676 bytes --]

diff --git a/src/eval.c b/src/eval.c
index 071fb1b..46689ef 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -172,7 +172,7 @@ evalstring(char *s, int flags)
 	status = 0;
 	while ((n = parsecmd(0)) != NEOF) {
 		evaltree(n, flags & ~(parser_eof() ? 0 : EV_EXIT));
-		status = exitstatus;
+		if (n) status = exitstatus;
 		popstackmark(&smark);
 		if (evalskip)
 			break;
diff --git a/src/main.c b/src/main.c
index bedb663..1b301fa 100644
--- a/src/main.c
+++ b/src/main.c
@@ -228,7 +228,7 @@ cmdloop(int top)
 			job_warning = (job_warning == 2) ? 1 : 0;
 			numeof = 0;
 			evaltree(n, 0);
-			status = exitstatus;
+			if (n) status = exitstatus;
 		}
 		popstackmark(&smark);
 

[PATCH] eval: Return status in eval functions

[Herbert Xu]

On Sat, Dec 05, 2015 at 04:40:59PM +0100, Harald van Dijk wrote:
> 
> used to print Fail, and needed the same modification in the evalstring 
> function to make that print OK (included in the attached patch). There 
> may be other similar bugs lurking.

Not exactly similar but there are a bunch of bugs caused by setting
exitstatus too early (i.e., before $? has been expanded).

This patch should fix those problems plus the one that you fixed.

Thanks!

---8<---
Harald van Dijk sent a ptch to fix a bug where an empty parse tree
may not set the exit status correctly in some contexts.

This in turn uncovered other bugs where the exit status is clobbered
in places such case case statements and loops.

This patch attempts to address all of them by making the eval
functions return the current status.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/src/eval.c b/src/eval.c
index 071fb1b..6f47562 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -81,16 +81,16 @@ int savestatus = -1;		/* exit status of last command outside traps */
 STATIC
 #endif
 void evaltreenr(union node *, int) __attribute__ ((__noreturn__));
-STATIC void evalloop(union node *, int);
-STATIC void evalfor(union node *, int);
-STATIC void evalcase(union node *, int);
-STATIC void evalsubshell(union node *, int);
+STATIC int evalloop(union node *, int);
+STATIC int evalfor(union node *, int);
+STATIC int evalcase(union node *, int);
+STATIC int evalsubshell(union node *, int);
 STATIC void expredir(union node *);
-STATIC void evalpipe(union node *, int);
+STATIC int evalpipe(union node *, int);
 #ifdef notyet
-STATIC void evalcommand(union node *, int, struct backcmd *);
+STATIC int evalcommand(union node *, int, struct backcmd *);
 #else
-STATIC void evalcommand(union node *, int);
+STATIC int evalcommand(union node *, int);
 #endif
 STATIC int evalbltin(const struct builtincmd *, int, char **, int);
 STATIC int evalfun(struct funcnode *, int, char **, int);
@@ -171,8 +171,7 @@ evalstring(char *s, int flags)
 
 	status = 0;
 	while ((n = parsecmd(0)) != NEOF) {
-		evaltree(n, flags & ~(parser_eof() ? 0 : EV_EXIT));
-		status = exitstatus;
+		status = evaltree(n, flags & ~(parser_eof() ? 0 : EV_EXIT));
 		popstackmark(&smark);
 		if (evalskip)
 			break;
@@ -191,13 +190,13 @@ evalstring(char *s, int flags)
  * exitstatus.
  */
 
-void
+int
 evaltree(union node *n, int flags)
 {
 	int checkexit = 0;
-	void (*evalfn)(union node *, int);
+	int (*evalfn)(union node *, int);
 	unsigned isor;
-	int status;
+	int status = 0;
 	if (n == NULL) {
 		TRACE(("evaltree(NULL) called\n"));
 		goto out;
@@ -220,8 +219,7 @@ evaltree(union node *n, int flags)
 		break;
 #endif
 	case NNOT:
-		evaltree(n->nnot.com, EV_TESTED);
-		status = !exitstatus;
+		status = !evaltree(n->nnot.com, EV_TESTED);
 		goto setstatus;
 	case NREDIR:
 		errlinno = lineno = n->nredir.linno;
@@ -229,11 +227,8 @@ evaltree(union node *n, int flags)
 			lineno -= funcline - 1;
 		expredir(n->nredir.redirect);
 		pushredir(n->nredir.redirect);
-		status = redirectsafe(n->nredir.redirect, REDIR_PUSH);
-		if (!status) {
-			evaltree(n->nredir.n, flags & EV_TESTED);
-			status = exitstatus;
-		}
+		status = redirectsafe(n->nredir.redirect, REDIR_PUSH) ?:
+			 evaltree(n->nredir.n, flags & EV_TESTED);
 		if (n->nredir.redirect)
 			popredir(0);
 		goto setstatus;
@@ -241,8 +236,8 @@ evaltree(union node *n, int flags)
 #ifdef notyet
 		if (eflag && !(flags & EV_TESTED))
 			checkexit = ~0;
-		evalcommand(n, flags, (struct backcmd *)NULL);
-		break;
+		status = evalcommand(n, flags, (struct backcmd *)NULL);
+		goto setstatus;
 #else
 		evalfn = evalcommand;
 checkexit:
@@ -277,43 +272,39 @@ checkexit:
 #error NOR + 1 != NSEMI
 #endif
 		isor = n->type - NAND;
-		evaltree(
-			n->nbinary.ch1,
-			(flags | ((isor >> 1) - 1)) & EV_TESTED
-		);
-		if (!exitstatus == isor)
+		status = evaltree(n->nbinary.ch1,
+				  (flags | ((isor >> 1) - 1)) & EV_TESTED);
+		if (!status == isor)
 			break;
 		if (!evalskip) {
 			n = n->nbinary.ch2;
 evaln:
 			evalfn = evaltree;
 calleval:
-			evalfn(n, flags);
-			break;
+			status = evalfn(n, flags);
 		}
-		break;
+		goto setstatus;
 	case NIF:
-		evaltree(n->nif.test, EV_TESTED);
+		status = evaltree(n->nif.test, EV_TESTED);
 		if (evalskip)
 			break;
-		if (exitstatus == 0) {
+		if (!status) {
 			n = n->nif.ifpart;
 			goto evaln;
 		} else if (n->nif.elsepart) {
 			n = n->nif.elsepart;
 			goto evaln;
 		}
-		goto success;
+		status = 0;
+		goto setstatus;
 	case NDEFUN:
 		defun(n);
-success:
-		status = 0;
 setstatus:
 		exitstatus = status;
 		break;
 	}
 out:
-	if (checkexit & exitstatus)
+	if (checkexit & status)
 		goto exexit;
 
 	dotrap();
@@ -322,6 +313,8 @@ out:
 exexit:
 		exraise(EXEXIT);
 	}
+
+	return status;
 }
 
 
@@ -362,7 +355,7 @@ static int skiploop(void)
 }
 
 
-STATIC void
+STATIC int
 evalloop(union node *n, int flags)
 {
 	int skip;
@@ -374,33 +367,34 @@ evalloop(union node *n, int flags)
 	do {
 		int i;
 
-		evaltree(n->nbinary.ch1, EV_TESTED);
+		i = evaltree(n->nbinary.ch1, EV_TESTED);
 		skip = skiploop();
+		if (skip == SKIPFUNC)
+			status = i;
 		if (skip)
 			continue;
-		i = exitstatus;
 		if (n->type != NWHILE)
 			i = !i;
 		if (i != 0)
 			break;
-		evaltree(n->nbinary.ch2, flags);
-		status = exitstatus;
+		status = evaltree(n->nbinary.ch2, flags);
 		skip = skiploop();
 	} while (!(skip & ~SKIPCONT));
-	if (skip != SKIPFUNC)
-		exitstatus = status;
 	loopnest--;
+
+	return status;
 }
 
 
 
-STATIC void
+STATIC int
 evalfor(union node *n, int flags)
 {
 	struct arglist arglist;
 	union node *argp;
 	struct strlist *sp;
 	struct stackmark smark;
+	int status;
 
 	errlinno = lineno = n->nfor.linno;
 	if (funcline)
@@ -413,28 +407,31 @@ evalfor(union node *n, int flags)
 	}
 	*arglist.lastp = NULL;
 
-	exitstatus = 0;
+	status = 0;
 	loopnest++;
 	flags &= EV_TESTED;
 	for (sp = arglist.list ; sp ; sp = sp->next) {
 		setvar(n->nfor.var, sp->text, 0);
-		evaltree(n->nfor.body, flags);
+		status = evaltree(n->nfor.body, flags);
 		if (skiploop() & ~SKIPCONT)
 			break;
 	}
 	loopnest--;
 	popstackmark(&smark);
+
+	return status;
 }
 
 
 
-STATIC void
+STATIC int
 evalcase(union node *n, int flags)
 {
 	union node *cp;
 	union node *patp;
 	struct arglist arglist;
 	struct stackmark smark;
+	int status = 0;
 
 	errlinno = lineno = n->ncase.linno;
 	if (funcline)
@@ -443,12 +440,12 @@ evalcase(union node *n, int flags)
 	setstackmark(&smark);
 	arglist.lastp = &arglist.list;
 	expandarg(n->ncase.expr, &arglist, EXP_TILDE);
-	exitstatus = 0;
 	for (cp = n->ncase.cases ; cp && evalskip == 0 ; cp = cp->nclist.next) {
 		for (patp = cp->nclist.pattern ; patp ; patp = patp->narg.next) {
 			if (casematch(patp, arglist.list->text)) {
 				if (evalskip == 0) {
-					evaltree(cp->nclist.body, flags);
+					status = evaltree(cp->nclist.body,
+							  flags);
 				}
 				goto out;
 			}
@@ -456,6 +453,8 @@ evalcase(union node *n, int flags)
 	}
 out:
 	popstackmark(&smark);
+
+	return status;
 }
 
 
@@ -464,7 +463,7 @@ out:
  * Kick off a subshell to evaluate a tree.
  */
 
-STATIC void
+STATIC int
 evalsubshell(union node *n, int flags)
 {
 	struct job *jp;
@@ -493,8 +492,8 @@ nofork:
 	status = 0;
 	if (! backgnd)
 		status = waitforjob(jp);
-	exitstatus = status;
 	INTON;
+	return status;
 }
 
 
@@ -540,7 +539,7 @@ expredir(union node *n)
  * of all the rest.)
  */
 
-STATIC void
+STATIC int
 evalpipe(union node *n, int flags)
 {
 	struct job *jp;
@@ -548,6 +547,7 @@ evalpipe(union node *n, int flags)
 	int pipelen;
 	int prevfd;
 	int pip[2];
+	int status = 0;
 
 	TRACE(("evalpipe(0x%lx) called\n", (long)n));
 	pipelen = 0;
@@ -588,10 +588,12 @@ evalpipe(union node *n, int flags)
 		close(pip[1]);
 	}
 	if (n->npipe.backgnd == 0) {
-		exitstatus = waitforjob(jp);
+		status = waitforjob(jp);
 		TRACE(("evalpipe:  job done exit status %d\n", exitstatus));
 	}
 	INTON;
+
+	return status;
 }
 
 
@@ -678,7 +680,7 @@ parse_command_args(char **argv, const char **path)
  * Execute a simple command.
  */
 
-STATIC void
+STATIC int
 #ifdef notyet
 evalcommand(union node *cmd, int flags, struct backcmd *backcmd)
 #else
@@ -848,7 +850,7 @@ bail:
 			INTOFF;
 			jp = makejob(cmd, 1);
 			if (forkshell(jp, cmd, FORK_FG) != 0) {
-				exitstatus = waitforjob(jp);
+				status = waitforjob(jp);
 				INTON;
 				break;
 			}
@@ -867,17 +869,19 @@ bail:
 		if (evalbltin(cmdentry.u.cmd, argc, argv, flags)) {
 			if (exception == EXERROR && spclbltin <= 0) {
 				FORCEINTON;
-				break;
+				goto readstatus;
 			}
 raise:
 			longjmp(handler->loc, 1);
 		}
-		break;
+		goto readstatus;
 
 	case CMDFUNCTION:
 		poplocalvars(1);
 		if (evalfun(cmdentry.u.func, argc, argv, flags))
 			goto raise;
+readstatus:
+		status = exitstatus;
 		break;
 	}
 
@@ -893,6 +897,8 @@ out:
 		 */
 		setvar("_", lastarg, 0);
 	popstackmark(&smark);
+
+	return status;
 }
 
 STATIC int
diff --git a/src/eval.h b/src/eval.h
index 6e8acda..63e7d86 100644
--- a/src/eval.h
+++ b/src/eval.h
@@ -53,7 +53,7 @@ struct backcmd {		/* result of evalbackcmd */
 
 int evalstring(char *, int);
 union node;	/* BLETCH for ansi C */
-void evaltree(union node *, int);
+int evaltree(union node *, int);
 void evalbackcmd(union node *, struct backcmd *);
 
 extern int evalskip;
diff --git a/src/main.c b/src/main.c
index bedb663..580dbea 100644
--- a/src/main.c
+++ b/src/main.c
@@ -227,8 +227,7 @@ cmdloop(int top)
 		} else if (nflag == 0) {
 			job_warning = (job_warning == 2) ? 1 : 0;
 			numeof = 0;
-			evaltree(n, 0);
-			status = exitstatus;
+			status = evaltree(n, 0);
 		}
 		popstackmark(&smark);
 
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] eval: Return status in eval functions

[Harald van Dijk]

On 06/06/16 12:13, Herbert Xu wrote:
> On Sat, Dec 05, 2015 at 04:40:59PM +0100, Harald van Dijk wrote:
>>
>> used to print Fail, and needed the same modification in the evalstring
>> function to make that print OK (included in the attached patch). There
>> may be other similar bugs lurking.
>
> Not exactly similar but there are a bunch of bugs caused by setting
> exitstatus too early (i.e., before $? has been expanded).
>
> This patch should fix those problems plus the one that you fixed.

Nice! It does indeed fix additional problems. It also introduces a few 
new bugs though (modified from the FreeBSD testsuite):

src/dash -c 'eval "false

"'; echo $?

This should print 1 and used to print 1, but with this patch applied, it 
prints 0. The parsing here adds a null commands, which shouldn't affect 
the exit status.

Unfortunately, attempting to fix this by simply setting
status = exitstatus for null commands breaks other things: given

src/dash -c 'false; case x in *) esac'; echo $?

the correct output is 0, dash used to print 0, and your patch doesn't 
change that, but attempting to fix the earlier problem by setting status 
= exitstatus for null commands makes this print 1.

Another one modified from the FreeBSD testsuite:

src/dash -xc 'f() {
   trap "return 1" USR1
   kill -USR1 $$
}
f'; echo $?

This should print 1 and used to print 1, but with this patch applied, it 
prints 0.

> @@ -588,10 +588,12 @@ evalpipe(union node *n, int flags)
>   		close(pip[1]);
>   	}
>   	if (n->npipe.backgnd == 0) {
> -		exitstatus = waitforjob(jp);
> +		status = waitforjob(jp);
>   		TRACE(("evalpipe:  job done exit status %d\n", exitstatus));

This line looks like it should be updated to print status rather than 
exitstatus when tracing is enabled. :)

Cheers,
Harald van Dijk

[PATCH v2] eval: Return status in eval functions

[Herbert Xu]

On Mon, Jun 06, 2016 at 11:14:40PM +0200, Harald van Dijk wrote:
>
> Nice! It does indeed fix additional problems. It also introduces a
> few new bugs though (modified from the FreeBSD testsuite):
> 
> src/dash -c 'eval "false
> 
> "'; echo $?
> 
> This should print 1 and used to print 1, but with this patch
> applied, it prints 0. The parsing here adds a null commands, which
> shouldn't affect the exit status.

OK, I have applied your original patch with some formatting fixes.

> Unfortunately, attempting to fix this by simply setting
> status = exitstatus for null commands breaks other things: given
> 
> src/dash -c 'false; case x in *) esac'; echo $?

This is actually the result of the nofork path created for -c.
I'll fix it using the same method as you used in the original patch.

> Another one modified from the FreeBSD testsuite:
> 
> src/dash -xc 'f() {
>   trap "return 1" USR1
>   kill -USR1 $$
> }
> f'; echo $?
> 
> This should print 1 and used to print 1, but with this patch
> applied, it prints 0.

I have fixed this by rereading exitstatus after dotrap.

> >@@ -588,10 +588,12 @@ evalpipe(union node *n, int flags)
> >  		close(pip[1]);
> >  	}
> >  	if (n->npipe.backgnd == 0) {
> >-		exitstatus = waitforjob(jp);
> >+		status = waitforjob(jp);
> >  		TRACE(("evalpipe:  job done exit status %d\n", exitstatus));
> 
> This line looks like it should be updated to print status rather
> than exitstatus when tracing is enabled. :)

Fixed too.

---8<--
The exit status is currently clobbered too early for case statements
and loops.  This patch fixes it by making the eval functions return
the current exit status and setting them in one place -- evaltree.

Harald van Dijk pointed out a number of bugs in the original patch.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/src/eval.c b/src/eval.c
index 30c05f9..7498f9d 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -81,16 +81,16 @@ int savestatus = -1;		/* exit status of last command outside traps */
 STATIC
 #endif
 void evaltreenr(union node *, int) __attribute__ ((__noreturn__));
-STATIC void evalloop(union node *, int);
-STATIC void evalfor(union node *, int);
-STATIC void evalcase(union node *, int);
-STATIC void evalsubshell(union node *, int);
+STATIC int evalloop(union node *, int);
+STATIC int evalfor(union node *, int);
+STATIC int evalcase(union node *, int);
+STATIC int evalsubshell(union node *, int);
 STATIC void expredir(union node *);
-STATIC void evalpipe(union node *, int);
+STATIC int evalpipe(union node *, int);
 #ifdef notyet
-STATIC void evalcommand(union node *, int, struct backcmd *);
+STATIC int evalcommand(union node *, int, struct backcmd *);
 #else
-STATIC void evalcommand(union node *, int);
+STATIC int evalcommand(union node *, int);
 #endif
 STATIC int evalbltin(const struct builtincmd *, int, char **, int);
 STATIC int evalfun(struct funcnode *, int, char **, int);
@@ -170,11 +170,13 @@ evalstring(char *s, int flags)
 	setstackmark(&smark);
 
 	status = 0;
-	while ((n = parsecmd(0)) != NEOF) {
-		evaltree(n, flags & ~(parser_eof() ? 0 : EV_EXIT));
+	for (; (n = parsecmd(0)) != NEOF; popstackmark(&smark)) {
+		int i;
+
+		i = evaltree(n, flags & ~(parser_eof() ? 0 : EV_EXIT));
 		if (n)
-			status = exitstatus;
-		popstackmark(&smark);
+			status = i;
+
 		if (evalskip)
 			break;
 	}
@@ -192,13 +194,13 @@ evalstring(char *s, int flags)
  * exitstatus.
  */
 
-void
+int
 evaltree(union node *n, int flags)
 {
 	int checkexit = 0;
-	void (*evalfn)(union node *, int);
+	int (*evalfn)(union node *, int);
 	unsigned isor;
-	int status;
+	int status = 0;
 	if (n == NULL) {
 		TRACE(("evaltree(NULL) called\n"));
 		goto out;
@@ -221,8 +223,7 @@ evaltree(union node *n, int flags)
 		break;
 #endif
 	case NNOT:
-		evaltree(n->nnot.com, EV_TESTED);
-		status = !exitstatus;
+		status = !evaltree(n->nnot.com, EV_TESTED);
 		goto setstatus;
 	case NREDIR:
 		errlinno = lineno = n->nredir.linno;
@@ -230,11 +231,8 @@ evaltree(union node *n, int flags)
 			lineno -= funcline - 1;
 		expredir(n->nredir.redirect);
 		pushredir(n->nredir.redirect);
-		status = redirectsafe(n->nredir.redirect, REDIR_PUSH);
-		if (!status) {
-			evaltree(n->nredir.n, flags & EV_TESTED);
-			status = exitstatus;
-		}
+		status = redirectsafe(n->nredir.redirect, REDIR_PUSH) ?:
+			 evaltree(n->nredir.n, flags & EV_TESTED);
 		if (n->nredir.redirect)
 			popredir(0);
 		goto setstatus;
@@ -242,8 +240,8 @@ evaltree(union node *n, int flags)
 #ifdef notyet
 		if (eflag && !(flags & EV_TESTED))
 			checkexit = ~0;
-		evalcommand(n, flags, (struct backcmd *)NULL);
-		break;
+		status = evalcommand(n, flags, (struct backcmd *)NULL);
+		goto setstatus;
 #else
 		evalfn = evalcommand;
 checkexit:
@@ -278,43 +276,37 @@ checkexit:
 #error NOR + 1 != NSEMI
 #endif
 		isor = n->type - NAND;
-		evaltree(
-			n->nbinary.ch1,
-			(flags | ((isor >> 1) - 1)) & EV_TESTED
-		);
-		if (!exitstatus == isor)
+		status = evaltree(n->nbinary.ch1,
+				  (flags | ((isor >> 1) - 1)) & EV_TESTED);
+		if (!status == isor || evalskip)
 			break;
-		if (!evalskip) {
-			n = n->nbinary.ch2;
+		n = n->nbinary.ch2;
 evaln:
-			evalfn = evaltree;
+		evalfn = evaltree;
 calleval:
-			evalfn(n, flags);
-			break;
-		}
-		break;
+		status = evalfn(n, flags);
+		goto setstatus;
 	case NIF:
-		evaltree(n->nif.test, EV_TESTED);
+		status = evaltree(n->nif.test, EV_TESTED);
 		if (evalskip)
 			break;
-		if (exitstatus == 0) {
+		if (!status) {
 			n = n->nif.ifpart;
 			goto evaln;
 		} else if (n->nif.elsepart) {
 			n = n->nif.elsepart;
 			goto evaln;
 		}
-		goto success;
+		status = 0;
+		goto setstatus;
 	case NDEFUN:
 		defun(n);
-success:
-		status = 0;
 setstatus:
 		exitstatus = status;
 		break;
 	}
 out:
-	if (checkexit & exitstatus)
+	if (checkexit & status)
 		goto exexit;
 
 	dotrap();
@@ -323,6 +315,8 @@ out:
 exexit:
 		exraise(EXEXIT);
 	}
+
+	return exitstatus;
 }
 
 
@@ -363,7 +357,7 @@ static int skiploop(void)
 }
 
 
-STATIC void
+STATIC int
 evalloop(union node *n, int flags)
 {
 	int skip;
@@ -375,33 +369,34 @@ evalloop(union node *n, int flags)
 	do {
 		int i;
 
-		evaltree(n->nbinary.ch1, EV_TESTED);
+		i = evaltree(n->nbinary.ch1, EV_TESTED);
 		skip = skiploop();
+		if (skip == SKIPFUNC)
+			status = i;
 		if (skip)
 			continue;
-		i = exitstatus;
 		if (n->type != NWHILE)
 			i = !i;
 		if (i != 0)
 			break;
-		evaltree(n->nbinary.ch2, flags);
-		status = exitstatus;
+		status = evaltree(n->nbinary.ch2, flags);
 		skip = skiploop();
 	} while (!(skip & ~SKIPCONT));
-	if (skip != SKIPFUNC)
-		exitstatus = status;
 	loopnest--;
+
+	return status;
 }
 
 
 
-STATIC void
+STATIC int
 evalfor(union node *n, int flags)
 {
 	struct arglist arglist;
 	union node *argp;
 	struct strlist *sp;
 	struct stackmark smark;
+	int status;
 
 	errlinno = lineno = n->nfor.linno;
 	if (funcline)
@@ -414,28 +409,31 @@ evalfor(union node *n, int flags)
 	}
 	*arglist.lastp = NULL;
 
-	exitstatus = 0;
+	status = 0;
 	loopnest++;
 	flags &= EV_TESTED;
 	for (sp = arglist.list ; sp ; sp = sp->next) {
 		setvar(n->nfor.var, sp->text, 0);
-		evaltree(n->nfor.body, flags);
+		status = evaltree(n->nfor.body, flags);
 		if (skiploop() & ~SKIPCONT)
 			break;
 	}
 	loopnest--;
 	popstackmark(&smark);
+
+	return status;
 }
 
 
 
-STATIC void
+STATIC int
 evalcase(union node *n, int flags)
 {
 	union node *cp;
 	union node *patp;
 	struct arglist arglist;
 	struct stackmark smark;
+	int status = 0;
 
 	errlinno = lineno = n->ncase.linno;
 	if (funcline)
@@ -444,12 +442,16 @@ evalcase(union node *n, int flags)
 	setstackmark(&smark);
 	arglist.lastp = &arglist.list;
 	expandarg(n->ncase.expr, &arglist, EXP_TILDE);
-	exitstatus = 0;
 	for (cp = n->ncase.cases ; cp && evalskip == 0 ; cp = cp->nclist.next) {
 		for (patp = cp->nclist.pattern ; patp ; patp = patp->narg.next) {
 			if (casematch(patp, arglist.list->text)) {
-				if (evalskip == 0) {
-					evaltree(cp->nclist.body, flags);
+				/* Ensure body is non-empty as otherwise
+				 * EV_EXIT may prevent us from setting the
+				 * exit status.
+				 */
+				if (evalskip == 0 && cp->nclist.body) {
+					status = evaltree(cp->nclist.body,
+							  flags);
 				}
 				goto out;
 			}
@@ -457,6 +459,8 @@ evalcase(union node *n, int flags)
 	}
 out:
 	popstackmark(&smark);
+
+	return status;
 }
 
 
@@ -465,7 +469,7 @@ out:
  * Kick off a subshell to evaluate a tree.
  */
 
-STATIC void
+STATIC int
 evalsubshell(union node *n, int flags)
 {
 	struct job *jp;
@@ -494,8 +498,8 @@ nofork:
 	status = 0;
 	if (! backgnd)
 		status = waitforjob(jp);
-	exitstatus = status;
 	INTON;
+	return status;
 }
 
 
@@ -541,7 +545,7 @@ expredir(union node *n)
  * of all the rest.)
  */
 
-STATIC void
+STATIC int
 evalpipe(union node *n, int flags)
 {
 	struct job *jp;
@@ -549,6 +553,7 @@ evalpipe(union node *n, int flags)
 	int pipelen;
 	int prevfd;
 	int pip[2];
+	int status = 0;
 
 	TRACE(("evalpipe(0x%lx) called\n", (long)n));
 	pipelen = 0;
@@ -589,10 +594,12 @@ evalpipe(union node *n, int flags)
 		close(pip[1]);
 	}
 	if (n->npipe.backgnd == 0) {
-		exitstatus = waitforjob(jp);
-		TRACE(("evalpipe:  job done exit status %d\n", exitstatus));
+		status = waitforjob(jp);
+		TRACE(("evalpipe:  job done exit status %d\n", status));
 	}
 	INTON;
+
+	return status;
 }
 
 
@@ -679,7 +686,7 @@ parse_command_args(char **argv, const char **path)
  * Execute a simple command.
  */
 
-STATIC void
+STATIC int
 #ifdef notyet
 evalcommand(union node *cmd, int flags, struct backcmd *backcmd)
 #else
@@ -849,7 +856,7 @@ bail:
 			INTOFF;
 			jp = makejob(cmd, 1);
 			if (forkshell(jp, cmd, FORK_FG) != 0) {
-				exitstatus = waitforjob(jp);
+				status = waitforjob(jp);
 				INTON;
 				break;
 			}
@@ -868,17 +875,19 @@ bail:
 		if (evalbltin(cmdentry.u.cmd, argc, argv, flags)) {
 			if (exception == EXERROR && spclbltin <= 0) {
 				FORCEINTON;
-				break;
+				goto readstatus;
 			}
 raise:
 			longjmp(handler->loc, 1);
 		}
-		break;
+		goto readstatus;
 
 	case CMDFUNCTION:
 		poplocalvars(1);
 		if (evalfun(cmdentry.u.func, argc, argv, flags))
 			goto raise;
+readstatus:
+		status = exitstatus;
 		break;
 	}
 
@@ -894,6 +903,8 @@ out:
 		 */
 		setvar("_", lastarg, 0);
 	popstackmark(&smark);
+
+	return status;
 }
 
 STATIC int
diff --git a/src/eval.h b/src/eval.h
index 6e8acda..63e7d86 100644
--- a/src/eval.h
+++ b/src/eval.h
@@ -53,7 +53,7 @@ struct backcmd {		/* result of evalbackcmd */
 
 int evalstring(char *, int);
 union node;	/* BLETCH for ansi C */
-void evaltree(union node *, int);
+int evaltree(union node *, int);
 void evalbackcmd(union node *, struct backcmd *);
 
 extern int evalskip;
diff --git a/src/main.c b/src/main.c
index 497ac16..fcd3e7d 100644
--- a/src/main.c
+++ b/src/main.c
@@ -225,11 +225,13 @@ cmdloop(int top)
 			}
 			numeof++;
 		} else if (nflag == 0) {
+			int i;
+
 			job_warning = (job_warning == 2) ? 1 : 0;
 			numeof = 0;
-			evaltree(n, 0);
+			i = evaltree(n, 0);
 			if (n)
-				status = exitstatus;
+				status = i;
 		}
 		popstackmark(&smark);
 
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt