diff options
| -rw-r--r-- | shell/README | 4 | ||||
| -rw-r--r-- | shell/hush.c | 330 | ||||
| -rw-r--r-- | shell/hush_test/hush-z_slow/leak_var.right | 2 | ||||
| -rwxr-xr-x | shell/hush_test/hush-z_slow/leak_var.tests | 69 |
4 files changed, 238 insertions, 167 deletions
diff --git a/shell/README b/shell/README index a9b4346..a09353d 100644 --- a/shell/README +++ b/shell/README @@ -1,6 +1,10 @@ Various bits of what is known about busybox shells, in no particular order. 2007-05-24 +hush: environment-related memory leak plugged, with net code size +decrease. + +2007-05-24 hush: '( echo ${name )' will show syntax error message, but prompt doesn't return (need to press <enter>). Pressing Ctrl-C, <enter>, '( echo ${name )' again, Ctrl-C segfaults. diff --git a/shell/hush.c b/shell/hush.c index d96615d..0b8e7d6 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -37,7 +37,6 @@ * across continuation lines. * * Bash grammar not implemented: (how many of these were in original sh?) - * $@ (those sure look like weird quoting rules) * $_ * ! negation operator for pipes * &> and >& redirection of stdout+stderr @@ -279,11 +278,18 @@ struct close_me { int fd; }; -struct variables { - struct variables *next; - const char *name; - const char *value; - smallint flg_export; +/* On program start, environ points to initial environment. + * putenv adds new pointers into it, unsetenv removes them. + * Neither of these (de)allocates the strings. + * setenv allocates new strings in malloc space and does putenv, + * and thus setenv is unusable (leaky) for shell's purposes */ +#define setenv(...) setenv_is_leaky_dont_use() +struct variable { + struct variable *next; + char *name; /* points to "name=" portion */ + char *value; /* points directly after "=" */ + int max_len; /* if > 0, name is part of initial env; else name is malloced */ + smallint flg_export; /* putenv should be done on this var */ smallint flg_read_only; }; @@ -322,14 +328,21 @@ enum { CHAR_SPECIAL = 3, /* example: $ */ }; +#define HUSH_VER_STR "0.02" -/* "Globals" within this file */ +static const char version_str[] = "HUSH_VERSION="HUSH_VER_STR; -#define HUSH_VER_STR "0.02" -static const struct variables const_shell_ver = { - NULL, "HUSH_VERSION", HUSH_VER_STR, 1, 1 +static const struct variable const_shell_ver = { + .next = NULL, + .name = (char*)version_str, + .value = (char*)version_str + sizeof("HUSH_VERSION=")-1, + .max_len = 1, /* 0 can provoke free(name) */ + .flg_export = 1, + .flg_read_only = 1, }; +/* "Globals" within this file */ + /* Sorted roughly by size (smaller offsets == smaller code) */ struct globals { #if ENABLE_HUSH_INTERACTIVE @@ -360,8 +373,8 @@ struct globals { struct close_me *close_me_head; const char *cwd; unsigned last_bg_pid; - struct variables *top_vars; /* = &shell_ver (both are set in main()) */ - struct variables shell_ver; /* = const_shell_ver */ + struct variable *top_var; /* = &shell_ver (both are set in main()) */ + struct variable shell_ver; /* = const_shell_ver */ #if ENABLE_FEATURE_SH_STANDALONE struct nofork_save_area nofork_save; #endif @@ -407,7 +420,7 @@ enum { run_list_level = 0 }; #define close_me_head (G.close_me_head ) #define cwd (G.cwd ) #define last_bg_pid (G.last_bg_pid ) -#define top_vars (G.top_vars ) +#define top_var (G.top_var ) #define shell_ver (G.shell_ver ) #if ENABLE_FEATURE_SH_STANDALONE #define nofork_save (G.nofork_save ) @@ -541,8 +554,8 @@ static char **expand_strvec_to_strvec(char **argv); static char *expand_strvec_to_string(char **argv); /* used for expansion of right hand of assignments */ static char *expand_string_to_string(const char *str); -static const char *get_local_var(const char *var); -static int set_local_var(const char *s, int flg_export); +static struct variable *get_local_var(const char *var); +static int set_local_var(char *s, int flg_export); static void unset_local_var(const char *name); /* Table of built-in functions. They can be forked or not, depending on @@ -795,7 +808,7 @@ static int builtin_exit(char **argv) /* built-in 'export VAR=value' handler */ static int builtin_export(char **argv) { - int res = 0; + const char *value; char *name = argv[1]; if (name == NULL) { @@ -810,36 +823,23 @@ static int builtin_export(char **argv) return EXIT_SUCCESS; } - name = xstrdup(name); - { - const char *value = strchr(name, '='); - - if (!value) { - char *tmp; - /* They are exporting something without an =VALUE */ - - value = get_local_var(name); - if (value) { - size_t ln = strlen(name); + value = strchr(name, '='); + if (!value) { + /* They are exporting something without a =VALUE */ + struct variable *var; - tmp = xrealloc(name, ln+strlen(value)+2); - sprintf(tmp+ln, "=%s", value); - name = tmp; - } else { - /* bash does not return an error when trying to export - * an undefined variable. Do likewise. */ - res = 1; - } + var = get_local_var(name); + if (var) { + var->flg_export = 1; + putenv(var->name); } + /* bash does not return an error when trying to export + * an undefined variable. Do likewise. */ + return EXIT_SUCCESS; } - if (res < 0) - bb_perror_msg("export"); - else if (res == 0) - res = set_local_var(name, 1); - else - res = 0; - free(name); - return res; + + set_local_var(xstrdup(name), 1); + return EXIT_SUCCESS; } #if ENABLE_HUSH_JOB @@ -965,20 +965,20 @@ static int builtin_read(char **argv) /* read string. name_len+1 chars are already used by 'name=' */ fgets(p, sizeof(string) - 1 - name_len, stdin); chomp(p); - return set_local_var(string, 0); + return set_local_var(xstrdup(string), 0); } /* built-in 'set [VAR=value]' handler */ static int builtin_set(char **argv) { char *temp = argv[1]; - struct variables *e; + struct variable *e; if (temp == NULL) - for (e = top_vars; e; e = e->next) - printf("%s=%s\n", e->name, e->value); + for (e = top_var; e; e = e->next) + puts(e->name); else - set_local_var(temp, 0); + set_local_var(xstrdup(temp), 0); return EXIT_SUCCESS; } @@ -1742,26 +1742,9 @@ static int run_pipe_real(struct pipe *pi) if (i != 0 && argv[i] == NULL) { /* assignments, but no command: set the local environment */ for (i = 0; argv[i] != NULL; i++) { - /* Ok, this case is tricky. We have to decide if this is a - * local variable, or an already exported variable. If it is - * already exported, we have to export the new value. If it is - * not exported, we need only set this as a local variable. - * This junk is all to decide whether or not to export this - * variable. */ - int export_me = 0; - char *name, *value; - name = xstrdup(argv[i]); - debug_printf("local environment set: %s\n", name); - value = strchr(name, '='); - if (value) - *value = '\0'; - if (get_local_var(name)) { - export_me = 1; - } - free(name); + debug_printf("local environment set: %s\n", argv[i]); p = expand_string_to_string(argv[i]); - set_local_var(p, export_me); - free(p); + set_local_var(p, 0); } return EXIT_SUCCESS; /* don't worry about errors in set_local_var() yet */ } @@ -2693,109 +2676,115 @@ static char* expand_strvec_to_string(char **argv) } /* This is used to get/check local shell variables */ -static const char *get_local_var(const char *s) +static struct variable *get_local_var(const char *s) { - struct variables *cur; + struct variable *cur; + int len; if (!s) return NULL; - for (cur = top_vars; cur; cur = cur->next) { - if (strcmp(cur->name, s) == 0) - return cur->value; + len = strlen(s); + for (cur = top_var; cur; cur = cur->next) { + if (strncmp(cur->name, s, len) == 0 && cur->name[len] == '=') + return cur; } return NULL; } -/* This is used to set local shell variables - flg_export == 0 if only local (not exporting) variable - flg_export == 1 if "new" exporting environ - flg_export > 1 if current startup environ (not call putenv()) */ -static int set_local_var(const char *s, int flg_export) +/* name holds "NAME=VAL" and is expected to be malloced. + * We take ownership of it. */ +static int set_local_var(char *name, int flg_export) { - char *name, *value; - int result = 0; - struct variables *cur; - - name = xstrdup(s); + struct variable *cur; + char *value; + int name_len; - /* Assume when we enter this function that we are already in - * NAME=VALUE format. So the first order of business is to - * split 's' on the '=' into 'name' and 'value' */ value = strchr(name, '='); - /*if (value == 0 && ++value == 0) ??? -vda */ - if (value == NULL || value[1] == '\0') { + if (!value) { /* not expected to ever happen? */ free(name); return -1; } - *value++ = '\0'; - for (cur = top_vars; cur; cur = cur->next) { - if (strcmp(cur->name, name) == 0) { - if (strcmp(cur->value, value) == 0) { - if (flg_export && !cur->flg_export) - cur->flg_export = flg_export; - else - result++; - } else if (cur->flg_read_only) { - bb_error_msg("%s: readonly variable", name); - result = -1; - } else { - if (flg_export > 0 || cur->flg_export > 1) - cur->flg_export = 1; - free((char*)cur->value); - cur->value = xstrdup(value); + name_len = value - name; + cur = top_var; /* cannot be NULL (we have HUSH_VERSION and it's RO) */ + while (1) { + if (strncmp(cur->name, name, name_len) != 0 || cur->name[name_len] != '=') { + if (!cur->next) { + /* cur points to last var in linked list */ + break; } - goto skip; + cur = cur->next; + continue; } - } - - cur = xzalloc(sizeof(*cur)); - /*cur->next = 0;*/ - cur->name = xstrdup(name); - cur->value = xstrdup(value); - cur->flg_export = flg_export; - /*cur->flg_read_only = 0;*/ - { - struct variables *bottom = top_vars; - while (bottom->next) - bottom = bottom->next; - bottom->next = cur; - } - skip: - if (result == 0 && cur->flg_export == 1) { - *(value-1) = '='; - result = putenv(name); - } else { - free(name); - if (result > 0) /* equivalent to previous set */ - result = 0; - } - return result; + /* We already have a var with this name */ + if (cur->flg_read_only) { + bb_error_msg("%s: readonly variable", name); + free(name); + return -1; + } + *value = '\0'; + unsetenv(name); /* just in case */ + *value++ = '='; + if (strcmp(cur->value, value) == 0) { + free_and_exp: + free(name); + goto exp; + } + if (cur->max_len >= strlen(name)) { + /* This one is from startup env, reuse space */ + strcpy(cur->name, name); + goto free_and_exp; + } + /* max_len == 0 signifies "malloced" var, which we can + * (and has to) free */ + if (!cur->max_len) + free(cur->name); + cur->max_len = 0; + goto set_name_and_exp; + } + + /* Not found - create next variable struct */ + cur->next = xzalloc(sizeof(*cur)); + cur = cur->next; + + set_name_and_exp: + cur->name = name; + exp: + cur->value = cur->name + name_len + 1; + if (flg_export) + cur->flg_export = 1; + if (cur->flg_export) + return putenv(cur->name); + return 0; } static void unset_local_var(const char *name) { - struct variables *cur, *next; + struct variable *cur; + struct variable *prev = prev; /* for gcc */ + int name_len; if (!name) return; - for (cur = top_vars; cur; cur = cur->next) { - if (strcmp(cur->name, name) == 0) { + name_len = strlen(name); + cur = top_var; + while (cur) { + if (strncmp(cur->name, name, name_len) == 0 && cur->name[name_len] == '=') { if (cur->flg_read_only) { bb_error_msg("%s: readonly variable", name); return; } - if (cur->flg_export) - unsetenv(cur->name); - free((char*)cur->name); - free((char*)cur->value); - next = top_vars; - while (next->next != cur) - next = next->next; - next->next = cur->next; + /* prev is ok to use here because 1st variable, HUSH_VERSION, + * is ro, and we cannot reach this code on the 1st pass */ + prev->next = cur->next; + unsetenv(cur->name); + if (!cur->max_len) + free(cur->name); free(cur); return; } + prev = cur; + cur = cur->next; } } @@ -3265,13 +3254,10 @@ static int parse_group(o_string *dest, struct p_context *ctx, * see the bash man page under "Parameter Expansion" */ static const char *lookup_param(const char *src) { - const char *p = NULL; - if (src) { - p = getenv(src); - if (!p) - p = get_local_var(src); - } - return p; + struct variable *var = get_local_var(src); + if (var) + return var->value; + return NULL; } /* return code: 0 for OK, 1 for syntax error */ @@ -3681,10 +3667,29 @@ int hush_main(int argc, char **argv) int opt; FILE *input; char **e; + struct variable *cur_var; PTR_TO_GLOBALS = xzalloc(sizeof(G)); - top_vars = &shell_ver; + shell_ver = const_shell_ver; /* copying struct here */ + top_var = &shell_ver; + /* initialize our shell local variables with the values + * currently living in the environment */ + e = environ; + cur_var = top_var; + if (e) while (*e) { + char *value = strchr(*e, '='); + if (value) { /* paranoia */ + cur_var->next = xzalloc(sizeof(*cur_var)); + cur_var = cur_var->next; + cur_var->name = *e; + cur_var->value = value + 1; + cur_var->max_len = strlen(*e); + cur_var->flg_export = 1; + } + e++; + } + putenv(shell_ver.name); #if ENABLE_FEATURE_EDITING line_input_state = new_line_input_t(FOR_SHELL); @@ -3701,14 +3706,8 @@ int hush_main(int argc, char **argv) PS2 = "> "; #endif - /* initialize our shell local variables with the values - * currently living in the environment */ - e = environ; - if (e) - while (*e) - set_local_var(*e++, 2); /* without call putenv() */ - - last_return_code = EXIT_SUCCESS; + if (EXIT_SUCCESS) /* otherwise is already done */ + last_return_code = EXIT_SUCCESS; if (argv[0] && argv[0][0] == '-') { debug_printf("sourcing /etc/profile\n"); @@ -3818,23 +3817,20 @@ int hush_main(int argc, char **argv) input = xfopen(argv[optind], "r"); opt = parse_and_run_file(input); + final_return: + #if ENABLE_FEATURE_CLEAN_UP fclose(input); if (cwd != bb_msg_unknown) free((char*)cwd); - { - struct variables *cur, *tmp; - for (cur = top_vars; cur; cur = tmp) { - tmp = cur->next; - if (!cur->flg_read_only) { - free((char*)cur->name); - free((char*)cur->value); - free(cur); - } - } + cur_var = top_var->next; + while (cur_var) { + struct variable *tmp = cur_var; + if (!cur_var->max_len) + free(cur_var->name); + cur_var = cur_var->next; + free(tmp); } #endif - - final_return: hush_exit(opt ? opt : last_return_code); } diff --git a/shell/hush_test/hush-z_slow/leak_var.right b/shell/hush_test/hush-z_slow/leak_var.right new file mode 100644 index 0000000..7bccc1e --- /dev/null +++ b/shell/hush_test/hush-z_slow/leak_var.right @@ -0,0 +1,2 @@ +Measuring memory leak... +vsz does not grow diff --git a/shell/hush_test/hush-z_slow/leak_var.tests b/shell/hush_test/hush-z_slow/leak_var.tests new file mode 100755 index 0000000..d3ca259 --- /dev/null +++ b/shell/hush_test/hush-z_slow/leak_var.tests @@ -0,0 +1,69 @@ +pid=$$ + +# Warm up +unset t +t=111111111111111111111111111111111111111111111111111111111111111111111111 +export t +unset t +t=111111111111111111111111111111111111111111111111111111111111111111111111 +export t +unset t +t=111111111111111111111111111111111111111111111111111111111111111111111111 +export t +unset t +t=111111111111111111111111111111111111111111111111111111111111111111111111 +export t +unset t +t=111111111111111111111111111111111111111111111111111111111111111111111111 +export t +i=1 +if test $i = 1111111111111111111111111111111111111111111111; then i=2; fi +beg=`ps -o pid,vsz | grep "^ *$pid "` + +echo "Measuring memory leak..." +beg=`ps -o pid,vsz | grep "^ *$pid "` +i=1 +while test $i != X; do + unset t + t=111111111111111111111111111111111111111111111111111111111111111111111111 + export t + unset t + t=111111111111111111111111111111111111111111111111111111111111111111111111 + export t + unset t + t=111111111111111111111111111111111111111111111111111111111111111111111111 + export t + unset t + t=111111111111111111111111111111111111111111111111111111111111111111111111 + export t + unset t + t=111111111111111111111111111111111111111111111111111111111111111111111111 + export t + i=1$i + if test $i = 1111111111111111111111111111111111111111111111; then i=2; fi + if test $i = 1111111111111111111111111111111111111111111112; then i=3; fi + if test $i = 1111111111111111111111111111111111111111111113; then i=4; fi + if test $i = 1111111111111111111111111111111111111111111114; then i=5; fi + if test $i = 1111111111111111111111111111111111111111111115; then i=6; fi + if test $i = 1111111111111111111111111111111111111111111116; then i=7; fi + if test $i = 1111111111111111111111111111111111111111111117; then i=8; fi + if test $i = 1111111111111111111111111111111111111111111118; then i=9; fi + if test $i = 1111111111111111111111111111111111111111111119; then i=a; fi + if test $i = 111111111111111111111111111111111111111111111a; then i=b; fi + if test $i = 111111111111111111111111111111111111111111111b; then i=c; fi + if test $i = 111111111111111111111111111111111111111111111c; then i=d; fi + if test $i = 111111111111111111111111111111111111111111111d; then i=e; fi + if test $i = 111111111111111111111111111111111111111111111e; then i=f; fi + if test $i = 111111111111111111111111111111111111111111111f; then i=g; fi + if test $i = 111111111111111111111111111111111111111111111g; then i=h; fi + if test $i = 111111111111111111111111111111111111111111111h; then i=i; fi + if test $i = 111111111111111111111111111111111111111111111i; then i=j; fi + if test $i = 111111111111111111111111111111111111111111111j; then i=X; fi +done +end=`ps -o pid,vsz | grep "^ *$pid "` + +if test "$beg" != "$end"; then + echo "vsz grows: $beg -> $end" +else + echo "vsz does not grow" +fi |