From owner-svn-src-all@FreeBSD.ORG Sun Jan 9 21:07:30 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A8D10106564A; Sun, 9 Jan 2011 21:07:30 +0000 (UTC) (envelope-from jilles@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 96DF98FC16; Sun, 9 Jan 2011 21:07:30 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id p09L7UFq056612; Sun, 9 Jan 2011 21:07:30 GMT (envelope-from jilles@svn.freebsd.org) Received: (from jilles@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id p09L7UG2056609; Sun, 9 Jan 2011 21:07:30 GMT (envelope-from jilles@svn.freebsd.org) Message-Id: <201101092107.p09L7UG2056609@svn.freebsd.org> From: Jilles Tjoelker Date: Sun, 9 Jan 2011 21:07:30 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r217206 - in head: bin/sh tools/regression/bin/sh/execution X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 09 Jan 2011 21:07:30 -0000 Author: jilles Date: Sun Jan 9 21:07:30 2011 New Revision: 217206 URL: http://svn.freebsd.org/changeset/base/217206 Log: sh: Remove special %builtin PATH entry. All builtins are now always found before a PATH search. 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 am removing 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). Note that aliases and functions took and take precedence above builtins. Because aliases work on a lexical level they can only ever be overridden on a lexical level (quoting or preceding 'builtin' or 'command'). Allowing override of functions via PATH does not really fit in the model of sh and it would work differently from %builtin if implemented. Note: POSIX says special builtins are found before functions. We comply to this because we do not allow functions with the same name as a special builtin. Silence from: freebsd-hackers@ (message sent 20101225) Discussed with: dougb Added: head/tools/regression/bin/sh/execution/path1.0 (contents, props changed) Modified: head/bin/sh/exec.c Modified: head/bin/sh/exec.c ============================================================================== --- head/bin/sh/exec.c Sun Jan 9 21:02:11 2011 (r217205) +++ head/bin/sh/exec.c Sun Jan 9 21:07:30 2011 (r217206) @@ -92,7 +92,6 @@ struct tblentry { static struct tblentry *cmdtable[CMDTABLESIZE]; -static int builtinloc = -1; /* index in path of %builtin, or -1 */ int exerrno = 0; /* Last exec error */ @@ -244,8 +243,7 @@ hashcmd(int argc __unused, char **argv _ } 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) { @@ -336,8 +334,8 @@ find_command(const char *name, struct cm 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) @@ -353,7 +351,7 @@ find_command(const char *name, struct cm prev = -1; /* where to start */ if (cmdp) { /* doing a rehash */ if (cmdp->cmdtype == CMDBUILTIN) - prev = builtinloc; + prev = -1; else prev = cmdp->param.index; } @@ -365,19 +363,7 @@ loop: 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 */ @@ -484,8 +470,7 @@ hashcd(void) 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; } } @@ -505,13 +490,11 @@ changepath(const char *newval) 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; @@ -522,19 +505,12 @@ changepath(const char *newval) } 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; } @@ -555,9 +531,7 @@ clearcmdentry(int firstchange) 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 { Added: head/tools/regression/bin/sh/execution/path1.0 ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ head/tools/regression/bin/sh/execution/path1.0 Sun Jan 9 21:07:30 2011 (r217206) @@ -0,0 +1,15 @@ +# $FreeBSD$ +# Some builtins should not be overridable via PATH. + +set -e +T=$(mktemp -d ${TMPDIR:-/tmp}/sh-test.XXXXXX) +trap 'rm -rf ${T}' 0 +echo '#!/bin/sh +echo bad' >"$T/cd" +chmod 755 "$T/cd" +cd /bin +oPATH=$PATH +PATH=$T:$PATH:%builtin +cd / +PATH=$oPATH +[ "$(pwd)" = / ]