hush: take care of several easy FIXMEs. -228 bytes.

1 files changed, 68 insertions, 124 deletions

diff --git a/shell/hush.c b/shell/hush.c
index 84f2870f2..7a4d7f934 100644
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -124,6 +124,7 @@ static const char *indenter(int i)
         return &blanks[sizeof(blanks) - i - 1];
 }
 #define debug_printf_clean(...) fprintf(stderr, __VA_ARGS__)
+#define DEBUG_CLEAN 1
 #endif
 
 
@@ -232,10 +233,10 @@ struct child_prog {
         glob_t glob_result;         /* result of parameter globbing */
         int is_stopped;             /* is the program currently running? */
         struct pipe *family;        /* pointer back to the child's parent pipe */
-        int sp;                     /* number of SPECIAL_VAR_SYMBOL */
+        //sp counting seems to be broken... so commented out, grep for '//sp:'
+        //sp: int sp;               /* number of SPECIAL_VAR_SYMBOL */
         int type;
 };
-// sp counting seems to be broken...
 /* argv vector may contain variable references (^Cvar^C, ^C0^C etc)
  * and on execution these are substituted with their values.
  * Substitution can make _several_ words out of one argv[n]!
@@ -383,7 +384,6 @@ static int b_check_space(o_string *o, int len);
 static int b_addchr(o_string *o, int ch);
 static void b_reset(o_string *o);
 static int b_addqchr(o_string *o, int ch, int quote);
-//static int b_adduint(o_string *o, unsigned i);
 /*  in_str manipulations: */
 static int static_get(struct in_str *i);
 static int static_peek(struct in_str *i);
@@ -396,7 +396,10 @@ static void mark_open(int fd);
 static void mark_closed(int fd);
 static void close_all(void);
 /*  "run" the final data structures: */
-//TODO: remove indent argument from non-debug build!
+#if !defined(DEBUG_CLEAN)
+#define free_pipe_list(head, indent) free_pipe_list(head)
+#define free_pipe(pi, indent)        free_pipe(pi)
+#endif
 static int free_pipe_list(struct pipe *head, int indent);
 static int free_pipe(struct pipe *pi, int indent);
 /*  really run the final data structures: */
@@ -425,7 +428,6 @@ static int parse_group(o_string *dest, struct p_context *ctx, struct in_str *inp
 static const char *lookup_param(const char *src);
 static char *make_string(char **inp);
 static int handle_dollar(o_string *dest, struct p_context *ctx, struct in_str *input);
-//static int parse_string(o_string *dest, struct p_context *ctx, const char *src);
 static int parse_stream(o_string *dest, struct p_context *ctx, struct in_str *input0, const char *end_trigger);
 /*   setup: */
 static int parse_stream_outer(struct in_str *inp, int parse_flag);
@@ -708,9 +710,8 @@ static int builtin_export(char **argv)
                 return EXIT_SUCCESS;
         }
 
-        name = strdup(name);
-
-        if (name) {
+        name = xstrdup(name);
+        {
                 const char *value = strchr(name, '=');
 
                 if (!value) {
@@ -721,13 +722,9 @@ static int builtin_export(char **argv)
                         if (value) {
                                 size_t ln = strlen(name);
 
-                                tmp = realloc(name, ln+strlen(value)+2);
-                                if (tmp == NULL)
-                                        res = -1;
-                                else {
-                                        sprintf(tmp+ln, "=%s", value);
-                                        name = tmp;
-                                }
+                                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. */
@@ -967,13 +964,9 @@ static int b_check_space(o_string *o, int len)
         /* It would be easy to drop a more restrictive policy
          * in here, such as setting a maximum string length */
         if (o->length + len > o->maxlen) {
-                char *old_data = o->data;
                 /* assert(data == NULL || o->maxlen != 0); */
                 o->maxlen += (2*len > B_CHUNK ? 2*len : B_CHUNK);
-                o->data = realloc(o->data, 1 + o->maxlen);
-                if (o->data == NULL) {
-                        free(old_data);
-                }
+                o->data = xrealloc(o->data, 1 + o->maxlen);
         }
         return o->data == NULL;
 }
@@ -1019,17 +1012,6 @@ static int b_addqchr(o_string *o, int ch, int quote)
         return b_addchr(o, ch);
 }
 
-//static int b_adduint(o_string *o, unsigned i)
-//{
-//      int r;
-//      char buf[sizeof(unsigned)*3 + 1];
-//      char *p = buf;
-//      *(utoa_to_buf(i, buf, sizeof(buf))) = '\0';
-//      /* no escape checking necessary */
-//      do r = b_addchr(o, *p++); while (r == 0 && *p);
-//      return r;
-//}
-//
 static int static_get(struct in_str *i)
 {
         int ch = *i->p++;
@@ -1286,7 +1268,7 @@ static void pseudo_exec_argv(char **argv)
         const struct built_in_command *x;
 
         for (i = 0; is_assignment(argv[i]); i++) {
-                debug_printf("pid %d environment modification: %s\n",
+                debug_printf_exec("pid %d environment modification: %s\n",
                                 getpid(), argv[i]);
 // FIXME: vfork case??
                 p = insert_var_value(argv[i]);
@@ -1317,23 +1299,27 @@ static void pseudo_exec_argv(char **argv)
                 }
         }
 
-        /* Check if the command matches any busybox internal commands
-         * ("applets") here.
-         * FIXME: This feature is not 100% safe, since
-         * BusyBox is not fully reentrant, so we have no guarantee the things
-         * from the .bss are still zeroed, or that things from .data are still
-         * at their defaults.  We could exec ourself from /proc/self/exe, but I
-         * really dislike relying on /proc for things.  We could exec ourself
-         * from global_argv[0], but if we are in a chroot, we may not be able
-         * to find ourself... */
+        /* Check if the command matches any busybox applets */
 #if ENABLE_FEATURE_SH_STANDALONE
-        debug_printf("running applet %s\n", argv[0]);
-// FIXME: check NOEXEC bit, and EXEC if not set!
-        run_applet_and_exit(argv[0], argv);
-// is it ok that run_applet_and_exit() does exit(), not _exit()?
-// NB: IIRC on NOMMU we are after _vfork_, not fork!
+        if (strchr(argv[0], '/') == NULL) {
+                const struct bb_applet *a = find_applet_by_name(argv[0]);
+                if (a) {
+                        if (a->noexec) {
+                                current_applet = a;
+                                debug_printf_exec("running applet '%s'\n", argv[0]);
+// is it ok that run_current_applet_and_exit() does exit(), not _exit()?
+                                run_current_applet_and_exit(argv);
+                        }
+                        /* re-exec ourselves with the new arguments */
+                        debug_printf_exec("re-execing applet '%s'\n", argv[0]);
+                        execvp(CONFIG_BUSYBOX_EXEC_PATH, argv);
+                        /* If they called chroot or otherwise made the binary no longer
+                         * executable, fall through */
+                }
+        }
 #endif
-        debug_printf("exec of %s\n", argv[0]);
+
+        debug_printf_exec("execing '%s'\n", argv[0]);
         execvp(argv[0], argv);
         bb_perror_msg("cannot exec '%s'", argv[0]);
         _exit(1);
@@ -1426,16 +1412,7 @@ static void insert_bg_job(struct pipe *pi)
 // TODO: do we really need to have so many fields which are just dead weight
 // at execution stage?
                 thejob->progs[i].pid = pi->progs[i].pid;
-                //rest:
-                //char **argv;                /* program name and arguments */
-                //struct pipe *group;         /* if non-NULL, first in group or subshell */
-                //int subshell;               /* flag, non-zero if group must be forked */
-                //struct redir_struct *redirects; /* I/O redirections */
-                //glob_t glob_result;         /* result of parameter globbing */
-                //int is_stopped;             /* is the program currently running? */
-                //struct pipe *family;        /* pointer back to the child's parent pipe */
-                //int sp;                     /* number of SPECIAL_VAR_SYMBOL */
-                //int type;
+                /* all other fields are not used and stay zero */
         }
         thejob->next = NULL;
         thejob->cmdtext = xstrdup(get_cmdtext(pi));
@@ -1594,10 +1571,6 @@ static int checkjobs(struct pipe* fg_pipe)
 
         if (childpid && errno != ECHILD)
                 bb_perror_msg("waitpid");
-
-        /* move the shell to the foreground */
-        //if (interactive_fd && tcsetpgrp(interactive_fd, getpgid(0)))
-        //      bb_perror_msg("tcsetpgrp-2");
         return rcode;
 }
 
@@ -1669,7 +1642,7 @@ static int run_pipe_real(struct pipe *pi)
         }
 
         if (single_fg && child->argv != NULL) {
-                char **argv_expanded, **free_me;
+                char **argv_expanded;
                 char **argv = child->argv;
 
                 for (i = 0; is_assignment(argv[i]); i++)
@@ -1704,13 +1677,12 @@ static int run_pipe_real(struct pipe *pi)
                 for (i = 0; is_assignment(argv[i]); i++) {
                         p = insert_var_value(argv[i]);
                         if (p != argv[i]) {
-                                child->sp--;
+                                //sp: child->sp--;
                                 putenv(p);
                         } else {
                                 putenv(xstrdup(p));
                         }
                 }
-                free_me = NULL;
                 for (x = bltins; x->cmd; x++) {
                         if (strcmp(argv[i], x->cmd) == 0) {
                                 if (x->function == builtin_exec && argv[i+1] == NULL) {
@@ -1723,14 +1695,12 @@ static int run_pipe_real(struct pipe *pi)
                                  * This is perfect for work that comes after exec().
                                  * Is it really safe for inline use?  Experimentally,
                                  * things seem to work with glibc. */
-// TODO: fflush(NULL)?
                                 setup_redirects(child, squirrel);
                                 debug_printf_exec(": builtin '%s' '%s'...\n", x->cmd, argv[i+1]);
-                                argv_expanded = argv + i;
-                                if (child->sp) /* btw we can do it unconditionally... */
-                                        free_me = argv_expanded = do_variable_expansion(argv + i);
+                                //sp: if (child->sp) /* btw we can do it unconditionally... */
+                                argv_expanded = do_variable_expansion(argv + i);
                                 rcode = x->function(argv_expanded);
-                                free(free_me);
+                                free(argv_expanded);
                                 restore_redirects(squirrel);
                                 debug_printf_exec("run_pipe_real return %d\n", rcode);
                                 return rcode;
@@ -1743,11 +1713,11 @@ static int run_pipe_real(struct pipe *pi)
                                 setup_redirects(child, squirrel);
                                 save_nofork_data(&nofork_save);
                                 argv_expanded = argv + i;
-                                if (child->sp)
-                                        free_me = argv_expanded = do_variable_expansion(argv + i);
+                                //sp: if (child->sp)
+                                argv_expanded = do_variable_expansion(argv + i);
                                 debug_printf_exec(": run_nofork_applet '%s' '%s'...\n", argv_expanded[0], argv_expanded[1]);
                                 rcode = run_nofork_applet_prime(&nofork_save, a, argv_expanded);
-                                free(free_me);
+                                free(argv_expanded);
                                 restore_redirects(squirrel);
                                 debug_printf_exec("run_pipe_real return %d\n", rcode);
                                 return rcode;
@@ -1802,7 +1772,7 @@ static int run_pipe_real(struct pipe *pi)
                                 }
                         }
 #endif
-                        // in non-interactive case fatal sigs are already SIG_DFL
+                        /* in non-interactive case fatal sigs are already SIG_DFL */
                         close_all();
                         if (nextin != 0) {
                                 dup2(nextin, 0);
@@ -1833,13 +1803,6 @@ static int run_pipe_real(struct pipe *pi)
                 if (pi->pgrp < 0)
                         pi->pgrp = child->pid;
 #endif
-
-                /* Don't check for errors.  The child may be dead already,
-                 * in which case setpgid returns error code EACCES. */
-                //why we do it at all?? child does it itself
-                //if (interactive_fd)
-                //      setpgid(child->pid, pi->pgrp);
-
                 if (nextin != 0)
                         close(nextin);
                 if (nextout != 1)
@@ -1911,8 +1874,8 @@ static void debug_print_tree(struct pipe *pi, int lvl)
 }
 #endif
 
-// NB: called by pseudo_exec, and therefore must not modify any
-// global data until exec/_exit (we can be a child after vfork!)
+/* NB: called by pseudo_exec, and therefore must not modify any
+ * global data until exec/_exit (we can be a child after vfork!) */
 static int run_list_real(struct pipe *pi)
 {
 #if ENABLE_HUSH_JOB
@@ -2224,9 +2187,7 @@ static int globhack(const char *src, int flags, glob_t *pglob)
                 if (*s == '\\') s++;
                 cnt++;
         }
-        dest = malloc(cnt);
-        if (!dest)
-                return GLOB_NOSPACE;
+        dest = xmalloc(cnt);
         if (!(flags & GLOB_APPEND)) {
                 pglob->gl_pathv = NULL;
                 pglob->gl_pathc = 0;
@@ -2234,9 +2195,7 @@ static int globhack(const char *src, int flags, glob_t *pglob)
                 pglob->gl_offs = 0;
         }
         pathc = ++pglob->gl_pathc;
-        pglob->gl_pathv = realloc(pglob->gl_pathv, (pathc+1) * sizeof(*pglob->gl_pathv));
-        if (pglob->gl_pathv == NULL)
-                return GLOB_NOSPACE;
+        pglob->gl_pathv = xrealloc(pglob->gl_pathv, (pathc+1) * sizeof(*pglob->gl_pathv));
         pglob->gl_pathv[pathc-1] = dest;
         pglob->gl_pathv[pathc] = NULL;
         for (s = src; s && *s; s++, dest++) {
@@ -2312,8 +2271,8 @@ static int count_ifs(const char *str)
         while (1) {
                 str += strcspn(str, ifs);
                 if (!*str) break;
-                str++; // str += strspn(str, ifs); ?
-                cnt++; // cnt += strspn(str, ifs); ?
+                str++; /* str += strspn(str, ifs); */
+                cnt++; /* cnt += strspn(str, ifs); - but this code is larger */
         }
         debug_printf_expand(" return %d\n", cnt);
         return cnt;
@@ -2704,31 +2663,23 @@ static int set_local_var(const char *s, int flg_export)
                                 if (flg_export > 0 || cur->flg_export > 1)
                                         cur->flg_export = 1;
                                 free((char*)cur->value);
-                                cur->value = strdup(value);
+                                cur->value = xstrdup(value);
                         }
                         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;
-                }
+        cur = xzalloc(sizeof(*cur));
+        cur->name = xstrdup(name);
+        cur->value = xstrdup(value);
+        /*cur->next = 0;*/
+        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) {
@@ -3019,7 +2970,7 @@ static int done_command(struct p_context *ctx)
         /*child->group = NULL;*/
         /*child->glob_result.gl_pathv = NULL;*/
         child->family = pi;
-        /*child->sp = 0;*/
+        //sp: /*child->sp = 0;*/
         child->type = ctx->parse_type;
 
         ctx->child = child;
@@ -3268,7 +3219,7 @@ static int handle_dollar(o_string *dest, struct p_context *ctx, struct in_str *i
         debug_printf_parse("handle_dollar entered: ch='%c'\n", ch);
         if (isalpha(ch)) {
                 b_addchr(dest, SPECIAL_VAR_SYMBOL);
-                ctx->child->sp++;
+                //sp: ctx->child->sp++;
                 while (1) {
                         debug_printf_parse(": '%c'\n", ch);
                         b_getch(input);
@@ -3282,7 +3233,7 @@ static int handle_dollar(o_string *dest, struct p_context *ctx, struct in_str *i
         } else if (isdigit(ch)) {
  make_one_char_var:
                 b_addchr(dest, SPECIAL_VAR_SYMBOL);
-                ctx->child->sp++;
+                //sp: ctx->child->sp++;
                 debug_printf_parse(": '%c'\n", ch);
                 b_getch(input);
                 b_addchr(dest, ch | quote_mask);
@@ -3297,7 +3248,7 @@ static int handle_dollar(o_string *dest, struct p_context *ctx, struct in_str *i
                         goto make_one_char_var;
                 case '{':
                         b_addchr(dest, SPECIAL_VAR_SYMBOL);
-                        ctx->child->sp++;
+                        //sp: ctx->child->sp++;
                         b_getch(input);
                         /* XXX maybe someone will try to escape the '}' */
                         while (1) {
@@ -3332,13 +3283,6 @@ static int handle_dollar(o_string *dest, struct p_context *ctx, struct in_str *i
         return 0;
 }
 
-//static int parse_string(o_string *dest, struct p_context *ctx, const char *src)
-//{
-//      struct in_str foo;
-//      setup_string_in_str(&foo, src);
-//      return parse_stream(dest, ctx, &foo, NULL);
-//}
-//
 /* return code is 0 for normal exit, 1 for syntax error */
 static int parse_stream(o_string *dest, struct p_context *ctx,
         struct in_str *input, const char *end_trigger)
@@ -3711,9 +3655,9 @@ int hush_main(int argc, char **argv)
                         opt = parse_string_outer(optarg, FLAG_PARSE_SEMICOLON);
                         goto final_return;
                 case 'i':
-                        // Well, we cannot just declare interactiveness,
-                        // we have to have some stuff (ctty, etc)
-                        /*interactive_fd++;*/
+                        /* Well, we cannot just declare interactiveness,
+                         * we have to have some stuff (ctty, etc) */
+                        /* interactive_fd++; */
                         break;
                 case 'f':
                         fake_mode++;