From cd418a26700629ad3f240ef6fd2e351ec0c2f87e Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Mon, 6 Apr 2009 18:08:35 +0000 Subject: [PATCH] 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 --- shell/hush.c | 57 +++++++++++++++-------- shell/hush_test/hush-misc/continue2.right | 1 + shell/hush_test/hush-misc/continue2.tests | 3 ++ shell/hush_test/hush-misc/continue3.right | 2 + shell/hush_test/hush-misc/continue3.tests | 3 ++ shell/hush_test/hush-misc/until1.right | 3 ++ shell/hush_test/hush-misc/until1.tests | 11 +++++ shell/hush_test/hush-misc/while2.right | 2 + shell/hush_test/hush-misc/while2.tests | 2 + shell/hush_test/run-all | 2 +- 10 files changed, 66 insertions(+), 20 deletions(-) create mode 100644 shell/hush_test/hush-misc/continue2.right create mode 100644 shell/hush_test/hush-misc/continue2.tests create mode 100644 shell/hush_test/hush-misc/continue3.right create mode 100644 shell/hush_test/hush-misc/continue3.tests create mode 100644 shell/hush_test/hush-misc/until1.right create mode 100644 shell/hush_test/hush-misc/until1.tests create mode 100644 shell/hush_test/hush-misc/while2.right create mode 100644 shell/hush_test/hush-misc/while2.tests 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 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"