From 929a41d5770c0531f037c2e7db25bf98f9029c9e Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Thu, 5 Apr 2018 14:09:14 +0200 Subject: [PATCH] hush: less mind-bending set_vars_and_save_old() function old new delta run_pipe 1651 1701 +50 set_local_var 510 557 +47 pseudo_exec_argv 544 581 +37 redirect_and_varexp_helper 64 56 -8 set_vars_and_save_old 164 149 -15 unset_local_var 274 256 -18 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 3/3 up/down: 134/-41) Total: 93 bytes Signed-off-by: Denys Vlasenko --- shell/hush.c | 172 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 101 insertions(+), 71 deletions(-) diff --git a/shell/hush.c b/shell/hush.c index 4740929e8..24ae237d7 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -2140,19 +2140,10 @@ static int set_local_var(char *str, unsigned flags) unsigned local_lvl = (flags >> SETFLAG_VARLVL_SHIFT); eq_sign = strchr(str, '='); - if (!eq_sign) { /* not expected to ever happen? */ - free(str); - return -1; - } + if (HUSH_DEBUG && !eq_sign) + bb_error_msg_and_die("BUG in setvar"); name_len = eq_sign - str + 1; /* including '=' */ -#if ENABLE_HUSH_LINENO_VAR - if (G.lineno_var) { - if (name_len == 7 && strncmp("LINENO", str, 6) == 0) - G.lineno_var = NULL; - } -#endif - cur_pp = &G.top_var; while ((cur = *cur_pp) != NULL) { if (strncmp(cur->varstr, str, name_len) != 0) { @@ -2175,19 +2166,6 @@ static int set_local_var(char *str, unsigned flags) *eq_sign = '='; } if (cur->var_nest_level < local_lvl) { - /* New variable is local ("local VAR=VAL" or - * "VAR=VAL cmd") - * and existing one is global, or local - * on a lower level that new one. - * Remove and save old one: - */ - debug_printf_env("shadowing %s'%s'/%u by '%s'/%u\n", - cur->flg_export ? "exported " : "", - cur->varstr, cur->var_nest_level, str, local_lvl - ); - *cur_pp = cur->next; - cur->next = *G.shadowed_vars_pp; - *G.shadowed_vars_pp = cur; /* bash 3.2.33(1) and exported vars: * # export z=z * # f() { local z=a; env | grep ^z; } @@ -2198,6 +2176,31 @@ static int set_local_var(char *str, unsigned flags) */ if (cur->flg_export) flags |= SETFLAG_EXPORT; + /* New variable is local ("local VAR=VAL" or + * "VAR=VAL cmd") + * and existing one is global, or local + * on a lower level that new one. + * Remove it from global variable list: + */ + *cur_pp = cur->next; + if (G.shadowed_vars_pp) { + /* Save in "shadowed" list */ + debug_printf_env("shadowing %s'%s'/%u by '%s'/%u\n", + cur->flg_export ? "exported " : "", + cur->varstr, cur->var_nest_level, str, local_lvl + ); + cur->next = *G.shadowed_vars_pp; + *G.shadowed_vars_pp = cur; + } else { + /* Came from pseudo_exec_argv(), no need to save: delete it */ + debug_printf_env("shadow-deleting %s'%s'/%u by '%s'/%u\n", + cur->flg_export ? "exported " : "", + cur->varstr, cur->var_nest_level, str, local_lvl + ); + if (cur->max_len == 0) /* allocated "VAR=VAL"? */ + free_me = cur->varstr; /* then free it later */ + free(cur); + } break; } @@ -2207,8 +2210,10 @@ static int set_local_var(char *str, unsigned flags) free(str); goto exp; } + + /* Replace the value in the found "struct variable" */ if (cur->max_len != 0) { - if (cur->max_len >= strlen(str)) { + if (cur->max_len >= strnlen(str, cur->max_len + 1)) { /* This one is from startup env, reuse space */ debug_printf_env("reusing startup env for '%s'\n", str); strcpy(cur->varstr, str); @@ -2244,13 +2249,6 @@ static int set_local_var(char *str, unsigned flags) #endif if (flags & SETFLAG_EXPORT) cur->flg_export = 1; - if (name_len == 4 && cur->varstr[0] == 'P' && cur->varstr[1] == 'S') - cmdedit_update_prompt(); -#if ENABLE_HUSH_GETOPTS - /* defoptindvar is a "OPTIND=..." constant string */ - if (strncmp(cur->varstr, defoptindvar, 7) == 0) - G.getopt_count = 0; -#endif if (cur->flg_export) { if (flags & SETFLAG_UNEXPORT) { cur->flg_export = 0; @@ -2265,6 +2263,23 @@ static int set_local_var(char *str, unsigned flags) } } free(free_me); + + /* Handle special names */ + if (name_len == 4 && cur->varstr[0] == 'P' && cur->varstr[1] == 'S') + cmdedit_update_prompt(); + else + if ((ENABLE_HUSH_LINENO_VAR || ENABLE_HUSH_GETOPTS) && name_len == 7) { +#if ENABLE_HUSH_LINENO_VAR + if (G.lineno_var && strncmp(cur->varstr, "LINENO", 6) == 0) + G.lineno_var = NULL; +#endif +#if ENABLE_HUSH_GETOPTS + /* defoptindvar is a "OPTIND=..." constant string */ + if (strncmp(cur->varstr, defoptindvar, 7) == 0) + G.getopt_count = 0; +#endif + } + return 0; } @@ -2282,14 +2297,16 @@ static int unset_local_var_len(const char *name, int name_len) if (!name) return EXIT_SUCCESS; + if ((ENABLE_HUSH_LINENO_VAR || ENABLE_HUSH_GETOPTS) && name_len == 6) { #if ENABLE_HUSH_GETOPTS - if (name_len == 6 && strncmp(name, "OPTIND", 6) == 0) - G.getopt_count = 0; + if (strncmp(name, "OPTIND", 6) == 0) + G.getopt_count = 0; #endif #if ENABLE_HUSH_LINENO_VAR - if (name_len == 6 && G.lineno_var && strncmp(name, "LINENO", 6) == 0) - G.lineno_var = NULL; + if (G.lineno_var && strncmp(name, "LINENO", 6) == 0) + G.lineno_var = NULL; #endif + } cur_pp = &G.top_var; while ((cur = *cur_pp) != NULL) { @@ -2355,13 +2372,12 @@ static void add_vars(struct variable *var) * which attempts to overwrite it. * The strings[] vector itself is freed. */ -static struct variable *set_vars_and_save_old(char **strings) +static void set_vars_and_save_old(char **strings) { char **s; - struct variable *old = NULL; if (!strings) - return old; + return; s = strings; while (*s) { @@ -2389,12 +2405,10 @@ static struct variable *set_vars_and_save_old(char **strings) do { *p = p[1]; p++; } while (*p); goto next; } - /* Remove variable from global linked list */ - debug_printf_env("%s: removing '%s'\n", __func__, var_p->varstr); - *var_pp = var_p->next; - /* Add it to returned list */ - var_p->next = old; - old = var_p; + /* below, set_local_var() with nest level will + * "shadow" (remove) this variable from + * global linked list. + */ } //bb_error_msg("G.var_nest_level:%d", G.var_nest_level); set_local_var(*s, (G.var_nest_level << SETFLAG_VARLVL_SHIFT) | SETFLAG_EXPORT); @@ -2405,7 +2419,6 @@ static struct variable *set_vars_and_save_old(char **strings) next: ; } free(strings); - return old; } @@ -7598,6 +7611,7 @@ static NOINLINE void pseudo_exec_argv(nommu_save_t *nommu_save, char **argv_expanded) { const struct built_in_command *x; + struct variable **sv_shadowed; char **new_env; IF_HUSH_COMMAND(char opt_vV = 0;) IF_HUSH_FUNCTIONS(const struct function *funcp;) @@ -7614,13 +7628,14 @@ static NOINLINE void pseudo_exec_argv(nommu_save_t *nommu_save, _exit(EXIT_SUCCESS); } + sv_shadowed = G.shadowed_vars_pp; #if BB_MMU - set_vars_and_save_old(new_env); - /* we can destroy set_vars_and_save_old's return value, - * to save memory */ + G.shadowed_vars_pp = NULL; /* "don't save, free them instead" */ #else - nommu_save->old_vars = set_vars_and_save_old(new_env); + G.shadowed_vars_pp = &nommu_save->old_vars; #endif + set_vars_and_save_old(new_env); + G.shadowed_vars_pp = sv_shadowed; if (argv_expanded) { argv = argv_expanded; @@ -8181,7 +8196,6 @@ static int checkjobs_and_fg_shell(struct pipe *fg_pipe) redirect_and_varexp_helper(old_vars_p, command, squirrel) #endif static int redirect_and_varexp_helper( - struct variable **old_vars_p, struct command *command, struct squirrel **sqp, char **argv_expanded) @@ -8196,7 +8210,7 @@ static int redirect_and_varexp_helper( dump_cmd_in_x_mode(new_env); dump_cmd_in_x_mode(argv_expanded); /* this takes ownership of new_env[i] elements, and frees new_env: */ - *old_vars_p = set_vars_and_save_old(new_env); + set_vars_and_save_old(new_env); } return rcode; } @@ -8288,6 +8302,7 @@ static NOINLINE int run_pipe(struct pipe *pi) const struct built_in_command *x; IF_HUSH_FUNCTIONS(const struct function *funcp;) IF_NOT_HUSH_FUNCTIONS(enum { funcp = 0 };) + struct variable **sv_shadowed; struct variable *old_vars; #if ENABLE_HUSH_LINENO_VAR @@ -8338,13 +8353,11 @@ static NOINLINE int run_pipe(struct pipe *pi) /* Expand the rest into (possibly) many strings each */ #if defined(CMD_SINGLEWORD_NOGLOB) - if (command->cmd_type == CMD_SINGLEWORD_NOGLOB) { + if (command->cmd_type == CMD_SINGLEWORD_NOGLOB) argv_expanded = expand_strvec_to_strvec_singleword_noglob(argv + command->assignment_cnt); - } else + else #endif - { argv_expanded = expand_strvec_to_strvec(argv + command->assignment_cnt); - } /* if someone gives us an empty string: `cmd with empty output` */ //TODO: what about: var=EXPR `` >FILE ? will var be set? Will FILE be created? @@ -8354,17 +8367,19 @@ static NOINLINE int run_pipe(struct pipe *pi) return G.last_exitcode; } -#if ENABLE_HUSH_FUNCTIONS + old_vars = NULL; + sv_shadowed = G.shadowed_vars_pp; + /* Check if argv[0] matches any functions (this goes before bltins) */ - funcp = find_function(argv_expanded[0]); -#endif - x = NULL; - if (!funcp) + IF_HUSH_FUNCTIONS(funcp = find_function(argv_expanded[0]);) + IF_HUSH_FUNCTIONS(x = NULL;) + IF_HUSH_FUNCTIONS(if (!funcp)) x = find_builtin(argv_expanded[0]); if (x || funcp) { if (x && x->b_function == builtin_exec && argv_expanded[1] == NULL) { debug_printf("exec with redirects only\n"); rcode = setup_redirects(command, NULL); +//TODO: what about: var=EXPR exec >FILE ? will var be set? /* rcode=1 can be if redir file can't be opened */ goto clean_up_and_ret1; } @@ -8373,9 +8388,22 @@ static NOINLINE int run_pipe(struct pipe *pi) * a=b true; env | grep ^a= */ enter_var_nest_level(); - rcode = redirect_and_varexp_helper(&old_vars, command, &squirrel, argv_expanded); + /* Collect all variables "shadowed" by helper + * (IOW: old vars overridden by "var1=val1 var2=val2 cmd..." syntax) + * into old_vars list: + */ + G.shadowed_vars_pp = &old_vars; + rcode = redirect_and_varexp_helper(command, &squirrel, argv_expanded); if (rcode == 0) { if (!funcp) { + /* Do not collect *to old_vars list* vars shadowed + * by e.g. "local VAR" builtin (collect them + * in the previously nested list instead): + * don't want them to be restored immediately + * after "local" completes. + */ + G.shadowed_vars_pp = sv_shadowed; + debug_printf_exec(": builtin '%s' '%s'...\n", x->b_cmd, argv_expanded[1]); fflush_all(); @@ -8384,17 +8412,15 @@ static NOINLINE int run_pipe(struct pipe *pi) } #if ENABLE_HUSH_FUNCTIONS else { - struct variable **sv = G.shadowed_vars_pp; - /* Make "local" builtins in the called function - * stash shadowed variables in old_vars list - */ - G.shadowed_vars_pp = &old_vars; - debug_printf_exec(": function '%s' '%s'...\n", funcp->name, argv_expanded[1]); rcode = run_function(funcp, argv_expanded) & 0xff; - - G.shadowed_vars_pp = sv; + /* + * But do collect *to old_vars list* vars shadowed + * within function execution. To that end, restore + * this pointer _after_ function run: + */ + G.shadowed_vars_pp = sv_shadowed; } #endif } @@ -8405,7 +8431,11 @@ static NOINLINE int run_pipe(struct pipe *pi) goto must_fork; enter_var_nest_level(); - rcode = redirect_and_varexp_helper(&old_vars, command, &squirrel, argv_expanded); + /* Collect all variables "shadowed" by helper into old_vars list */ + G.shadowed_vars_pp = &old_vars; + rcode = redirect_and_varexp_helper(command, &squirrel, argv_expanded); + G.shadowed_vars_pp = sv_shadowed; + if (rcode == 0) { debug_printf_exec(": run_nofork_applet '%s' '%s'...\n", argv_expanded[0], argv_expanded[1]);