[git commit] ash: fix "return N" not setting $? in loop conditionals

Denys Vlasenko vda.linux at googlemail.com
Sat Oct 1 17:56:52 UTC 2016


commit: https://git.busybox.net/busybox/commit/?id=35ec818fa23b9c68572f871da5c325101009939a
branch: https://git.busybox.net/busybox/commit/?id=refs/heads/master

Upstream commit 1:

    Date: Mon, 6 Oct 2014 20:45:04 +0800
    [EVAL] Move common skipcount logic into skiploop

    The functions evalloop and evalfor share the logic on checking
    and updating skipcount.  This patch moves that into the helper
    function skiploop.

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

Upstream commit 2:

    Date: Mon, 6 Oct 2014 21:22:43 +0800
    [BUILTIN] Allow return in loop conditional to set exit status

    https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=332954

    When return is used in a loop conditional the exit status will
    be lost because we always set the exit status at the end of the
    loop to that of the last command executed in the body.

    This is counterintuitive and contrary to what most other shells do.

    This patch fixes this by always preserving the exit status of
    return when it is used in a loop conditional.

    The patch was originally written by Gerrit Pape <pape at smarden.org>.

    Reported-by: Stephane Chazelas <stephane_chazelas at yahoo.fr>
    Signed-off-by: Herbert Xu <herbert at gondor.apana.org.au>

Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
 shell/ash.c                         | 56 +++++++++++++++++++------------------
 shell/ash_test/ash-misc/func6.right |  2 ++
 shell/ash_test/ash-misc/func6.tests | 11 ++++++++
 3 files changed, 42 insertions(+), 27 deletions(-)

diff --git a/shell/ash.c b/shell/ash.c
index 06df07d..e4349cc 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -8632,37 +8632,50 @@ static
 #endif
 int evaltreenr(union node *, int) __attribute__ ((alias("evaltree"),__noreturn__));
 
+static int skiploop(void)
+{
+	int skip = evalskip;
+
+	switch (skip) {
+	case 0:
+		break;
+	case SKIPBREAK:
+	case SKIPCONT:
+		if (--skipcount <= 0) {
+			evalskip = 0;
+			break;
+		}
+		skip = SKIPBREAK;
+		break;
+	}
+	return skip;
+}
+
 static int
 evalloop(union node *n, int flags)
 {
+	int skip;
 	int status;
 
 	loopnest++;
 	status = 0;
 	flags &= EV_TESTED;
-	for (;;) {
+	do {
 		int i;
 
 		i = evaltree(n->nbinary.ch1, EV_TESTED);
-		if (evalskip) {
- skipping:
-			if (evalskip == SKIPCONT && --skipcount <= 0) {
-				evalskip = 0;
-				continue;
-			}
-			if (evalskip == SKIPBREAK && --skipcount <= 0)
-				evalskip = 0;
-			break;
-		}
+		skip = skiploop();
+		if (skip == SKIPFUNC)
+			status = i;
+		if (skip)
+			continue;
 		if (n->type != NWHILE)
 			i = !i;
 		if (i != 0)
 			break;
 		status = evaltree(n->nbinary.ch2, flags);
-		if (evalskip)
-			goto skipping;
-	}
-	exitstatus = status;
+		skip = skiploop();
+	} while (!(skip & ~SKIPCONT));
 	loopnest--;
 
 	return status;
@@ -8682,9 +8695,6 @@ evalfor(union node *n, int flags)
 	arglist.lastp = &arglist.list;
 	for (argp = n->nfor.args; argp; argp = argp->narg.next) {
 		expandarg(argp, &arglist, EXP_FULL | EXP_TILDE);
-		/* XXX */
-		if (evalskip)
-			goto out;
 	}
 	*arglist.lastp = NULL;
 
@@ -8693,18 +8703,10 @@ evalfor(union node *n, int flags)
 	for (sp = arglist.list; sp; sp = sp->next) {
 		setvar0(n->nfor.var, sp->text);
 		status = evaltree(n->nfor.body, flags);
-		if (evalskip) {
-			if (evalskip == SKIPCONT && --skipcount <= 0) {
-				evalskip = 0;
-				continue;
-			}
-			if (evalskip == SKIPBREAK && --skipcount <= 0)
-				evalskip = 0;
+		if (skiploop() & ~SKIPCONT)
 			break;
-		}
 	}
 	loopnest--;
- out:
 	popstackmark(&smark);
 
 	return status;
diff --git a/shell/ash_test/ash-misc/func6.right b/shell/ash_test/ash-misc/func6.right
new file mode 100644
index 0000000..0ebd8e5
--- /dev/null
+++ b/shell/ash_test/ash-misc/func6.right
@@ -0,0 +1,2 @@
+Two:2
+Two:2
diff --git a/shell/ash_test/ash-misc/func6.tests b/shell/ash_test/ash-misc/func6.tests
new file mode 100755
index 0000000..029c3e8
--- /dev/null
+++ b/shell/ash_test/ash-misc/func6.tests
@@ -0,0 +1,11 @@
+f1() {
+	while return 2; do :; done
+}
+f1
+echo Two:$?
+
+f2() {
+	while :; do return 2; done
+}
+f2
+echo Two:$?


More information about the busybox-cvs mailing list