From owner-freebsd-hackers@FreeBSD.ORG Sat Dec 25 20:35:13 2010 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id DBB36106566B for ; Sat, 25 Dec 2010 20:35:13 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (relay04.stack.nl [IPv6:2001:610:1108:5010::107]) by mx1.freebsd.org (Postfix) with ESMTP id 61A8A8FC0C for ; Sat, 25 Dec 2010 20:35:13 +0000 (UTC) Received: by mx1.stack.nl (Postfix, from userid 65534) id B21C21DDAA2; Sat, 25 Dec 2010 21:35:10 +0100 (CET) X-Spam-DCC: : scanner01.stack.nl 1282; Body=1 Fuz1=1 Fuz2=1 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on scanner01.stack.nl X-Spam-Level: X-Spam-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_00, DNS_FROM_OPENWHOIS,NO_RELAYS autolearn=no version=3.2.5 X-Spam-Relay-Country: _RELAYCOUNTRY_ Received: from turtle.stack.nl (turtle.stack.nl [IPv6:2001:610:1108:5010::132]) by mx1.stack.nl (Postfix) with ESMTP id 623721DD40C for ; Sat, 25 Dec 2010 21:35:08 +0100 (CET) Received: by turtle.stack.nl (Postfix, from userid 1677) id 4494217104; Sat, 25 Dec 2010 21:34:08 +0100 (CET) Date: Sat, 25 Dec 2010 21:34:08 +0100 From: Jilles Tjoelker To: freebsd-hackers@freebsd.org Message-ID: <20101225203408.GA52505@stack.nl> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="M9NhX3UHpAaciwkO" Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Removing PATH=...%builtin... from sh X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 25 Dec 2010 20:35:13 -0000 --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--