svn commit: trunk/busybox/shell
vda at busybox.net
vda at busybox.net
Tue Jun 17 05:11:43 UTC 2008
Author: vda
Date: 2008-06-16 22:11:43 -0700 (Mon, 16 Jun 2008)
New Revision: 22386
Log:
hush: fix memory leak. it was actually rather invloved problem.
Now finally glob/variable expansion is done IN THE RIGHT ORDER!
It opens up a possibility to cleanly fix remaining known bugs.
function old new delta
o_save_ptr 115 286 +171
o_save_ptr_helper - 115 +115
done_word 591 690 +99
o_get_last_ptr - 31 +31
expand_on_ifs 125 97 -28
add_string_to_strings 28 - -28
run_list 1895 1862 -33
debug_print_strings 42 - -42
add_strings_to_strings 126 - -126
expand_variables 1550 1394 -156
o_debug_list 168 - -168
expand_strvec_to_strvec 388 10 -378
------------------------------------------------------------------------------
(add/remove: 2/4 grow/shrink: 2/4 up/down: 416/-959) Total: -543 bytes
Modified:
trunk/busybox/shell/hush.c
Changeset:
Modified: trunk/busybox/shell/hush.c
===================================================================
--- trunk/busybox/shell/hush.c 2008-06-16 17:15:20 UTC (rev 22385)
+++ trunk/busybox/shell/hush.c 2008-06-17 05:11:43 UTC (rev 22386)
@@ -104,6 +104,8 @@
#define debug_printf_exec(...) do {} while (0)
#define debug_printf_jobs(...) do {} while (0)
#define debug_printf_expand(...) do {} while (0)
+#define debug_printf_glob(...) do {} while (0)
+#define debug_printf_list(...) do {} while (0)
#define debug_printf_clean(...) do {} while (0)
#ifndef debug_printf
@@ -120,14 +122,29 @@
#ifndef debug_printf_jobs
#define debug_printf_jobs(...) fprintf(stderr, __VA_ARGS__)
-#define DEBUG_SHELL_JOBS 1
+#define DEBUG_JOBS 1
+#else
+#define DEBUG_JOBS 0
#endif
#ifndef debug_printf_expand
#define debug_printf_expand(...) fprintf(stderr, __VA_ARGS__)
#define DEBUG_EXPAND 1
+#else
+#define DEBUG_EXPAND 0
#endif
+#ifndef debug_printf_glob
+#define debug_printf_glob(...) fprintf(stderr, __VA_ARGS__)
+#define DEBUG_GLOB 1
+#else
+#define DEBUG_GLOB 0
+#endif
+
+#ifndef debug_printf_list
+#define debug_printf_list(...) fprintf(stderr, __VA_ARGS__)
+#endif
+
/* Keep unconditionally on for now */
#define ENABLE_HUSH_DEBUG 1
@@ -143,7 +160,18 @@
#define DEBUG_CLEAN 1
#endif
+#if 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
+
/*
* Leak hunting. Use hush_leaktool.sh for post-processing.
*/
@@ -309,6 +337,7 @@
int length;
int maxlen;
smallint o_quote;
+ smallint o_glob;
smallint nonnull;
smallint has_empty_slot;
} o_string;
@@ -531,6 +560,18 @@
static void unset_local_var(const char *name);
+static int glob_needed(const char *s)
+{
+ while (*s) {
+ if (*s == '\\')
+ s++;
+ if (*s == '*' || *s == '[' || *s == '?')
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
static char **add_strings_to_strings(int need_xstrdup, char **strings, char **add)
{
int i;
@@ -592,18 +633,7 @@
}
#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);
static int builtin_echo(char **argv);
@@ -1260,9 +1290,35 @@
* o_finalize_list() operation post-processes this structure - calculates
* and stores actual char* ptrs in list[]. Oh, it NULL terminates it as well.
*/
-static int o_save_ptr(o_string *o, int n)
+#if DEBUG_EXPAND || DEBUG_GLOB
+static void debug_print_list(const char *prefix, o_string *o, int n)
{
char **list = (char**)o->data;
+ int string_start = ((n + 0xf) & ~0xf) * sizeof(list[0]);
+ int i = 0;
+ fprintf(stderr, "%s: list:%p n:%d string_start:%d length:%d maxlen:%d\n",
+ prefix, list, n, string_start, o->length, o->maxlen);
+ while (i < n) {
+ fprintf(stderr, " list[%d]=%d '%s' %p\n", i, (int)list[i],
+ o->data + (int)list[i] + string_start,
+ o->data + (int)list[i] + string_start);
+ i++;
+ }
+ if (n) {
+ const char *p = o->data + (int)list[n - 1] + string_start;
+ fprintf(stderr, " total_sz:%d\n", (p + strlen(p) + 1) - o->data);
+ }
+}
+#else
+#define debug_print_list(prefix, o, n) ((void)0)
+#endif
+
+/* n = o_save_ptr_helper(str, n) "starts new string" by storing an index value
+ * in list[n] so that it points past last stored byte so far.
+ * It returns n+1. */
+static int o_save_ptr_helper(o_string *o, int n)
+{
+ char **list = (char**)o->data;
int string_start;
int string_len;
@@ -1270,26 +1326,27 @@
string_start = ((n + 0xf) & ~0xf) * sizeof(list[0]);
string_len = o->length - string_start;
if (!(n & 0xf)) { /* 0, 0x10, 0x20...? */
- //bb_error_msg("list[%d]=%d string_start=%d (growing)", n, string_len, string_start);
+ debug_printf_list("list[%d]=%d string_start=%d (growing)", n, string_len, string_start);
/* list[n] points to string_start, make space for 16 more pointers */
o->maxlen += 0x10 * sizeof(list[0]);
o->data = xrealloc(o->data, o->maxlen + 1);
list = (char**)o->data;
memmove(list + n + 0x10, list + n, string_len);
o->length += 0x10 * sizeof(list[0]);
- }
- //else bb_error_msg("list[%d]=%d string_start=%d", n, string_len, string_start);
+ } else
+ debug_printf_list("list[%d]=%d string_start=%d", n, string_len, string_start);
} else {
/* We have empty slot at list[n], reuse without growth */
string_start = ((n+1 + 0xf) & ~0xf) * sizeof(list[0]); /* NB: n+1! */
string_len = o->length - string_start;
- //bb_error_msg("list[%d]=%d string_start=%d (empty slot)", n, string_len, string_start);
+ debug_printf_list("list[%d]=%d string_start=%d (empty slot)", n, string_len, string_start);
o->has_empty_slot = 0;
}
list[n] = (char*)string_len;
return n + 1;
}
+/* "What was our last o_save_ptr'ed position (byte offset relative o->data)?" */
static int o_get_last_ptr(o_string *o, int n)
{
char **list = (char**)o->data;
@@ -1298,14 +1355,89 @@
return ((int)list[n-1]) + string_start;
}
+/* Convert every \x to x in-place, return ptr past NUL. */
+static char *unbackslash(char *src)
+{
+ char *dst = src;
+ while (1) {
+ if (*src == '\\')
+ src++;
+ if ((*dst++ = *src++) == '\0')
+ break;
+ }
+ return dst;
+}
+
+static int o_glob(o_string *o, int n)
+{
+ glob_t globdata;
+ int gr;
+ char *pattern;
+
+ debug_printf_glob("start o_glob: n:%d o->data:%p", n, o->data);
+ if (!o->data)
+ return o_save_ptr_helper(o, n);
+ pattern = o->data + o_get_last_ptr(o, n);
+ debug_printf_glob("glob pattern '%s'", pattern);
+ if (!glob_needed(pattern)) {
+ literal:
+ o->length = unbackslash(pattern) - o->data;
+ debug_printf_glob("glob pattern '%s' is literal", pattern);
+ return o_save_ptr_helper(o, n);
+ }
+
+ memset(&globdata, 0, sizeof(globdata));
+ gr = glob(pattern, 0, NULL, &globdata);
+ debug_printf_glob("glob('%s'):%d\n", pattern, gr);
+ if (gr == GLOB_NOSPACE)
+ bb_error_msg_and_die("out of memory during glob");
+ if (gr == GLOB_NOMATCH) {
+ globfree(&globdata);
+ goto literal;
+ }
+ if (gr != 0) { /* GLOB_ABORTED ? */
+//TODO: testcase for bad glob pattern behavior
+ bb_error_msg("glob(3) error %d on '%s'", gr, pattern);
+ }
+ if (globdata.gl_pathv && globdata.gl_pathv[0]) {
+ char **argv = globdata.gl_pathv;
+ o->length = pattern - o->data; /* "forget" pattern */
+ while (1) {
+ o_addstr(o, *argv, strlen(*argv) + 1);
+ n = o_save_ptr_helper(o, n);
+ argv++;
+ if (!*argv)
+ break;
+ }
+ }
+ globfree(&globdata);
+ if (DEBUG_GLOB)
+ debug_print_list("o_glob returning", o, n);
+ return n;
+}
+
+/* o_save_ptr_helper + but glob the string so far remembered
+ * if o->o_glob == 1 */
+static int o_save_ptr(o_string *o, int n)
+{
+ if (o->o_glob)
+ return o_glob(o, n); /* o_save_ptr_helper is inside */
+ return o_save_ptr_helper(o, n);
+}
+
+/* "Please convert list[n] to real char* ptrs, and NULL terminate it." */
static char **o_finalize_list(o_string *o, int n)
{
- char **list = (char**)o->data;
+ char **list;
int string_start;
- o_save_ptr(o, n); /* force growth for list[n] if necessary */
- string_start = ((n+1 + 0xf) & ~0xf) * sizeof(list[0]);
- list[n] = NULL;
+ n = o_save_ptr(o, n); /* force growth for list[n] if necessary */
+ if (DEBUG_EXPAND)
+ debug_print_list("finalized", o, n);
+ debug_printf_expand("finalized n:%d", n);
+ list = (char**)o->data;
+ string_start = ((n + 0xf) & ~0xf) * sizeof(list[0]);
+ list[--n] = NULL;
while (n) {
n--;
list[n] = o->data + (int)list[n] + string_start;
@@ -1313,29 +1445,7 @@
return list;
}
-#ifdef DEBUG_EXPAND
-static void o_debug_list(const char *prefix, o_string *o, int n)
-{
- char **list = (char**)o->data;
- int string_start = ((n + 0xf) & ~0xf) * sizeof(list[0]);
- int i = 0;
- fprintf(stderr, "%s: list:%p n:%d string_start:%d length:%d maxlen:%d\n",
- prefix, list, n, string_start, o->length, o->maxlen);
- while (i < n) {
- fprintf(stderr, " list[%d]=%d '%s'\n", i, (int)list[i],
- o->data + (int)list[i] + string_start);
- i++;
- }
- if (n) {
- const char *p = o->data + (int)list[n] + string_start;
- fprintf(stderr, " total_sz:%d\n", (p + strlen(p) + 1) - o->data);
- }
-}
-#else
-#define o_debug_list(prefix, o, n) ((void)0)
-#endif
-
/*
* in_str support
*/
@@ -1782,7 +1892,7 @@
while ((childpid = waitpid(-1, &status, attributes)) > 0) {
const int dead = WIFEXITED(status) || WIFSIGNALED(status);
-#ifdef DEBUG_SHELL_JOBS
+#if DEBUG_JOBS
if (WIFSTOPPED(status))
debug_printf_jobs("pid %d stopped by sig %d (exitcode %d)\n",
childpid, WSTOPSIG(status), WEXITSTATUS(status));
@@ -2502,11 +2612,11 @@
if (!*str) /* EOL - do not finalize word */
break;
o_addchr(output, '\0');
- o_debug_list("expand_on_ifs", output, n);
+ debug_print_list("expand_on_ifs", output, n);
n = o_save_ptr(output, n);
str += strspn(str, ifs); /* skip ifs chars */
}
- o_debug_list("expand_on_ifs[1]", output, n);
+ debug_print_list("expand_on_ifs[1]", output, n);
return n;
}
@@ -2530,9 +2640,9 @@
ored_ch = 0;
debug_printf_expand("expand_vars_to_list: arg '%s'\n", arg);
- o_debug_list("expand_vars_to_list", output, n);
+ debug_print_list("expand_vars_to_list", output, n);
n = o_save_ptr(output, n);
- o_debug_list("expand_vars_to_list[0]", output, n);
+ debug_print_list("expand_vars_to_list[0]", output, n);
while ((p = strchr(arg, SPECIAL_VAR_SYMBOL)) != NULL) {
#if ENABLE_HUSH_TICK
@@ -2540,7 +2650,7 @@
#endif
o_addQstr(output, arg, p - arg);
- o_debug_list("expand_vars_to_list[1]", output, n);
+ debug_print_list("expand_vars_to_list[1]", output, n);
arg = ++p;
p = strchr(p, SPECIAL_VAR_SYMBOL);
@@ -2575,9 +2685,9 @@
/* this argv[] is not empty and not last:
* put terminating NUL, start new word */
o_addchr(output, '\0');
- o_debug_list("expand_vars_to_list[2]", output, n);
+ debug_print_list("expand_vars_to_list[2]", output, n);
n = o_save_ptr(output, n);
- o_debug_list("expand_vars_to_list[3]", output, n);
+ debug_print_list("expand_vars_to_list[3]", output, n);
}
}
} else
@@ -2589,7 +2699,7 @@
if (++i >= global_argc)
break;
o_addchr(output, '\0');
- o_debug_list("expand_vars_to_list[4]", output, n);
+ debug_print_list("expand_vars_to_list[4]", output, n);
n = o_save_ptr(output, n);
}
} else { /* quoted $*: add as one word */
@@ -2647,9 +2757,9 @@
} /* end of "while (SPECIAL_VAR_SYMBOL is found) ..." */
if (arg[0]) {
- o_debug_list("expand_vars_to_list[a]", output, n);
+ debug_print_list("expand_vars_to_list[a]", output, n);
o_addQstr(output, arg, strlen(arg) + 1);
- o_debug_list("expand_vars_to_list[b]", output, n);
+ debug_print_list("expand_vars_to_list[b]", output, n);
} else if (output->length == o_get_last_ptr(output, n) /* expansion is empty */
&& !(ored_ch & 0x80) /* and all vars were not quoted. */
) {
@@ -2662,106 +2772,33 @@
return n;
}
-static char **expand_variables(char **argv, char or_mask)
+static char **expand_variables(char **argv, int or_mask)
{
int n;
char **list;
char **v;
o_string output = NULL_O_STRING;
+ if (or_mask & 0x100)
+ output.o_glob = 1;
+
n = 0;
v = argv;
- while (*v)
- n = expand_vars_to_list(&output, n, *v++, or_mask);
- o_debug_list("expand_variables", &output, n);
+ while (*v) {
+ n = expand_vars_to_list(&output, n, *v, (char)or_mask);
+ v++;
+ }
+ debug_print_list("expand_variables", &output, n);
/* output.data (malloced in one block) gets returned in "list" */
list = o_finalize_list(&output, n);
+ debug_print_strings("expand_variables[1]", list);
return list;
}
-/* Remove non-backslashed backslashes and add to "strings" vector.
- * XXX broken if the last character is '\\', check that before calling.
- */
-static char **add_unq_string_to_strings(char **strings, const char *src)
-{
- int cnt;
- const char *s;
- char *v, *dest;
-
- for (cnt = 1, s = src; s && *s; s++) {
- if (*s == '\\') s++;
- cnt++;
- }
- v = dest = xmalloc(cnt);
- for (s = src; s && *s; s++, dest++) {
- if (*s == '\\') s++;
- *dest = *s;
- }
- *dest = '\0';
-
- return add_string_to_strings(strings, v);
-}
-
-/* XXX broken if the last character is '\\', check that before calling */
-static int glob_needed(const char *s)
-{
- for (; *s; s++) {
- if (*s == '\\')
- s++;
- if (strchr("*[?", *s))
- return 1;
- }
- return 0;
-}
-
-static void xglob(char ***pglob, const char *pattern)
-{
- if (glob_needed(pattern)) {
- glob_t globdata;
- int gr;
-
- memset(&globdata, 0, sizeof(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) {
- globfree(&globdata);
- goto literal;
- }
- if (gr != 0) { /* GLOB_ABORTED ? */
- bb_error_msg("glob(3) error %d on '%s'", gr, pattern);
-//TODO: testcase for bad glob pattern behavior
- }
- if (globdata.gl_pathv && globdata.gl_pathv[0])
- *pglob = add_strings_to_strings(1, *pglob, globdata.gl_pathv);
- globfree(&globdata);
- return;
- }
-
- literal:
- /* quote removal, or more accurately, backslash removal */
- *pglob = add_unq_string_to_strings(*pglob, pattern);
- debug_print_strings("after xglob", *pglob);
-}
-
-//LEAK is here: callers expect result to be free()able, but we
-//actually require free_strings(). free() leaks strings.
static char **expand_strvec_to_strvec(char **argv)
{
- char **exp;
- char **res = xzalloc(sizeof(res[0]));
-
- 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) {
- xglob(&res, *argv++);
- }
- free(exp);
- debug_print_strings("expand_strvec_to_strvec: res", res);
- return res;
+ return expand_variables(argv, 0x100);
}
/* used for expansion of right hand of assignments */
More information about the busybox-cvs
mailing list