From c666f71e3b91285d66260567b0b4c57332e0ff1f Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Wed, 16 May 2007 22:18:54 +0000 Subject: hush: take care of several easy FIXMEs. -228 bytes. --- shell/hush.c | 192 +++++++++++++++++++++-------------------------------------- 1 file changed, 68 insertions(+), 124 deletions(-) (limited to 'shell') diff --git a/shell/hush.c b/shell/hush.c index 84f2870..7a4d7f9 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++; -- cgit v1.1