hush: fix a bunch of obscure while/until/continue bugs

function                                             old     new   delta
run_list                                            1159    1214     +55
done_pipe                                            106     123     +17
done_command                                          86      98     +12
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 3/0 up/down: 84/0)               Total: 84 bytes

10 files changed, 66 insertions, 20 deletions

diff --git a/shell/hush.c b/shell/hush.c
index 227e73507..c74d10c1b 100644
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -2994,7 +2994,7 @@ static void debug_print_tree(struct pipe *pi, int lvl)
 			struct command *command = &pi->cmds[prn];
 			char **argv = command->argv;
 
-			fprintf(stderr, "%*s prog %d assignment_cnt:%d",
+			fprintf(stderr, "%*s cmd %d assignment_cnt:%d",
 					lvl*2, "", prn,
 					command->assignment_cnt);
 			if (command->group) {
@@ -3038,8 +3038,8 @@ static int run_list(struct pipe *pi)
 #else
 	enum { cond_code = 0 };
 #endif
-	/*enum reserved_style*/ smallint rword;
-	/*enum reserved_style*/ smallint last_rword;
+	smallint rword; /* enum reserved_style */
+	smallint last_rword; /* ditto */
 
 	debug_printf_exec("run_list start lvl %d\n", G.run_list_level + 1);
 
@@ -3307,8 +3307,7 @@ static int run_list(struct pipe *pi)
 				if (G.run_list_level == 1)
 					insert_bg_job(pi);
 #endif
-				rcode = EXIT_SUCCESS;
-				G.last_return_code = EXIT_SUCCESS;
+				G.last_return_code = rcode = EXIT_SUCCESS;
 				debug_printf_exec(": cmd&: exitcode EXIT_SUCCESS\n");
 			} else {
 #if ENABLE_HUSH_JOB
@@ -3334,17 +3333,23 @@ static int run_list(struct pipe *pi)
 			cond_code = rcode;
 #endif
 #if ENABLE_HUSH_LOOPS
-		if (rword == RES_WHILE) {
-			if (rcode) {
-				rcode = 0; /* "while false; do...done" - exitcode 0 */
-				goto check_jobs_and_break;
+		/* Beware of "while false; true; do ..."! */
+		if (pi->next && pi->next->res_word == RES_DO) {
+			if (rword == RES_WHILE) {
+				if (rcode) {
+					/* "while false; do...done" - exitcode 0 */
+					G.last_return_code = rcode = EXIT_SUCCESS;
+					debug_printf_exec(": while expr is false: breaking (exitcode:EXIT_SUCCESS)\n");
+					goto check_jobs_and_break;
+				}
 			}
-		}
-		if (rword == RES_UNTIL) {
-			if (!rcode) {
+			if (rword == RES_UNTIL) {
+				if (!rcode) {
+					debug_printf_exec(": until expr is true: breaking\n");
  check_jobs_and_break:
-				checkjobs(NULL);
-				break;
+					checkjobs(NULL);
+					break;
+				}
 			}
 		}
 #endif
@@ -3498,10 +3503,12 @@ static int done_command(struct parse_context *ctx)
 		 && command->redirects == NULL
 		) {
 			debug_printf_parse("done_command: skipping null cmd, num_cmds=%d\n", pi->num_cmds);
+			memset(command, 0, sizeof(*command)); /* paranoia */
 			return pi->num_cmds;
 		}
 		pi->num_cmds++;
 		debug_printf_parse("done_command: ++num_cmds=%d\n", pi->num_cmds);
+		//debug_print_tree(ctx->list_head, 20);
 	} else {
 		debug_printf_parse("done_command: initializing, num_cmds=%d\n", pi->num_cmds);
 	}
@@ -3526,16 +3533,26 @@ static void done_pipe(struct parse_context *ctx, pipe_style type)
 	/* Close previous command */
 	not_null = done_command(ctx);
 	ctx->pipe->followup = type;
-	IF_HAS_KEYWORDS(ctx->pipe->pi_inverted = ctx->ctx_inverted;)
-	IF_HAS_KEYWORDS(ctx->ctx_inverted = 0;)
-	IF_HAS_KEYWORDS(ctx->pipe->res_word = ctx->ctx_res_w;)
+#if HAS_KEYWORDS
+	ctx->pipe->pi_inverted = ctx->ctx_inverted;
+	ctx->ctx_inverted = 0;
+	ctx->pipe->res_word = ctx->ctx_res_w;
+#endif
 
 	/* Without this check, even just <enter> on command line generates
 	 * tree of three NOPs (!). Which is harmless but annoying.
 	 * IOW: it is safe to do it unconditionally.
 	 * RES_NONE case is for "for a in; do ..." (empty IN set)
-	 * to work, possibly other cases too. */
-	if (not_null IF_HAS_KEYWORDS(|| ctx->ctx_res_w != RES_NONE)) {
+	 * and other cases to work. */
+	if (not_null
+#if HAS_KEYWORDS
+	 || ctx->ctx_res_w == RES_FI
+	 || ctx->ctx_res_w == RES_DONE
+	 || ctx->ctx_res_w == RES_FOR
+	 || ctx->ctx_res_w == RES_IN
+	 || ctx->ctx_res_w == RES_ESAC
+#endif
+	) {
 		struct pipe *new_p;
 		debug_printf_parse("done_pipe: adding new pipe: "
 				"not_null:%d ctx->ctx_res_w:%d\n",
@@ -3564,6 +3581,7 @@ static void done_pipe(struct parse_context *ctx, pipe_style type)
 		 * ctx->command = &ctx->pipe->cmds[0];
 		 */
 		done_command(ctx);
+		//debug_print_tree(ctx->list_head, 10);
 	}
 	debug_printf_parse("done_pipe return\n");
 }
@@ -5483,6 +5501,7 @@ static int builtin_exec(char **argv)
 
 static int builtin_exit(char **argv)
 {
+	debug_printf_exec("%s()\n", __func__);
 // TODO: bash does it ONLY on top-level sh exit (+interacive only?)
 	//puts("exit"); /* bash does it */
 // TODO: warn if we have background jobs: "There are stopped jobs"
diff --git a/shell/hush_test/hush-misc/continue2.right b/shell/hush_test/hush-misc/continue2.right
new file mode 100644
index 000000000..49d3ebd3a
--- /dev/null
+++ b/shell/hush_test/hush-misc/continue2.right
@@ -0,0 +1 @@
+Ok:1
diff --git a/shell/hush_test/hush-misc/continue2.tests b/shell/hush_test/hush-misc/continue2.tests
new file mode 100644
index 000000000..c2df07195
--- /dev/null
+++ b/shell/hush_test/hush-misc/continue2.tests
@@ -0,0 +1,3 @@
+e=''
+(while test $e && exit 1; true; do e=1; continue; done)
+echo Ok:$?
diff --git a/shell/hush_test/hush-misc/continue3.right b/shell/hush_test/hush-misc/continue3.right
new file mode 100644
index 000000000..aa47d0d46
--- /dev/null
+++ b/shell/hush_test/hush-misc/continue3.right
@@ -0,0 +1,2 @@
+0
+0
diff --git a/shell/hush_test/hush-misc/continue3.tests b/shell/hush_test/hush-misc/continue3.tests
new file mode 100644
index 000000000..0aff867cd
--- /dev/null
+++ b/shell/hush_test/hush-misc/continue3.tests
@@ -0,0 +1,3 @@
+# Test that "continue" does affect exitcode (sets to 0)
+e=''
+while echo $?; test $e && exit; true; do e=1; false; continue; done
diff --git a/shell/hush_test/hush-misc/until1.right b/shell/hush_test/hush-misc/until1.right
new file mode 100644
index 000000000..be2daad95
--- /dev/null
+++ b/shell/hush_test/hush-misc/until1.right
@@ -0,0 +1,3 @@
+1
+1
+Ok:0
diff --git a/shell/hush_test/hush-misc/until1.tests b/shell/hush_test/hush-misc/until1.tests
new file mode 100644
index 000000000..10ab28381
--- /dev/null
+++ b/shell/hush_test/hush-misc/until1.tests
@@ -0,0 +1,11 @@
+x=1
+until test "$x" = 4; do echo $x; x=4; done
+
+# We had a bug in multi-line form
+x=1
+until test "$x" = 4; do
+        echo $x
+        x=4
+done
+
+echo Ok:$?
diff --git a/shell/hush_test/hush-misc/while2.right b/shell/hush_test/hush-misc/while2.right
new file mode 100644
index 000000000..07207cc84
--- /dev/null
+++ b/shell/hush_test/hush-misc/while2.right
@@ -0,0 +1,2 @@
+Hello
+OK:0
diff --git a/shell/hush_test/hush-misc/while2.tests b/shell/hush_test/hush-misc/while2.tests
new file mode 100644
index 000000000..2247adc74
--- /dev/null
+++ b/shell/hush_test/hush-misc/while2.tests
@@ -0,0 +1,2 @@
+while echo Hello; false; do echo NOT SHOWN; done
+echo OK:$?
diff --git a/shell/hush_test/run-all b/shell/hush_test/run-all
index 3fe3ba51d..c7989a172 100755
--- a/shell/hush_test/run-all
+++ b/shell/hush_test/run-all
@@ -46,7 +46,7 @@ do_test()
 	test -x "$x" || continue
 	name="${x%%.tests}"
 	test -f "$name.right" || continue
-#	echo Running test: "$name.right"
+#	echo Running test: "$x"
 	{
 	    "$THIS_SH" "./$x" >"$name.xx" 2>&1
 	    diff -u "$name.xx" "$name.right" >"../$1-$x.fail" && rm -f "$name.xx" "../$1-$x.fail"