hush: fixing fallout from last big glob fix:

 fix segfault; identify where we leak memory

function                                             old     new   delta
expand_strvec_to_strvec                              353     336     -17

1 files changed, 80 insertions, 80 deletions

diff --git a/shell/hush.c b/shell/hush.c
index 7c5a44274..0e1976443 100644
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -148,6 +148,11 @@ static const char *indenter(int i)
  * Leak hunting. Use hush_leaktool.sh for post-processing.
  */
 #ifdef FOR_HUSH_LEAKTOOL
+/* suppress "warning: no previous prototype..." */
+void *xxmalloc(int lineno, size_t size);
+void *xxrealloc(int lineno, void *ptr, size_t size);
+char *xxstrdup(int lineno, const char *str);
+void xxfree(void *ptr);
 void *xxmalloc(int lineno, size_t size)
 {
         void *ptr = xmalloc((size + 0xff) & ~0xff);
@@ -2474,72 +2479,6 @@ static int run_and_free_list(struct pipe *pi)
         return rcode;
 }
 
-/* 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 int 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", gr);
-                }
-                if (globdata.gl_pathv && globdata.gl_pathv[0])
-                        *pglob = add_strings_to_strings(1, *pglob, globdata.gl_pathv);
-                globfree(&globdata);
-                return gr;
-        }
-
- literal:
-        /* quote removal, or more accurately, backslash removal */
-        *pglob = add_unq_string_to_strings(*pglob, pattern);
-        debug_print_strings("after xglob", *pglob);
-        return 0;
-}
-
 /* expand_strvec_to_strvec() takes a list of strings, expands
  * all variable references within and returns a pointer to
  * a list of expanded strings, possibly with larger number
@@ -2596,7 +2535,9 @@ static int expand_vars_to_list(o_string *output, int n, char *arg, char or_mask)
         o_debug_list("expand_vars_to_list[0]", output, n);
 
         while ((p = strchr(arg, SPECIAL_VAR_SYMBOL)) != NULL) {
+#if ENABLE_HUSH_TICK
                 o_string subst_result = NULL_O_STRING;
+#endif
 
                 o_addQstr(output, arg, p - arg);
                 o_debug_list("expand_vars_to_list[1]", output, n);
@@ -2661,10 +2602,12 @@ static int expand_vars_to_list(o_string *output, int n, char *arg, char or_mask)
                                 }
                         }
                         break;
+#if ENABLE_HUSH_TICK
                 case '`': {
                         struct in_str input;
                         *p = '\0';
                         arg++;
+//TODO: can we just stuff it into "output" directly?
                         //bb_error_msg("SUBST '%s' first_ch %x", arg, first_ch);
                         setup_string_in_str(&input, arg);
                         process_command_subs(&subst_result, &input, NULL);
@@ -2672,6 +2615,7 @@ static int expand_vars_to_list(o_string *output, int n, char *arg, char or_mask)
                         val = subst_result.data;
                         goto store_val;
                 }
+#endif
                 default:
                         *p = '\0';
                         arg[0] = first_ch & 0x7f;
@@ -2696,7 +2640,9 @@ static int expand_vars_to_list(o_string *output, int n, char *arg, char or_mask)
                         o_addQstr(output, val, strlen(val));
                 }
 
+#if ENABLE_HUSH_TICK
                 o_free(&subst_result);
+#endif
                 arg = ++p;
         } /* end of "while (SPECIAL_VAR_SYMBOL is found) ..." */
 
@@ -2731,33 +2677,87 @@ static char **expand_variables(char **argv, char or_mask)
 
         /* output.data (malloced in one block) gets returned in "list" */
         list = o_finalize_list(&output, n);
+        return list;
+}
 
-#ifdef DEBUG_EXPAND
-        {
-                int m = 0;
-                while (m <= n) {
-                        debug_printf_expand("list[%d]=%p '%s'\n", m, list[m], list[m]);
-                        m++;
+/* 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;
         }
-#endif
-        return list;
+
+ 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 = NULL;
+        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) {
-                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++;
+                xglob(&res, *argv++);
         }
         free(exp);
         debug_print_strings("expand_strvec_to_strvec: res", res);