hush: fix "unterminated last line loops forever" bug

hush: add testsuite infrastructure

diff --git a/shell/hush.c b/shell/hush.c
index 8ffb117deb04b9ae193e6792219424d0966883ca..32cd65c725f3016b4b6d3efaf9e10988b1ff3f0e 100644 (file)
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -317,8 +317,10 @@ typedef struct {
 /* I can almost use ordinary FILE *.  Is open_memstream() universally
  * available?  Where is it documented? */
 struct in_str {
-       const char *p;
-       char peek_buf[2];
+       union {
+               const char *p;
+               int cached_ch;
+       };
 #if ENABLE_HUSH_INTERACTIVE
        int __promptme;
        int promptmode;
@@ -1112,8 +1114,10 @@ static int file_get(struct in_str *i)
 
        ch = 0;
        /* If there is data waiting, eat it up */
-       if (i->p && *i->p) {
-               ch = *i->p++;
+       if (i->cached_ch) {
+               ch = i->cached_ch ^ 0x100;
+               if (ch != EOF)
+                       i->cached_ch = 0;
        } else {
                /* need to double check i->file because we might be doing something
                 * more complicated by now, like sourcing or substituting. */
@@ -1133,7 +1137,7 @@ static int file_get(struct in_str *i)
                {
                        ch = fgetc(i->file);
                }
-               debug_printf("b_getch: got a %d\n", ch);
+               debug_printf("file_get: got a %d\n", ch);
        }
 #if ENABLE_HUSH_INTERACTIVE
        if (ch == '\n')
@@ -1147,14 +1151,14 @@ static int file_get(struct in_str *i)
  */
 static int file_peek(struct in_str *i)
 {
-       if (i->p && *i->p) {
-               return *i->p;
+       int ch;
+       if (i->cached_ch) {
+               return i->cached_ch ^ 0x100;
        }
-       i->peek_buf[0] = fgetc(i->file);
-       i->peek_buf[1] = '\0';
-       i->p = i->peek_buf;
-       debug_printf("b_peek: got a %d\n", *i->p);
-       return *i->p;
+       ch = fgetc(i->file);
+       i->cached_ch = ch ^ 0x100; /* ^ 0x100 so that it is never 0 */
+       debug_printf("file_peek: got a %d '%c'\n", ch, ch);
+       return ch;
 }
 
 static void setup_file_in_str(struct in_str *i, FILE *f)
@@ -1670,10 +1674,10 @@ static int run_pipe_real(struct pipe *pi)
                                int export_me = 0;
                                char *name, *value;
                                name = xstrdup(argv[i]);
-                               debug_printf("Local environment set: %s\n", name);
+                               debug_printf("local environment set: %s\n", name);
                                value = strchr(name, '=');
                                if (value)
-                                       *value = 0;
+                                       *value = '\0';
                                if (get_local_var(name)) {
                                        export_me = 1;
                                }
@@ -2364,9 +2368,10 @@ static const char *get_local_var(const char *s)
 
        if (!s)
                return NULL;
-       for (cur = top_vars; cur; cur = cur->next)
+       for (cur = top_vars; cur; cur = cur->next) {
                if (strcmp(cur->name, s) == 0)
                        return cur->value;
+       }
        return NULL;
 }
 
@@ -2380,7 +2385,7 @@ static int set_local_var(const char *s, int flg_export)
        int result = 0;
        struct variables *cur;
 
-       name = strdup(s);
+       name = xstrdup(s);
 
        /* Assume when we enter this function that we are already in
         * NAME=VALUE format.  So the first order of business is to
@@ -2394,48 +2399,46 @@ static int set_local_var(const char *s, int flg_export)
        *value++ = '\0';
 
        for (cur = top_vars; cur; cur = cur->next) {
-               if (strcmp(cur->name, name) == 0)
-                       break;
-       }
-
-       if (cur) {
-               if (strcmp(cur->value, value) == 0) {
-                       if (flg_export > 0 && cur->flg_export == 0)
-                               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 = strdup(value);
-               }
-       } else {
-               cur = malloc(sizeof(struct variables));
-               if (!cur) {
-                       result = -1;
-               } else {
-                       cur->name = strdup(name);
-                       if (!cur->name) {
-                               free(cur);
+               if (strcmp(cur->name, name) == 0) {
+                       if (strcmp(cur->value, value) == 0) {
+                               if (flg_export > 0 && cur->flg_export == 0)
+                                       cur->flg_export = flg_export;
+                               else
+                                       result++;
+                       } else if (cur->flg_read_only) {
+                               bb_error_msg("%s: readonly variable", name);
                                result = -1;
                        } else {
-                               struct variables *bottom = top_vars;
+                               if (flg_export > 0 || cur->flg_export > 1)
+                                       cur->flg_export = 1;
+                               free((char*)cur->value);
                                cur->value = strdup(value);
-                               cur->next = 0;
-                               cur->flg_export = flg_export;
-                               cur->flg_read_only = 0;
-                               while (bottom->next)
-                                       bottom = bottom->next;
-                               bottom->next = cur;
                        }
                }
+               goto skip;
        }
 
+// TODO: need simpler/generic rollback on malloc failure - see ash
+       cur = malloc(sizeof(*cur));
+       if (!cur) {
+               result = -1;
+       } else {
+               cur->name = strdup(name);
+               if (!cur->name) {
+                       free(cur);
+                       result = -1;
+               } else {
+                       struct variables *bottom = top_vars;
+                       cur->value = strdup(value);
+                       cur->next = 0;
+                       cur->flg_export = flg_export;
+                       cur->flg_read_only = 0;
+                       while (bottom->next)
+                               bottom = bottom->next;
+                       bottom->next = cur;
+               }
+       }
+ skip:
        if (result == 0 && cur->flg_export == 1) {
                *(value-1) = '=';
                result = putenv(name);
@@ -2975,11 +2978,16 @@ static int handle_dollar(o_string *dest, struct p_context *ctx, struct in_str *i
        int i, advance = 0;
        char sep[] = " ";
        int ch = input->peek(input);  /* first character after the $ */
-       debug_printf("handle_dollar: ch=%c\n", ch);
+
+       debug_printf_parse("handle_dollar entered: ch='%c'\n", ch);
        if (isalpha(ch)) {
                b_addchr(dest, SPECIAL_VAR_SYMBOL);
                ctx->child->sp++;
-               while (ch = b_peek(input), isalnum(ch) || ch == '_') {
+               while (1) {
+                       ch = b_peek(input);
+                       if (!isalnum(ch) && ch != '_')
+                               break;
+                       debug_printf_parse(": '%c'\n", ch);
                        b_getch(input);
                        b_addchr(dest, ch);
                }
@@ -3014,14 +3022,16 @@ static int handle_dollar(o_string *dest, struct p_context *ctx, struct in_str *i
                        /* XXX maybe someone will try to escape the '}' */
                        while (1) {
                                ch = b_getch(input);
-                               if (ch == EOF || ch == '}')
+                               if (ch == EOF) {
+                                       syntax();
+                                       debug_printf_parse("handle_dollar return 1: unterminated ${name}\n");
+                                       return 1;
+                               }
+                               if (ch == '}')
                                        break;
+                               debug_printf_parse(": '%c'\n", ch);
                                b_addchr(dest, ch);
                        }
-                       if (ch != '}') {
-                               syntax();
-                               return 1;
-                       }
                        b_addchr(dest, SPECIAL_VAR_SYMBOL);
                        break;
                case '(':
@@ -3052,6 +3062,7 @@ static int handle_dollar(o_string *dest, struct p_context *ctx, struct in_str *i
         * a nice size-optimized program.  Hah!  That'll be the day.
         */
        if (advance) b_getch(input);
+       debug_printf_parse("handle_dollar return 0\n");
        return 0;
 }
 
@@ -3079,7 +3090,7 @@ static int parse_stream(o_string *dest, struct p_context *ctx,
 
        while ((ch = b_getch(input)) != EOF) {
                m = map[ch];
-               next = (ch == '\n') ? 0 : b_peek(input);
+               next = (ch == '\n') ? '\0' : b_peek(input);
                debug_printf_parse(": ch=%c (%d) m=%d quote=%d\n",
                                                ch, ch, m, dest->quote);
                if (m == MAP_ORDINARY

diff --git a/shell/hush_test/hush-parsing/noeol.right b/shell/hush_test/hush-parsing/noeol.right
new file mode 100644 (file)
index 0000000..e427984
--- /dev/null
+++ b/shell/hush_test/hush-parsing/noeol.right
@@ -0,0 +1 @@
+HELLO

diff --git a/shell/hush_test/hush-parsing/noeol.tests b/shell/hush_test/hush-parsing/noeol.tests
new file mode 100644 (file)
index 0000000..a93113a
--- /dev/null
+++ b/shell/hush_test/hush-parsing/noeol.tests
@@ -0,0 +1,2 @@
+# next line has no EOL!
+echo HELLO
\ No newline at end of file

diff --git a/shell/hush_test/hush-vars/var.right b/shell/hush_test/hush-vars/var.right
new file mode 100644 (file)
index 0000000..c13b98e
--- /dev/null
+++ b/shell/hush_test/hush-vars/var.right
@@ -0,0 +1,4 @@
+http://busybox.net
+http://busybox.net_abc
+1
+0

diff --git a/shell/hush_test/hush-vars/var.tests b/shell/hush_test/hush-vars/var.tests
new file mode 100644 (file)
index 0000000..b0637ea
--- /dev/null
+++ b/shell/hush_test/hush-vars/var.tests
@@ -0,0 +1,10 @@
+URL=http://busybox.net
+
+echo $URL
+echo ${URL}_abc
+
+true
+false; echo $?
+true
+# BUG: prints 0, must be 1
+{ false; echo $?; }

diff --git a/shell/hush_test/run-all b/shell/hush_test/run-all
new file mode 100644 (file)
index 0000000..2c2bac6
--- /dev/null
+++ b/shell/hush_test/run-all
@@ -0,0 +1,59 @@
+#!/bin/sh
+
+test -x hush || { echo "No ./hush?!"; exit; }
+
+PATH="$PWD:$PATH" # for hush and recho/zecho/printenv
+export PATH
+
+THIS_SH="$PWD/hush"
+export THIS_SH
+
+do_test()
+{
+    test -d "$1" || return 0
+    (
+    cd "$1" || { echo "cannot cd $1!"; exit 1; }
+    for x in run-*; do
+       test -f "$x" || continue
+       case "$x" in
+           "$0"|run-minimal|run-gprof) ;;
+           *.orig|*~) ;;
+           #*) echo $x ; sh $x ;;
+           *)
+           sh "$x" >"../$1-$x.fail" 2>&1 && \
+           { echo "$1/$x: ok"; rm "../$1-$x.fail"; } || echo "$1/$x: fail";
+           ;;
+       esac
+    done
+    # Many bash run-XXX scripts just do this,
+    # no point in duplication it all over the place
+    for x in *.tests; do
+       test -x "$x" || continue
+       name="${x%%.tests}"
+       test -f "$name.right" || continue
+       {
+           "$THIS_SH" "./$x" >"$name.xx" 2>&1
+           diff -u "$name.xx" "$name.right" >"../$1-$x.fail" && rm -f "$name.xx" "../$1-$x.fail"
+       } && echo "$1/$x: ok" || echo "$1/$x: fail"
+    done
+    )
+}
+
+# main part of this script
+# Usage: run-all [directories]
+
+if [ $# -lt 1 ]; then
+    # All sub directories
+    modules=`ls -d hush-*`
+
+    for module in $modules; do
+       do_test $module
+    done
+else
+    while [ $# -ge 1 ]; do
+       if [ -d $1 ]; then
+           do_test $1
+       fi
+       shift
+    done
+fi