From c96865f4458f357df41eeea73d456e15755b51f4 Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Fri, 10 Apr 2009 00:20:58 +0000 Subject: [PATCH] hush: readability improvements. fix some more obscure bugs. a new redir4.tests is known to fail. --- shell/hush.c | 216 ++++++++++++++----------- shell/hush_test/hush-misc/redir1.right | 2 + shell/hush_test/hush-misc/redir1.tests | 6 + shell/hush_test/hush-misc/redir4.right | 25 +++ shell/hush_test/hush-misc/redir4.tests | 80 +++++++++ 5 files changed, 232 insertions(+), 97 deletions(-) create mode 100644 shell/hush_test/hush-misc/redir4.right create mode 100755 shell/hush_test/hush-misc/redir4.tests diff --git a/shell/hush.c b/shell/hush.c index ac2410c48..21590adfb 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -341,7 +341,11 @@ typedef enum redir_type { REDIRECT_HEREDOC = 4, REDIRECT_IO = 5, REDIRECT_HEREDOC2 = 6, /* REDIRECT_HEREDOC after heredoc is loaded */ - REDIRFD_CLOSE = -3, + + REDIRFD_CLOSE = -3, + REDIRFD_SYNTAX_ERR = -2, + REDIRFD_TO_FILE = -1, /* otherwise, rd_fd if redirected to rd_dup */ + HEREDOC_SKIPTABS = 1, HEREDOC_QUOTED = 2, } redir_type; @@ -2427,6 +2431,7 @@ static int setup_redirects(struct command *prog, int squirrel[]) for (redir = prog->redirects; redir; redir = redir->next) { if (redir->rd_type == REDIRECT_HEREDOC2) { + /* rd_fd<rd_fd < 3) { squirrel[redir->rd_fd] = dup(redir->rd_fd); } @@ -2438,15 +2443,16 @@ static int setup_redirects(struct command *prog, int squirrel[]) continue; } - if (redir->rd_dup == -1) { + if (redir->rd_dup == REDIRFD_TO_FILE) { + /* rd_fd<*>file case (<*> is <,>,>>,<>) */ char *p; if (redir->rd_filename == NULL) { /* Something went wrong in the parse. * Pretend it didn't happen */ + bb_error_msg("bug in redirect parse"); continue; } mode = redir_table[redir->rd_type].mode; -//TODO: check redir for names like '\\' p = expand_string_to_string(redir->rd_filename); openfd = open_or_warn(p, mode); free(p); @@ -2457,6 +2463,7 @@ static int setup_redirects(struct command *prog, int squirrel[]) return 1; } } else { + /* rd_fd<*>rd_dup or rd_fd<*>- cases */ openfd = redir->rd_dup; } @@ -2469,7 +2476,7 @@ static int setup_redirects(struct command *prog, int squirrel[]) close(redir->rd_fd); } else { xdup2(openfd, redir->rd_fd); - if (redir->rd_dup == -1) + if (redir->rd_dup == REDIRFD_TO_FILE) close(openfd); } } @@ -3963,17 +3970,6 @@ static int done_word(o_string *word, struct parse_context *ctx) debug_printf_parse("done_word return 0: true null, ignored\n"); return 0; } - /* If this word wasn't an assignment, next ones definitely - * can't be assignments. Even if they look like ones. */ - if (word->o_assignment != DEFINITELY_ASSIGNMENT - && word->o_assignment != WORD_IS_KEYWORD - ) { - word->o_assignment = NOT_ASSIGNMENT; - } else { - if (word->o_assignment == DEFINITELY_ASSIGNMENT) - command->assignment_cnt++; - word->o_assignment = MAYBE_ASSIGNMENT; - } if (ctx->pending_redirect) { /* We do not glob in e.g. >*.tmp case. bash seems to glob here @@ -3989,29 +3985,47 @@ static int done_word(o_string *word, struct parse_context *ctx) * the expansion would result in one word." */ ctx->pending_redirect->rd_filename = xstrdup(word->data); + /* Cater for >\file case: + * >\a creates file a; >\\a, >"\a", >"\\a" create file \a + * Same with heredocs: + * for <<\H delim is H; <<\\H, <<"\H", <<"\\H" - \H + */ + unbackslash(ctx->pending_redirect->rd_filename); + /* Is it <<"HEREDOC"? */ if (ctx->pending_redirect->rd_type == REDIRECT_HEREDOC && word->o_quoted ) { ctx->pending_redirect->rd_dup |= HEREDOC_QUOTED; } - word->o_assignment = NOT_ASSIGNMENT; debug_printf_parse("word stored in rd_filename: '%s'\n", word->data); } else { - /* "{ echo foo; } echo bar" - bad */ - /* NB: bash allows e.g.: - * if true; then { echo foo; } fi - * while if false; then false; fi do break; done - * and disallows: - * while if false; then false; fi; do; break; done - * TODO? */ + /* If this word wasn't an assignment, next ones definitely + * can't be assignments. Even if they look like ones. */ + if (word->o_assignment != DEFINITELY_ASSIGNMENT + && word->o_assignment != WORD_IS_KEYWORD + ) { + word->o_assignment = NOT_ASSIGNMENT; + } else { + if (word->o_assignment == DEFINITELY_ASSIGNMENT) + command->assignment_cnt++; + word->o_assignment = MAYBE_ASSIGNMENT; + } + if (command->group) { + /* "{ echo foo; } echo bar" - bad */ + /* NB: bash allows e.g.: + * if true; then { echo foo; } fi + * while if false; then false; fi do break; done + * and disallows: + * while if false; then false; fi; do; break; done + * TODO? */ syntax_error_at(word->data); debug_printf_parse("done_word return 1: syntax error, " "groups and arglists don't mix\n"); return 1; } #if HAS_KEYWORDS -#if ENABLE_HUSH_CASE +# if ENABLE_HUSH_CASE if (ctx->ctx_dsemicolon && strcmp(word->data, "esac") != 0 /* not "... pattern) cmd;; esac" */ ) { @@ -4019,12 +4033,12 @@ static int done_word(o_string *word, struct parse_context *ctx) /* ctx->ctx_res_w = RES_MATCH; */ ctx->ctx_dsemicolon = 0; } else -#endif +# endif if (!command->argv /* if it's the first word... */ -#if ENABLE_HUSH_LOOPS +# if ENABLE_HUSH_LOOPS && ctx->ctx_res_w != RES_FOR /* ...not after FOR or IN */ && ctx->ctx_res_w != RES_IN -#endif +# endif ) { debug_printf_parse(": checking '%s' for reserved-ness\n", word->data); if (reserved_word(word, ctx)) { @@ -4090,20 +4104,23 @@ static int done_word(o_string *word, struct parse_context *ctx) /* Peek ahead in the input to find out if we have a "&n" construct, * as in "2>&1", that represents duplicating a file descriptor. - * Return: REDIRFD_CLOSE (-3) if >&- "close fd" construct is seen, - * -2 (syntax error), -1 if no & was seen, or the number found. + * Return: + * REDIRFD_CLOSE if >&- "close fd" construct is seen, + * REDIRFD_SYNTAX_ERR if syntax error, + * REDIRFD_TO_FILE if no & was seen, + * or the number found. */ #if BB_MMU -#define redirect_dup_num(as_string, input) \ - redirect_dup_num(input) +#define parse_redir_right_fd(as_string, input) \ + parse_redir_right_fd(input) #endif -static int redirect_dup_num(o_string *as_string, struct in_str *input) +static int parse_redir_right_fd(o_string *as_string, struct in_str *input) { int ch, d, ok; ch = i_peek(input); if (ch != '&') - return -1; + return REDIRFD_TO_FILE; ch = i_getch(input); /* get the & */ nommu_addchr(as_string, ch); @@ -4127,10 +4144,10 @@ static int redirect_dup_num(o_string *as_string, struct in_str *input) //TODO: this is the place to catch ">&file" bashism (redirect both fd 1 and 2) bb_error_msg("ambiguous redirect"); - return -2; + return REDIRFD_SYNTAX_ERR; } -/* Return code is 0 normally, 1 if a syntax error is detected +/* Return code is 0 normal, 1 if a syntax error is detected */ static int parse_redirect(struct parse_context *ctx, int fd, @@ -4142,12 +4159,12 @@ static int parse_redirect(struct parse_context *ctx, struct redir_struct **redirp; int dup_num; - dup_num = -1; + dup_num = REDIRFD_TO_FILE; if (style != REDIRECT_HEREDOC) { - /* Check for a '2>&1' type redirect */ - dup_num = redirect_dup_num(&ctx->as_string, input); - if (dup_num == -2) - return 1; /* syntax error */ + /* Check for a '>&1' type redirect */ + dup_num = parse_redir_right_fd(&ctx->as_string, input); + if (dup_num == REDIRFD_SYNTAX_ERR) + return 1; } else { int ch = i_peek(input); dup_num = (ch == '-'); /* HEREDOC_SKIPTABS bit is 1 */ @@ -4158,7 +4175,7 @@ static int parse_redirect(struct parse_context *ctx, } } - if (style == REDIRECT_OVERWRITE && dup_num == -1) { + if (style == REDIRECT_OVERWRITE && dup_num == REDIRFD_TO_FILE) { int ch = i_peek(input); if (ch == '|') { /* >|FILE redirect ("clobbering" >). @@ -4185,7 +4202,7 @@ static int parse_redirect(struct parse_context *ctx, redir_table[style].descrip); redir->rd_dup = dup_num; - if (style != REDIRECT_HEREDOC && dup_num != -1) { + if (style != REDIRECT_HEREDOC && dup_num != REDIRFD_TO_FILE) { /* Erik had a check here that the file descriptor in question * is legit; I postpone that to "run time" * A "-" representation of "close me" shows up as a -3 here */ @@ -4862,9 +4879,6 @@ static int parse_stream_dquoted(o_string *as_string, * only when followed by one of the following characters: * $, `, ", \, or . A double quote may be quoted * within double quotes by preceding it with a backslash. - * If enabled, history expansion will be performed unless - * an ! appearing in double quotes is escaped using - * a backslash. The backslash preceding the ! is not removed." */ if (strchr("$`\"\\", next) != NULL) { o_addqchr(dest, i_getch(input)); @@ -5081,17 +5095,71 @@ static struct pipe *parse_stream(char **pstring, if (is_ifs) continue; - if (dest.o_assignment == MAYBE_ASSIGNMENT) { - /* ch is a special char and thus this word - * cannot be an assignment */ - dest.o_assignment = NOT_ASSIGNMENT; - } - next = '\0'; if (ch != '\n') { next = i_peek(input); } + /* Catch <, > before deciding whether this word is + * an assignment. a=1 2>z b=2: b=2 is still assignment */ + switch (ch) { + case '>': + redir_fd = redirect_opt_num(&dest); + if (done_word(&dest, &ctx)) { + goto parse_error; + } + redir_style = REDIRECT_OVERWRITE; + if (next == '>') { + redir_style = REDIRECT_APPEND; + ch = i_getch(input); + nommu_addchr(&ctx.as_string, ch); + } +#if 0 + else if (next == '(') { + syntax_error(">(process) not supported"); + goto parse_error; + } +#endif + if (parse_redirect(&ctx, redir_fd, redir_style, input)) + goto parse_error; + continue; /* back to top of while (1) */ + case '<': + redir_fd = redirect_opt_num(&dest); + if (done_word(&dest, &ctx)) { + goto parse_error; + } + redir_style = REDIRECT_INPUT; + if (next == '<') { + redir_style = REDIRECT_HEREDOC; + heredoc_cnt++; + debug_printf_parse("++heredoc_cnt=%d\n", heredoc_cnt); + ch = i_getch(input); + nommu_addchr(&ctx.as_string, ch); + } else if (next == '>') { + redir_style = REDIRECT_IO; + ch = i_getch(input); + nommu_addchr(&ctx.as_string, ch); + } +#if 0 + else if (next == '(') { + syntax_error("<(process) not supported"); + goto parse_error; + } +#endif + if (parse_redirect(&ctx, redir_fd, redir_style, input)) + goto parse_error; + continue; /* back to top of while (1) */ + } + + if (dest.o_assignment == MAYBE_ASSIGNMENT + /* check that we are not in word in "a=1 2>word b=1": */ + && !ctx.pending_redirect + ) { + /* ch is a special char and thus this word + * cannot be an assignment */ + dest.o_assignment = NOT_ASSIGNMENT; + } + switch (ch) { case '#': if (dest.length == 0) { @@ -5171,52 +5239,6 @@ static struct pipe *parse_stream(char **pstring, break; } #endif - case '>': - redir_fd = redirect_opt_num(&dest); - if (done_word(&dest, &ctx)) { - goto parse_error; - } - redir_style = REDIRECT_OVERWRITE; - if (next == '>') { - redir_style = REDIRECT_APPEND; - ch = i_getch(input); - nommu_addchr(&ctx.as_string, ch); - } -#if 0 - else if (next == '(') { - syntax_error(">(process) not supported"); - goto parse_error; - } -#endif - if (parse_redirect(&ctx, redir_fd, redir_style, input)) - goto parse_error; - break; - case '<': - redir_fd = redirect_opt_num(&dest); - if (done_word(&dest, &ctx)) { - goto parse_error; - } - redir_style = REDIRECT_INPUT; - if (next == '<') { - redir_style = REDIRECT_HEREDOC; - heredoc_cnt++; - debug_printf_parse("++heredoc_cnt=%d\n", heredoc_cnt); - ch = i_getch(input); - nommu_addchr(&ctx.as_string, ch); - } else if (next == '>') { - redir_style = REDIRECT_IO; - ch = i_getch(input); - nommu_addchr(&ctx.as_string, ch); - } -#if 0 - else if (next == '(') { - syntax_error("<(process) not supported"); - goto parse_error; - } -#endif - if (parse_redirect(&ctx, redir_fd, redir_style, input)) - goto parse_error; - break; case ';': #if ENABLE_HUSH_CASE case_semi: diff --git a/shell/hush_test/hush-misc/redir1.right b/shell/hush_test/hush-misc/redir1.right index ac90b4a0a..15515d1af 100644 --- a/shell/hush_test/hush-misc/redir1.right +++ b/shell/hush_test/hush-misc/redir1.right @@ -1,3 +1,5 @@ +Test 0: var:ok +File created:ok Test 1: var:ok File created:ok Test 2: var:ok diff --git a/shell/hush_test/hush-misc/redir1.tests b/shell/hush_test/hush-misc/redir1.tests index 7e204514c..70e9e17f0 100755 --- a/shell/hush_test/hush-misc/redir1.tests +++ b/shell/hush_test/hush-misc/redir1.tests @@ -1,3 +1,9 @@ +rm shell_test_$$ 2>/dev/null +var=bad +>shell_test_$$ var=ok +echo "Test 0: var:$var" +test -f shell_test_$$ && echo "File created:ok" + rm shell_test_$$ 2>/dev/null var=bad var=ok >shell_test_$$ diff --git a/shell/hush_test/hush-misc/redir4.right b/shell/hush_test/hush-misc/redir4.right new file mode 100644 index 000000000..ada6c2d85 --- /dev/null +++ b/shell/hush_test/hush-misc/redir4.right @@ -0,0 +1,25 @@ +shell_test +\shell_test +\shell_test +\shell_test +Here1 +Ok1 +Here2 +Ok2 +Here3 +Ok3 +Here4 +Ok4 +How with variable refs +shell_test_1 +\shell_test_1 +\shell_test_1 +\shell_test_1 +Here1 +Ok1 +Here2 +Ok2 +Here3 +Ok3 +Here4 +Ok4 diff --git a/shell/hush_test/hush-misc/redir4.tests b/shell/hush_test/hush-misc/redir4.tests new file mode 100755 index 000000000..ac2a44166 --- /dev/null +++ b/shell/hush_test/hush-misc/redir4.tests @@ -0,0 +1,80 @@ +rm *shell_test* 2>/dev/null + +>\shell_test +echo *shell_test* +rm *shell_test* + +>\\shell_test +echo *shell_test* +rm *shell_test* + +>"\shell_test" +echo *shell_test* +rm *shell_test* + +>"\\shell_test" +echo *shell_test* +rm *shell_test* + + +cat <<\shell_test +Here1 +shell_test +echo Ok1 + +cat <<\\shell_test +Here2 +\shell_test +echo Ok2 + +cat <<"\shell_test" +Here3 +\shell_test +echo Ok3 + +cat <<"\\shell_test" +Here4 +\shell_test +echo Ok4 + + +echo How with variable refs +i=1 + + +>\shell_test_$i +echo *shell_test* +rm *shell_test* + +>\\shell_test_$i +echo *shell_test* +rm *shell_test* + +>"\shell_test_$i" +echo *shell_test* +rm *shell_test* + +>"\\shell_test_$i" +echo *shell_test* +rm *shell_test* + + +cat <<\shell_test_$i +Here1 +shell_test_$i +echo Ok1 + +cat <<\\shell_test_$i +Here2 +\shell_test_$i +echo Ok2 + +cat <<"\shell_test_$i" +Here3 +\shell_test_$i +echo Ok3 + +cat <<"\\shell_test_$i" +Here4 +\shell_test_$i +echo Ok4 -- 2.25.1