From 82dfec3e4e6d2b8354d901a525f9acff35369512 Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Mon, 16 Jun 2008 12:47:11 +0000 Subject: [PATCH] hush: fix hush-bugs/glob_and_vars.tests testcase: globbing is now done _after_ variable/`cmd` substitution function old new delta expand_strvec_to_strvec 7 353 +346 expand_variables 1348 1383 +35 add_string_to_strings - 28 +28 globhack 114 - -114 done_word 778 579 -199 ------------------------------------------------------------------------------ (add/remove: 1/1 grow/shrink: 2/1 up/down: 409/-313) Total: 96 bytes --- shell/hush.c | 109 ++++++++++-------- shell/hush_test/hush-bugs/empty_for.right | 1 + shell/hush_test/hush-bugs/empty_for.tests | 3 + shell/hush_test/hush-bugs/glob_and_vars.tests | 2 +- .../glob_and_vars.right | 0 shell/hush_test/hush-vars/glob_and_vars.tests | 2 + 6 files changed, 70 insertions(+), 47 deletions(-) create mode 100644 shell/hush_test/hush-bugs/empty_for.right create mode 100755 shell/hush_test/hush-bugs/empty_for.tests rename shell/hush_test/{hush-bugs => hush-vars}/glob_and_vars.right (100%) create mode 100755 shell/hush_test/hush-vars/glob_and_vars.tests diff --git a/shell/hush.c b/shell/hush.c index bc192b38b..7c5a44274 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -481,10 +481,6 @@ static int run_list(struct pipe *pi); static void pseudo_exec_argv(char **ptrs2free, char **argv) ATTRIBUTE_NORETURN; static void pseudo_exec(char **ptrs2free, struct child_prog *child) ATTRIBUTE_NORETURN; static int run_pipe(struct pipe *pi); -/* extended glob support: */ -static char **globhack(const char *src, char **strings); -static int glob_needed(const char *s); -static int xglob(o_string *dest, char ***pglob); /* variable assignment: */ static int is_assignment(const char *s); /* data structure manipulation: */ @@ -591,6 +587,17 @@ static char **alloc_ptrs(char **argv) } #endif +#ifdef DEBUG_EXPAND +static void debug_print_strings(const char *prefix, char **vv) +{ + fprintf(stderr, "%s:\n", prefix); + while (*vv) + fprintf(stderr, " '%s'\n", *vv++); +} +#else +#define debug_print_strings(prefix, vv) ((void)0) +#endif + /* Function prototypes for builtins */ static int builtin_cd(char **argv); @@ -2280,7 +2287,9 @@ static int run_list(struct pipe *pi) if (!pi->next->progs->argv) continue; /* create list of variable values */ + debug_print_strings("for_list made from", pi->next->progs->argv); for_list = expand_strvec_to_strvec(pi->next->progs->argv); + debug_print_strings("for_list", for_list); for_lcur = for_list; for_varname = pi->progs->argv[0]; pi->progs->argv[0] = NULL; @@ -2465,13 +2474,10 @@ static int run_and_free_list(struct pipe *pi) return rcode; } -/* The API for glob is arguably broken. This routine pushes a non-matching - * string into the output structure, removing non-backslashed backslashes. - * If someone can prove me wrong, by performing this function within the - * original glob(3) api, feel free to rewrite this routine into oblivion. +/* Remove non-backslashed backslashes and add to "strings" vector. * XXX broken if the last character is '\\', check that before calling. */ -static char **globhack(const char *src, char **strings) +static char **add_unq_string_to_strings(char **strings, const char *src) { int cnt; const char *s; @@ -2503,33 +2509,20 @@ static int glob_needed(const char *s) return 0; } -static int xglob(o_string *dest, char ***pglob) +static int xglob(char ***pglob, const char *pattern) { - /* short-circuit for null word */ - /* we can code this better when the debug_printf's are gone */ - if (dest->length == 0) { - if (dest->nonnull) { - /* bash man page calls this an "explicit" null */ - *pglob = globhack(dest->data, *pglob); - } - return 0; - } - - if (glob_needed(dest->data)) { + if (glob_needed(pattern)) { glob_t globdata; int gr; memset(&globdata, 0, sizeof(globdata)); - gr = glob(dest->data, 0, NULL, &globdata); + gr = glob(pattern, 0, NULL, &globdata); debug_printf("glob returned %d\n", gr); if (gr == GLOB_NOSPACE) bb_error_msg_and_die("out of memory during glob"); if (gr == GLOB_NOMATCH) { - debug_printf("globhack returned %d\n", gr); - /* quote removal, or more accurately, backslash removal */ - *pglob = globhack(dest->data, *pglob); globfree(&globdata); - return 0; + goto literal; } if (gr != 0) { /* GLOB_ABORTED ? */ bb_error_msg("glob(3) error %d", gr); @@ -2540,7 +2533,10 @@ static int xglob(o_string *dest, char ***pglob) return gr; } - *pglob = globhack(dest->data, *pglob); + literal: + /* quote removal, or more accurately, backslash removal */ + *pglob = add_unq_string_to_strings(*pglob, pattern); + debug_print_strings("after xglob", *pglob); return 0; } @@ -2733,7 +2729,7 @@ static char **expand_variables(char **argv, char or_mask) n = expand_vars_to_list(&output, n, *v++, or_mask); o_debug_list("expand_variables", &output, n); - /* output.data (malloced) gets returned in "list" */ + /* output.data (malloced in one block) gets returned in "list" */ list = o_finalize_list(&output, n); #ifdef DEBUG_EXPAND @@ -2750,9 +2746,27 @@ static char **expand_variables(char **argv, char or_mask) static char **expand_strvec_to_strvec(char **argv) { - return expand_variables(argv, 0); + char **exp; + char **res = NULL; + + debug_print_strings("expand_strvec_to_strvec: pre expand", argv); + exp = argv = expand_variables(argv, 0); + debug_print_strings("expand_strvec_to_strvec: post expand", argv); + while (*argv) { + int r = xglob(&res, *argv); + if (r) + bb_error_msg("xglob returned %d on '%s'", r, *argv); +//TODO: testcase for bad glob pattern behavior + argv++; + } + free(exp); + debug_print_strings("expand_strvec_to_strvec: res", res); + return res; } +/* used for expansion of right hand of assignments */ +/* NB: should NOT do globbing! "export v=/bin/c*; env | grep ^v=" outputs + * "v=/bin/c*" */ static char *expand_string_to_string(const char *str) { char *argv[2], **list; @@ -2769,6 +2783,7 @@ static char *expand_string_to_string(const char *str) return (char*)list; } +/* used for eval */ static char* expand_strvec_to_string(char **argv) { char **list; @@ -3106,7 +3121,6 @@ static int done_word(o_string *word, struct p_context *ctx) { struct child_prog *child = ctx->child; char ***glob_target; - int gr; debug_printf_parse("done_word entered: '%s' %p\n", word->data, child); if (word->length == 0 && !word->nonnull) { @@ -3131,12 +3145,10 @@ static int done_word(o_string *word, struct p_context *ctx) } glob_target = &child->argv; } -//BUG! globbing should be done after variable expansion! -//See glob_and_vars testcase - gr = xglob(word, glob_target); - if (gr != 0) { - debug_printf_parse("done_word return 1: xglob returned %d\n", gr); - return 1; + + if (word->length || word->nonnull) { + *glob_target = add_string_to_strings(*glob_target, xstrdup(word->data)); + debug_print_strings("glob_target appended", *glob_target); } o_reset(word); @@ -3371,6 +3383,11 @@ static int process_command_subs(o_string *dest, o_addQchr(dest, '\n'); eol_cnt--; } + /* Even unquoted `echo '\'` results in two backslashes + * (which are converted into one by globbing later) */ + if (!dest->o_quote && ch == '\\') { + o_addchr(dest, ch); + } o_addQchr(dest, ch); } @@ -3440,7 +3457,7 @@ static void add_till_single_quote(o_string *dest, struct in_str *input) break; if (ch == '\'') break; - o_addqchr(dest, ch); + o_addchr(dest, ch); } } /* "...\"...`..`...." - do we need to handle "...$(..)..." too? */ @@ -3451,15 +3468,15 @@ static void add_till_double_quote(o_string *dest, struct in_str *input) if (ch == '"') break; if (ch == '\\') { /* \x. Copy both chars. */ - o_addqchr(dest, ch); + o_addchr(dest, ch); ch = i_getch(input); } if (ch == EOF) break; - o_addqchr(dest, ch); + o_addchr(dest, ch); if (ch == '`') { add_till_backquote(dest, input); - o_addqchr(dest, ch); + o_addchr(dest, ch); continue; } //if (ch == '$') ... @@ -3488,12 +3505,12 @@ static void add_till_backquote(o_string *dest, struct in_str *input) if (ch == '\\') { /* \x. Copy both chars unless it is \` */ int ch2 = i_getch(input); if (ch2 != '`' && ch2 != '$' && ch2 != '\\') - o_addqchr(dest, ch); + o_addchr(dest, ch); ch = ch2; } if (ch == EOF) break; - o_addqchr(dest, ch); + o_addchr(dest, ch); } } /* Process $(cmd) - copy contents until ")" is seen. Complicated by @@ -3520,22 +3537,22 @@ static void add_till_closing_curly_brace(o_string *dest, struct in_str *input) if (ch == ')') if (--count < 0) break; - o_addqchr(dest, ch); + o_addchr(dest, ch); if (ch == '\'') { add_till_single_quote(dest, input); - o_addqchr(dest, ch); + o_addchr(dest, ch); continue; } if (ch == '"') { add_till_double_quote(dest, input); - o_addqchr(dest, ch); + o_addchr(dest, ch); continue; } if (ch == '\\') { /* \x. Copy verbatim. Important for \(, \) */ ch = i_getch(input); if (ch == EOF) break; - o_addqchr(dest, ch); + o_addchr(dest, ch); continue; } } diff --git a/shell/hush_test/hush-bugs/empty_for.right b/shell/hush_test/hush-bugs/empty_for.right new file mode 100644 index 000000000..290d39b7e --- /dev/null +++ b/shell/hush_test/hush-bugs/empty_for.right @@ -0,0 +1 @@ +OK: 0 diff --git a/shell/hush_test/hush-bugs/empty_for.tests b/shell/hush_test/hush-bugs/empty_for.tests new file mode 100755 index 000000000..0cb52e849 --- /dev/null +++ b/shell/hush_test/hush-bugs/empty_for.tests @@ -0,0 +1,3 @@ +false +for a in; do echo "HELLO"; done +echo OK: $? diff --git a/shell/hush_test/hush-bugs/glob_and_vars.tests b/shell/hush_test/hush-bugs/glob_and_vars.tests index c8e0c067d..482cf9d8a 100755 --- a/shell/hush_test/hush-bugs/glob_and_vars.tests +++ b/shell/hush_test/hush-bugs/glob_and_vars.tests @@ -1,2 +1,2 @@ v=. -echo $v/glob_and_vars.* +echo $v/glob_and_vars.[tr]* diff --git a/shell/hush_test/hush-bugs/glob_and_vars.right b/shell/hush_test/hush-vars/glob_and_vars.right similarity index 100% rename from shell/hush_test/hush-bugs/glob_and_vars.right rename to shell/hush_test/hush-vars/glob_and_vars.right diff --git a/shell/hush_test/hush-vars/glob_and_vars.tests b/shell/hush_test/hush-vars/glob_and_vars.tests new file mode 100755 index 000000000..482cf9d8a --- /dev/null +++ b/shell/hush_test/hush-vars/glob_and_vars.tests @@ -0,0 +1,2 @@ +v=. +echo $v/glob_and_vars.[tr]*