[git commit] hush: fix bug where in "var=val func" var's value is not visible in func
Denys Vlasenko
vda.linux at googlemail.com
Sun May 3 22:14:30 UTC 2009
function old new delta
unset_local_var - 168 +168
set_vars_all_and_save_old - 87 +87
get_ptr_to_local_var - 77 +77
free_strings_and_unset - 53 +53
builtin_export 266 274 +8
get_local_var_value 31 33 +2
putenv_all 27 - -27
free_strings_and_unsetenv 53 - -53
get_local_var 68 - -68
run_list 2475 2350 -125
builtin_unset 380 220 -160
------------------------------------------------------------------------------
(add/remove: 4/3 grow/shrink: 2/2 up/down: 395/-433) Total: -38 bytes
Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
shell/hush.c | 199 ++++++++++++++------------
shell/hush_test/hush-bugs/env_and_func.right | 2 -
shell/hush_test/hush-bugs/env_and_func.tests | 6 -
shell/hush_test/hush-misc/env_and_func.right | 2 +
shell/hush_test/hush-misc/env_and_func.tests | 4 +
5 files changed, 115 insertions(+), 98 deletions(-)
delete mode 100644 shell/hush_test/hush-bugs/env_and_func.right
delete mode 100755 shell/hush_test/hush-bugs/env_and_func.tests
create mode 100644 shell/hush_test/hush-misc/env_and_func.right
create mode 100755 shell/hush_test/hush-misc/env_and_func.tests
diff --git a/shell/hush.c b/shell/hush.c
index c6e9405..0890f09 100644
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -56,7 +56,7 @@
*
* Licensed under the GPL v2 or later, see the file LICENSE in this tarball.
*/
-#include "busybox.h" /* for APPLET_IS_NOFORK/NOEXEC */
+#include "busybox.h" /* for APPLET_IS_NOFORK/NOEXEC */
#include <glob.h>
/* #include <dmalloc.h> */
#if ENABLE_HUSH_CASE
@@ -65,7 +65,7 @@
#include "math.h"
#include "match.h"
#ifndef PIPE_BUF
-# define PIPE_BUF 4096 /* amount of buffering in a pipe */
+# define PIPE_BUF 4096 /* amount of buffering in a pipe */
#endif
@@ -138,6 +138,8 @@
#define SPECIAL_VAR_SYMBOL 3
+struct variable;
+
static const char hush_version_str[] ALIGN1 = "HUSH_VERSION="BB_VER;
/* This supports saving pointers malloced in vfork child,
@@ -146,7 +148,7 @@ static const char hush_version_str[] ALIGN1 = "HUSH_VERSION="BB_VER;
#if !BB_MMU
typedef struct nommu_save_t {
char **new_env;
- char **old_env;
+ struct variable *old_vars;
char **argv;
char **argv_from_re_execing;
} nommu_save_t;
@@ -282,8 +284,8 @@ struct command {
#if ENABLE_HUSH_FUNCTIONS
# define GRP_FUNCTION 2
#endif
- struct pipe *group; /* if non-NULL, this "command" is { list },
- * ( list ), or a compound statement */
+ /* if non-NULL, this "command" is { list }, ( list ), or a compound statement */
+ struct pipe *group;
#if !BB_MMU
char *group_as_string;
#endif
@@ -883,6 +885,7 @@ static char **xx_add_strings_to_strings(int lineno, char **strings, char **add,
xx_add_strings_to_strings(__LINE__, strings, add, need_to_dup)
#endif
+/* Note: takes ownership of "add" ptr (it is not strdup'ed) */
static char **add_string_to_strings(char **strings, char *add)
{
char *v[2];
@@ -901,44 +904,9 @@ static char **xx_add_string_to_strings(int lineno, char **strings, char *add)
xx_add_string_to_strings(__LINE__, strings, add)
#endif
-static void putenv_all(char **strings)
-{
- if (!strings)
- return;
- while (*strings) {
- debug_printf_env("putenv '%s'\n", *strings);
- putenv(*strings++);
- }
-}
-
-static char **putenv_all_and_save_old(char **strings)
-{
- char **old = NULL;
- char **s = strings;
-
- if (!strings)
- return old;
- while (*strings) {
- char *v, *eq;
-
- eq = strchr(*strings, '=');
- if (eq) {
- *eq = '\0';
- v = getenv(*strings);
- *eq = '=';
- if (v) {
- /* v points to VAL in VAR=VAL, go back to VAR */
- v -= (eq - *strings) + 1;
- old = add_string_to_strings(old, v);
- }
- }
- strings++;
- }
- putenv_all(s);
- return old;
-}
+static int unset_local_var(const char *name);
-static void free_strings_and_unsetenv(char **strings, int unset)
+static void free_strings_and_unset(char **strings, int unset)
{
char **v;
@@ -948,8 +916,7 @@ static void free_strings_and_unsetenv(char **strings, int unset)
v = strings;
while (*v) {
if (unset) {
- debug_printf_env("unsetenv '%s'\n", *v);
- bb_unsetenv(*v);
+ unset_local_var(*v);
}
free(*v++);
}
@@ -958,7 +925,7 @@ static void free_strings_and_unsetenv(char **strings, int unset)
static void free_strings(char **strings)
{
- free_strings_and_unsetenv(strings, 0);
+ free_strings_and_unset(strings, 0);
}
@@ -1242,27 +1209,38 @@ static const char *set_cwd(void)
}
-/* Get/check local shell variables */
-static struct variable *get_local_var(const char *name)
+/*
+ * Shell and environment variable support
+ */
+static struct variable **get_ptr_to_local_var(const char *name)
{
+ struct variable **pp;
struct variable *cur;
int len;
- if (!name)
- return NULL;
len = strlen(name);
- for (cur = G.top_var; cur; cur = cur->next) {
+ pp = &G.top_var;
+ while ((cur = *pp) != NULL) {
if (strncmp(cur->varstr, name, len) == 0 && cur->varstr[len] == '=')
- return cur;
+ return pp;
+ pp = &cur->next;
}
return NULL;
}
-static const char *get_local_var_value(const char *src)
+static struct variable *get_local_var(const char *name)
{
- struct variable *var = get_local_var(src);
- if (var)
- return strchr(var->varstr, '=') + 1;
+ struct variable **pp = get_ptr_to_local_var(name);
+ if (pp)
+ return *pp;
+ return NULL;
+}
+
+static const char *get_local_var_value(const char *name)
+{
+ struct variable **pp = get_ptr_to_local_var(name);
+ if (pp)
+ return strchr((*pp)->varstr, '=') + 1;
return NULL;
}
@@ -1423,6 +1401,57 @@ static void arith_set_local_var(const char *name, const char *val, int flags)
/*
+ * Helpers for "var1=val1 var2=val2 cmd" feature
+ */
+static void add_vars(struct variable *var)
+{
+ struct variable *next;
+
+ while (var) {
+ next = var->next;
+ var->next = G.top_var;
+ G.top_var = var;
+ if (var->flg_export)
+ putenv(var->varstr);
+ var = next;
+ }
+}
+
+static struct variable *set_vars_all_and_save_old(char **strings)
+{
+ char **s;
+ struct variable *old = NULL;
+
+ if (!strings)
+ return old;
+ s = strings;
+ while (*s) {
+ struct variable *var_p;
+ struct variable **var_pp;
+ char *eq;
+
+ eq = strchr(*s, '=');
+ if (eq) {
+ *eq = '\0';
+ var_pp = get_ptr_to_local_var(*s);
+ *eq = '=';
+ if (var_pp) {
+ /* Remove variable from global linked list */
+ var_p = *var_pp;
+ *var_pp = var_p->next;
+ /* Add it to returned list */
+ var_p->next = old;
+ old = var_p;
+ }
+ set_local_var(*s, 1, 0);
+ }
+ s++;
+ }
+ return old;
+}
+
+
+/*
* in_str support
*/
static int static_get(struct in_str *i)
@@ -2855,17 +2884,17 @@ static struct function *new_function(char *name)
* body_as_string was not malloced! */
if (funcp->body) {
free_pipe_list(funcp->body);
-#if !BB_MMU
+# if !BB_MMU
free(funcp->body_as_string);
-#endif
+# endif
}
} else {
debug_printf_exec("reinserting in tree & replacing function '%s'\n", funcp->name);
cmd->argv[0] = funcp->name;
cmd->group = funcp->body;
-#if !BB_MMU
+# if !BB_MMU
cmd->group_as_string = funcp->body_as_string;
-#endif
+# endif
}
goto skip;
}
@@ -2892,9 +2921,9 @@ static void unset_func(const char *name)
* body_as_string was not malloced! */
if (funcp->body) {
free_pipe_list(funcp->body);
-#if !BB_MMU
+# if !BB_MMU
free(funcp->body_as_string);
-#endif
+# endif
}
free(funcp);
break;
@@ -2903,10 +2932,10 @@ static void unset_func(const char *name)
}
}
-#if BB_MMU
+# if BB_MMU
#define exec_function(nommu_save, funcp, argv) \
exec_function(funcp, argv)
-#endif
+# endif
static void exec_function(nommu_save_t *nommu_save,
const struct function *funcp,
char **argv) NORETURN;
@@ -2938,37 +2967,31 @@ static int run_function(const struct function *funcp, char **argv)
{
int rc;
save_arg_t sv;
-#if ENABLE_HUSH_FUNCTIONS
smallint sv_flg;
-#endif
save_and_replace_G_args(&sv, argv);
-#if ENABLE_HUSH_FUNCTIONS
/* "we are in function, ok to use return" */
sv_flg = G.flag_return_in_progress;
G.flag_return_in_progress = -1;
-#endif
/* On MMU, funcp->body is always non-NULL */
-#if !BB_MMU
+# if !BB_MMU
if (!funcp->body) {
/* Function defined by -F */
parse_and_run_string(funcp->body_as_string);
rc = G.last_exitcode;
} else
-#endif
+# endif
{
rc = run_list(funcp->body);
}
-#if ENABLE_HUSH_FUNCTIONS
G.flag_return_in_progress = sv_flg;
-#endif
restore_G_args(&sv, argv);
return rc;
}
-#endif
+#endif /* ENABLE_HUSH_FUNCTIONS */
#if BB_MMU
@@ -2998,11 +3021,13 @@ static void pseudo_exec_argv(nommu_save_t *nommu_save,
new_env = expand_assignments(argv, assignment_cnt);
#if BB_MMU
- putenv_all(new_env);
+ set_vars_all_and_save_old(new_env);
free(new_env); /* optional */
+ /* we can also destroy set_vars_all_and_save_old's return value,
+ * to save memory */
#else
nommu_save->new_env = new_env;
- nommu_save->old_env = putenv_all_and_save_old(new_env);
+ nommu_save->old_vars = set_vars_all_and_save_old(new_env);
#endif
if (argv_expanded) {
argv = argv_expanded;
@@ -3443,10 +3468,10 @@ static int run_pipe(struct pipe *pi)
funcp = new_function(command->argv[0]);
/* funcp->name is already set to argv[0] */
funcp->body = command->group;
-#if !BB_MMU
+# if !BB_MMU
funcp->body_as_string = command->group_as_string;
command->group_as_string = NULL;
-#endif
+# endif
command->group = NULL;
command->argv[0] = NULL;
debug_printf_exec("cmd %p has child func at %p\n", command, funcp);
@@ -3481,7 +3506,7 @@ static int run_pipe(struct pipe *pi)
enum { funcp = 0 };
#endif
char **new_env = NULL;
- char **old_env = NULL;
+ struct variable *old_vars = NULL;
if (argv[command->assignment_cnt] == NULL) {
/* Assignments, but no command */
@@ -3529,7 +3554,7 @@ static int run_pipe(struct pipe *pi)
rcode = setup_redirects(command, squirrel);
if (rcode == 0) {
new_env = expand_assignments(argv, command->assignment_cnt);
- old_env = putenv_all_and_save_old(new_env);
+ old_vars = set_vars_all_and_save_old(new_env);
if (!funcp) {
debug_printf_exec(": builtin '%s' '%s'...\n",
x->cmd, argv_expanded[1]);
@@ -3548,11 +3573,8 @@ static int run_pipe(struct pipe *pi)
clean_up_and_ret:
#endif
restore_redirects(squirrel);
- free_strings_and_unsetenv(new_env, 1);
- putenv_all(old_env);
- /* Free the pointers, but the strings themselves
- * are in environ now, don't use free_strings! */
- free(old_env);
+ free_strings_and_unset(new_env, 1);
+ add_vars(old_vars);
clean_up_and_ret1:
free(argv_expanded);
IF_HAS_KEYWORDS(if (pi->pi_inverted) rcode = !rcode;)
@@ -3567,7 +3589,7 @@ static int run_pipe(struct pipe *pi)
rcode = setup_redirects(command, squirrel);
if (rcode == 0) {
new_env = expand_assignments(argv, command->assignment_cnt);
- old_env = putenv_all_and_save_old(new_env);
+ old_vars = set_vars_all_and_save_old(new_env);
debug_printf_exec(": run_nofork_applet '%s' '%s'...\n",
argv_expanded[0], argv_expanded[1]);
rcode = run_nofork_applet(i, argv_expanded);
@@ -3592,7 +3614,7 @@ static int run_pipe(struct pipe *pi)
#if !BB_MMU
volatile nommu_save_t nommu_save;
nommu_save.new_env = NULL;
- nommu_save.old_env = NULL;
+ nommu_save.old_vars = NULL;
nommu_save.argv = NULL;
nommu_save.argv_from_re_execing = NULL;
#endif
@@ -3665,11 +3687,8 @@ static int run_pipe(struct pipe *pi)
/* Clean up after vforked child */
free(nommu_save.argv);
free(nommu_save.argv_from_re_execing);
- free_strings_and_unsetenv(nommu_save.new_env, 1);
- putenv_all(nommu_save.old_env);
- /* Free the pointers, but the strings themselves
- * are in environ now, don't use free_strings! */
- free(nommu_save.old_env);
+ free_strings_and_unset(nommu_save.new_env, 1);
+ add_vars(nommu_save.old_vars);
#endif
free(argv_expanded);
argv_expanded = NULL;
diff --git a/shell/hush_test/hush-bugs/env_and_func.right b/shell/hush_test/hush-bugs/env_and_func.right
deleted file mode 100644
index 4a15450..0000000
--- a/shell/hush_test/hush-bugs/env_and_func.right
+++ /dev/null
@@ -1,2 +0,0 @@
-var=val
-var=old
diff --git a/shell/hush_test/hush-bugs/env_and_func.tests b/shell/hush_test/hush-bugs/env_and_func.tests
deleted file mode 100755
index d62c1af..0000000
--- a/shell/hush_test/hush-bugs/env_and_func.tests
+++ /dev/null
@@ -1,6 +0,0 @@
-# UNFIXED BUG
-
-var=old
-f() { echo "var=$var"; }
-var=val f
-echo "var=$var"
diff --git a/shell/hush_test/hush-misc/env_and_func.right b/shell/hush_test/hush-misc/env_and_func.right
new file mode 100644
index 0000000..4a15450
--- /dev/null
+++ b/shell/hush_test/hush-misc/env_and_func.right
@@ -0,0 +1,2 @@
+var=val
+var=old
diff --git a/shell/hush_test/hush-misc/env_and_func.tests b/shell/hush_test/hush-misc/env_and_func.tests
new file mode 100755
index 0000000..1d4eaf3
--- /dev/null
+++ b/shell/hush_test/hush-misc/env_and_func.tests
@@ -0,0 +1,4 @@
+var=old
+f() { echo "var=$var"; }
+var=val f
+echo "var=$var"
--
1.6.0.6
More information about the busybox-cvs
mailing list