From 35ec818fa23b9c68572f871da5c325101009939a Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sat, 1 Oct 2016 19:56:52 +0200 Subject: [PATCH] ash: fix "return N" not setting $? in loop conditionals 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 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 . Reported-by: Stephane Chazelas Signed-off-by: Herbert Xu Signed-off-by: Denys Vlasenko --- 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(-) create mode 100644 shell/ash_test/ash-misc/func6.right create mode 100755 shell/ash_test/ash-misc/func6.tests diff --git a/shell/ash.c b/shell/ash.c index 06df07d06..e4349ccad 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 000000000..0ebd8e5a3 --- /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 000000000..029c3e85e --- /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:$? -- 2.25.1