Date: Sat, 25 Dec 2010 21:34:08 +0100 From: Jilles Tjoelker <jilles@stack.nl> To: freebsd-hackers@freebsd.org Subject: Removing PATH=...%builtin... from sh Message-ID: <20101225203408.GA52505@stack.nl>
next in thread | raw e-mail | index | archive | help
--M9NhX3UHpAaciwkO Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Most ash derivatives have an undocumented feature where the presence of an entry "%builtin" in $PATH will cause builtins to be checked at that point of the PATH search, rather than before looking at any directories as documented in the man page (very old versions do document this feature). I would like to remove this feature from sh, as it complicates the code, may violate expectations (for example, /usr/bin/alias is very close to a forkbomb with PATH=/usr/bin:%builtin, only /usr/bin/builtin not being another link saves it) and appears to be unused (all the %builtin google code search finds is in some sort of ash source code). The feature also has a bug in that it does not work properly if there is a PATH assignment before 'type', 'command -v' or 'command -V' and %builtin differs between the set and temporary paths. (This worked in 8.x for 'command'; 'type' ignores PATH assignment in 8.x.) It is true that dash ignores %builtin for the special builtins and the "regular builtins" that are found before PATH, using it only for the other builtins. I could do the same but I think it is not useful enough. See also http://lists.freebsd.org/pipermail/svn-src-head/2010-November/022613.html Which builtins this is about: POSIX special builtins: : . break continue eval exec exit export readonly return set shift times trap unset POSIX commands found before PATH (regular builtins): alias bg cd command false fc fg getopts jobs kill pwd read true umask unalias wait Other POSIX utilities that must be builtins: hash type ulimit Other POSIX utilities: [ echo printf test Non-POSIX utilities that must be builtins: builtin bind chdir jobid local setvar Undocumented builtins: exp let wordexp And for completeness one command that should really be a builtin or otherwise magic: Missing POSIX commands found before PATH (regular builtins): newgrp -- Jilles Tjoelker --M9NhX3UHpAaciwkO Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="sh-builtin-always-first.patch" Index: bin/sh/exec.c =================================================================== --- bin/sh/exec.c (revision 216690) +++ bin/sh/exec.c (working copy) @@ -92,7 +92,6 @@ static struct tblentry *cmdtable[CMDTABLESIZE]; -static int builtinloc = -1; /* index in path of %builtin, or -1 */ int exerrno = 0; /* Last exec error */ @@ -245,8 +244,7 @@ } while ((name = *argptr) != NULL) { if ((cmdp = cmdlookup(name, 0)) != NULL - && (cmdp->cmdtype == CMDNORMAL - || (cmdp->cmdtype == CMDBUILTIN && builtinloc >= 0))) + && cmdp->cmdtype == CMDNORMAL) delete_cmd_entry(); find_command(name, &entry, DO_ERR, pathval()); if (verbose) { @@ -337,8 +335,8 @@ goto success; } - /* If %builtin not in path, check for builtin next */ - if (builtinloc < 0 && (i = find_builtin(name, &spec)) >= 0) { + /* Check for builtin next */ + if ((i = find_builtin(name, &spec)) >= 0) { INTOFF; cmdp = cmdlookup(name, 1); if (cmdp->cmdtype == CMDFUNCTION) @@ -354,7 +352,7 @@ prev = -1; /* where to start */ if (cmdp) { /* doing a rehash */ if (cmdp->cmdtype == CMDBUILTIN) - prev = builtinloc; + prev = -1; else prev = cmdp->param.index; } @@ -366,19 +364,7 @@ stunalloc(fullname); idx++; if (pathopt) { - if (prefix("builtin", pathopt)) { - if ((i = find_builtin(name, &spec)) < 0) - goto loop; - INTOFF; - cmdp = cmdlookup(name, 1); - if (cmdp->cmdtype == CMDFUNCTION) - cmdp = &loc_cmd; - cmdp->cmdtype = CMDBUILTIN; - cmdp->param.index = i; - cmdp->special = spec; - INTON; - goto success; - } else if (prefix("func", pathopt)) { + if (prefix("func", pathopt)) { /* handled below */ } else { goto loop; /* ignore unimplemented options */ @@ -485,8 +471,7 @@ for (pp = cmdtable ; pp < &cmdtable[CMDTABLESIZE] ; pp++) { for (cmdp = *pp ; cmdp ; cmdp = cmdp->next) { - if (cmdp->cmdtype == CMDNORMAL - || (cmdp->cmdtype == CMDBUILTIN && builtinloc >= 0)) + if (cmdp->cmdtype == CMDNORMAL) cmdp->rehash = 1; } } @@ -506,13 +491,11 @@ const char *old, *new; int idx; int firstchange; - int bltin; old = pathval(); new = newval; firstchange = 9999; /* assume no change */ idx = 0; - bltin = -1; for (;;) { if (*old != *new) { firstchange = idx; @@ -523,19 +506,12 @@ } if (*new == '\0') break; - if (*new == '%' && bltin < 0 && prefix("builtin", new + 1)) - bltin = idx; if (*new == ':') { idx++; } new++, old++; } - if (builtinloc < 0 && bltin >= 0) - builtinloc = bltin; /* zap builtins */ - if (builtinloc >= 0 && bltin < 0) - firstchange = 0; clearcmdentry(firstchange); - builtinloc = bltin; } @@ -556,9 +532,7 @@ pp = tblp; while ((cmdp = *pp) != NULL) { if ((cmdp->cmdtype == CMDNORMAL && - cmdp->param.index >= firstchange) - || (cmdp->cmdtype == CMDBUILTIN && - builtinloc >= firstchange)) { + cmdp->param.index >= firstchange)) { *pp = cmdp->next; ckfree(cmdp); } else { --M9NhX3UHpAaciwkO--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20101225203408.GA52505>