Subject: Re: bin/19354: Semantics of /bin/sh "command" builtin violates POSIX
To: None <netbsd-bugs@netbsd.org>
From: David Laight <david@l8s.co.uk>
List: netbsd-bugs
Date: 12/19/2002 00:12:37
> >Synopsis: Semantics of /bin/sh "command" builtin violates POSIX
>
> The current semantics of "command" require a shell builtin
> to be given. As per POSIX and /bin/ksh, "command" should merely
> suppresses shell function lookup.
Fixed by attached patch.
Things fixed:
- command and command -p
- don't hash PATH=xxx yyy
- fix hash table when a function that has 'local PATH' returns
- use supplied PATH for 'PATH=xxx type yyy' and 'PATH=xxx hash yyy'
David
Index: cd.c
===================================================================
RCS file: /cvsroot/basesrc/bin/sh/cd.c,v
retrieving revision 1.29
diff -u -r1.29 cd.c
--- cd.c 2002/11/24 22:35:39 1.29
+++ cd.c 2002/12/18 23:57:19
@@ -85,7 +85,7 @@
const char *path;
char *p, *d;
struct stat statb;
- int print = 0;
+ int print = cdprint; /* set -cdprint to enable */
nextopt(nullstr);
Index: eval.c
===================================================================
RCS file: /cvsroot/basesrc/bin/sh/eval.c,v
retrieving revision 1.69
diff -u -r1.69 eval.c
--- eval.c 2002/11/25 21:55:58 1.69
+++ eval.c 2002/12/18 23:57:23
@@ -45,6 +45,7 @@
#endif
#endif /* not lint */
+#include <stdlib.h>
#include <signal.h>
#include <stdio.h>
#include <unistd.h>
@@ -52,6 +53,7 @@
#include <sys/types.h>
#include <sys/wait.h>
+#include <sys/sysctl.h>
/*
* Evaluate a command.
@@ -108,7 +110,6 @@
STATIC void evalcommand(union node *, int, struct backcmd *);
STATIC void prehash(union node *);
-
/*
* Called to reset things after an exception.
*/
@@ -579,6 +580,27 @@
result->fd, result->buf, result->nleft, result->jp));
}
+static const char *
+syspath(void)
+{
+ static char *sys_path = NULL;
+ static int mib[] = {CTL_USER, USER_CS_PATH};
+ int len;
+
+ if (sys_path == NULL) {
+ if (sysctl(mib, 2, 0, &len, 0, 0) != -1 &&
+ (sys_path = ckmalloc(len + 5)) != NULL &&
+ sysctl(mib, 2, sys_path + 5, &len, 0, 0) != -1) {
+ memcpy(sys_path, "PATH=", 5);
+ } else {
+ ckfree(sys_path);
+ /* something to keep things happy */
+ sys_path = "PATH=/usr/bin:/bin:/usr/sbin:/sbin";
+ }
+ }
+ return sys_path;
+}
+
int vforked = 0;
@@ -609,6 +631,7 @@
struct localvar *volatile savelocalvars;
volatile int e;
char *lastarg;
+ const char *path = pathval();
#if __GNUC__
/* Avoid longjmp clobbering */
(void) &argv;
@@ -687,42 +710,60 @@
/* Now locate the command. */
if (argc == 0) {
- cmdentry.cmdtype = CMDBUILTIN;
+ cmdentry.cmdtype = CMDSPLBLTIN;
cmdentry.u.bltin = bltincmd;
} else {
static const char PATH[] = "PATH=";
- const char *path = pathval();
+ int cmd_flags = DO_ERR;
/*
* Modify the command lookup path, if a PATH= assignment
* is present
*/
- for (sp = varlist.list ; sp ; sp = sp->next)
+ for (sp = varlist.list; sp; sp = sp->next)
if (strncmp(sp->text, PATH, sizeof(PATH) - 1) == 0)
path = sp->text + sizeof(PATH) - 1;
- find_command(argv[0], &cmdentry, DO_ERR, path);
- if (cmdentry.cmdtype == CMDUNKNOWN) { /* command not found */
- exitstatus = 127;
- flushout(&errout);
- return;
- }
- /* implement the 'command' (was bltin) builtin here */
- while (cmdentry.cmdtype == CMDBUILTIN && cmdentry.u.bltin == bltincmd) {
- argv++;
- if (--argc == 0)
- break;
- if ((cmdentry.u.bltin = find_builtin(*argv)) != 0)
- continue;
- if ((cmdentry.u.bltin = find_splbltin(*argv)) == 0) {
- outfmt(&errout, "%s: not found\n", *argv);
+ do {
+ find_command(argv[0], &cmdentry, cmd_flags, path);
+ if (cmdentry.cmdtype == CMDUNKNOWN) {
exitstatus = 127;
flushout(&errout);
return;
}
- cmdentry.cmdtype = CMDSPLBLTIN;
- break;
- }
+
+ /* implement the 'command' builtin here */
+ if (cmdentry.cmdtype != CMDBUILTIN ||
+ cmdentry.u.bltin != bltincmd)
+ break;
+ cmd_flags |= DO_NOFUNC;
+ for (;;) {
+ char *cp;
+ argv++;
+ if (--argc == 0)
+ break;
+ cp = *argv;
+ if (*cp++ != '-')
+ break;
+ if (*cp == '-' == cp[1] == 0) {
+ argv++;
+ argc--;
+ break;
+ }
+ switch (*cp++) {
+ case 'p':
+ path = syspath() + 5;
+ break;
+ /* XXX 'v' and 'V' TBS */
+ default:
+ outfmt(&errout, "Illegal option -%c\n",
+ cp[-1]);
+ exitstatus = 1;
+ flushout(&errout);
+ return;
+ }
+ }
+ } while (argc != 0);
}
/* Fork off a child process if necessary. */
@@ -775,8 +816,7 @@
}
savehandler = handler;
handler = &jmploc;
- for (sp = varlist.list; sp; sp = sp->next)
- mklocal(sp->text, VEXPORT);
+ listmklocal(varlist.list, VEXPORT | VNOFUNC);
forkchild(jp, cmd, mode, vforked);
break;
default:
@@ -819,7 +859,8 @@
/* This is the child process if a fork occurred. */
/* Execute the command. */
- if (cmdentry.cmdtype == CMDFUNCTION) {
+ switch (cmdentry.cmdtype) {
+ case CMDFUNCTION:
#ifdef DEBUG
trputs("Shell function: "); trargs(argv);
#endif
@@ -849,9 +890,10 @@
}
savehandler = handler;
handler = &jmploc;
- for (sp = varlist.list ; sp ; sp = sp->next)
- mklocal(sp->text, 0);
- funcnest++;
+ listmklocal(varlist.list, 0);
+ /* stop shell blowing its stack */
+ if (++funcnest > 1000)
+ error("too many nested function calls");
evaltree(cmdentry.u.func, flags & EV_TESTED);
funcnest--;
INTOFF;
@@ -868,7 +910,10 @@
}
if (flags & EV_EXIT)
exitshell(exitstatus);
- } else if (cmdentry.cmdtype == CMDBUILTIN || cmdentry.cmdtype == CMDSPLBLTIN) {
+ break;
+
+ case CMDBUILTIN:
+ case CMDSPLBLTIN:
#ifdef DEBUG
trputs("builtin command: "); trargs(argv);
#endif
@@ -881,6 +926,8 @@
}
savehandler = handler;
handler = &jmploc;
+ savelocalvars = localvars;
+ localvars = 0;
e = -1;
if (!setjmp(jmploc.loc)) {
redirect(cmd->ncmd.redirect, mode);
@@ -898,6 +945,9 @@
/* and getopt */
optreset = 1;
optind = 1;
+ /* 'PATH=xxx type/hash ...' need this */
+ if (path != pathval() && cmdentry.cmdtype == CMDBUILTIN)
+ mklocal(path - 5 /* PATH= */, 0);
exitstatus = cmdentry.u.bltin(argc, argv);
} else {
e = exception;
@@ -908,6 +958,8 @@
out1 = &output;
out2 = &errout;
freestdout();
+ poplocalvars();
+ localvars = savelocalvars;
cmdenviron = NULL;
if (e != EXSHELLPROC) {
commandname = savecmdname;
@@ -919,14 +971,6 @@
if ((e != EXERROR && e != EXEXEC)
|| cmdentry.cmdtype == CMDSPLBLTIN
|| cmdentry.u.bltin == bltincmd)
-#if 0
- || cmdentry.u.bltin == dotcmd
- || cmdentry.u.bltin == evalcmd
-#ifndef SMALL
- || cmdentry.u.bltin == histcmd
-#endif
- || cmdentry.u.bltin == execcmd)
-#endif
exraise(e);
FORCEINTON;
}
@@ -937,7 +981,9 @@
backcmd->nleft = memout.nextc - memout.buf;
memout.buf = NULL;
}
- } else {
+ break;
+
+ default:
#ifdef DEBUG
trputs("normal command: "); trargs(argv);
#endif
@@ -947,7 +993,8 @@
for (sp = varlist.list ; sp ; sp = sp->next)
setvareq(sp->text, VEXPORT|VSTACK);
envp = environment();
- shellexec(argv, envp, pathval(), cmdentry.u.index, vforked);
+ shellexec(argv, envp, path, cmdentry.u.index, vforked);
+ break;
}
goto out;
@@ -963,6 +1010,10 @@
out:
if (lastarg)
+ /* dsl: I think this is intended to be used to support
+ * '_' in 'vi' command mode during line editing...
+ * However I implemented that within libedit itself.
+ */
setvar("_", lastarg, 0);
popstackmark(&smark);
@@ -997,15 +1048,13 @@
*/
/*
- * No command given, or a bltin command with no arguments. Set the
+ * No command given, or 'command' builtin with no arguments. Set the
* specified variables.
*/
int
bltincmd(int argc, char **argv)
{
- /* "command" isn't a special builtin.... */
- listsetvar(cmdenviron, 0);
/*
* Preserve exitstatus of a previous possible redirection
* as POSIX mandates
@@ -1086,7 +1134,7 @@
iflag = 0; /* exit on error */
mflag = 0;
optschanged();
- for (sp = cmdenviron; sp ; sp = sp->next)
+ for (sp = cmdenviron; sp; sp = sp->next)
setvareq(sp->text, VEXPORT|VSTACK);
shellexec(argv + 1, environment(), pathval(), 0, 0);
}
Index: exec.c
===================================================================
RCS file: /cvsroot/basesrc/bin/sh/exec.c,v
retrieving revision 1.34
diff -u -r1.34 exec.c
--- exec.c 2002/11/24 22:35:39 1.34
+++ exec.c 2002/12/18 23:57:26
@@ -428,7 +428,7 @@
void
find_command(char *name, struct cmdentry *entry, int act, const char *path)
{
- struct tblentry *cmdp;
+ struct tblentry *cmdp, loc_cmd;
int idx;
int prev;
char *fullname;
@@ -436,7 +436,7 @@
int e;
int (*bltin)(int,char **);
- /* If name contains a slash, don't use the hash table */
+ /* If name contains a slash, don't use PATH or hash table */
if (strchr(name, '/') != NULL) {
if (act & DO_ABS) {
while (stat(name, &statb) < 0) {
@@ -459,20 +459,46 @@
return;
}
- /* If name is in the table, and not invalidated by cd, we're done */
- if ((cmdp = cmdlookup(name, 0)) != NULL && cmdp->rehash == 0)
- goto success;
+ if (path != pathval())
+ act |= DO_ALTPATH;
- /* If %builtin not in path, check for builtin next */
- if (builtinloc < 0 && (bltin = find_builtin(name)) != 0) {
- INTOFF;
- cmdp = cmdlookup(name, 1);
- cmdp->cmdtype = CMDBUILTIN;
- cmdp->param.bltin = bltin;
- INTON;
- goto success;
+ if (act & DO_ALTPATH && strstr(path, "%builtin") != NULL)
+ act |= DO_ALTBLTIN;
+
+ /* If name is in the table, check answer will be ok */
+ if ((cmdp = cmdlookup(name, 0)) != NULL) {
+ do {
+ switch (cmdp->cmdtype) {
+ case CMDNORMAL:
+ if (act & DO_ALTPATH) {
+ cmdp = NULL;
+ continue;
+ }
+ break;
+ case CMDFUNCTION:
+ if (act & DO_NOFUNC) {
+ cmdp = NULL;
+ continue;
+ }
+ break;
+ case CMDBUILTIN:
+ if ((act & DO_ALTBLTIN) || builtinloc >= 0) {
+ cmdp = NULL;
+ continue;
+ }
+ break;
+ }
+ /* if not invalidated by cd, we're done */
+ if (cmdp->rehash == 0)
+ goto success;
+ } while (0);
}
+ /* If %builtin not in path, check for builtin next */
+ if ((act & DO_ALTPATH ? !(act & DO_ALTBLTIN) : builtinloc < 0) &&
+ (bltin = find_builtin(name)) != 0)
+ goto builtin_success;
+
/* We have to search path. */
prev = -1; /* where to start */
if (cmdp) { /* doing a rehash */
@@ -492,16 +518,12 @@
if (prefix("builtin", pathopt)) {
if ((bltin = find_builtin(name)) == 0)
goto loop;
- INTOFF;
- cmdp = cmdlookup(name, 1);
- cmdp->cmdtype = CMDBUILTIN;
- cmdp->param.bltin = bltin;
- INTON;
- goto success;
+ goto builtin_success;
} else if (prefix("func", pathopt)) {
/* handled below */
} else {
- goto loop; /* ignore unimplemented options */
+ /* ignore unimplemented options */
+ goto loop;
}
}
/* if rehash, don't redo absolute path names */
@@ -524,14 +546,19 @@
if (!S_ISREG(statb.st_mode))
goto loop;
if (pathopt) { /* this is a %func directory */
+ if (act & DO_NOFUNC)
+ goto loop;
stalloc(strlen(fullname) + 1);
readcmdfile(fullname);
- if ((cmdp = cmdlookup(name, 0)) == NULL || cmdp->cmdtype != CMDFUNCTION)
+ if ((cmdp = cmdlookup(name, 0)) == NULL ||
+ cmdp->cmdtype != CMDFUNCTION)
error("%s not defined in %s", name, fullname);
stunalloc(fullname);
goto success;
}
#ifdef notdef
+ /* XXX this code stops root executing stuff, and is buggy
+ if you need a group from the group list. */
if (statb.st_uid == geteuid()) {
if ((statb.st_mode & 0100) == 0)
goto loop;
@@ -545,7 +572,11 @@
#endif
TRACE(("searchexec \"%s\" returns \"%s\"\n", name, fullname));
INTOFF;
- cmdp = cmdlookup(name, 1);
+ if (act & DO_ALTPATH) {
+ stalloc(strlen(fullname) + 1);
+ cmdp = &loc_cmd;
+ } else
+ cmdp = cmdlookup(name, 1);
cmdp->cmdtype = CMDNORMAL;
cmdp->param.index = idx;
INTON;
@@ -560,6 +591,15 @@
entry->cmdtype = CMDUNKNOWN;
return;
+builtin_success:
+ INTOFF;
+ if (act & DO_ALTPATH)
+ cmdp = &loc_cmd;
+ else
+ cmdp = cmdlookup(name, 1);
+ cmdp->cmdtype = CMDBUILTIN;
+ cmdp->param.bltin = bltin;
+ INTON;
success:
cmdp->rehash = 0;
entry->cmdtype = cmdp->cmdtype;
@@ -643,9 +683,10 @@
/*
+ * Fix command hash table when PATH changed.
* Called before PATH is changed. The argument is the new value of PATH;
- * pathval() still returns the old value at this point. Called with
- * interrupts off.
+ * pathval() still returns the old value at this point.
+ * Called with interrupts off.
*/
void
@@ -844,7 +885,7 @@
* the same name - except special builtins.
*/
-void
+STATIC void
addcmdentry(char *name, struct cmdentry *entry)
{
struct tblentry *cmdp;
@@ -888,7 +929,8 @@
{
struct tblentry *cmdp;
- if ((cmdp = cmdlookup(name, 0)) != NULL && cmdp->cmdtype == CMDFUNCTION) {
+ if ((cmdp = cmdlookup(name, 0)) != NULL &&
+ cmdp->cmdtype == CMDFUNCTION) {
freefunc(cmdp->param.func);
delete_cmd_entry();
return (0);
@@ -932,8 +974,7 @@
if ((cmdp = cmdlookup(argv[i], 0)) != NULL) {
entry.cmdtype = cmdp->cmdtype;
entry.u = cmdp->param;
- }
- else {
+ } else {
/* Finally use brute force */
find_command(argv[i], &entry, DO_ABS, pathval());
}
Index: exec.h
===================================================================
RCS file: /cvsroot/basesrc/bin/sh/exec.h,v
retrieving revision 1.19
diff -u -r1.19 exec.h
--- exec.h 2002/11/24 22:35:39 1.19
+++ exec.h 2002/12/18 23:57:27
@@ -39,11 +39,11 @@
*/
/* values of cmdtype */
-#define CMDUNKNOWN -1 /* no entry in table for command */
-#define CMDNORMAL 0 /* command is an executable program */
-#define CMDFUNCTION 1 /* command is a shell function */
-#define CMDBUILTIN 2 /* command is a shell builtin */
-#define CMDSPLBLTIN 3 /* command is a special shell builtin */
+#define CMDUNKNOWN -1 /* no entry in table for command */
+#define CMDNORMAL 0 /* command is an executable program */
+#define CMDFUNCTION 1 /* command is a shell function */
+#define CMDBUILTIN 2 /* command is a shell builtin */
+#define CMDSPLBLTIN 3 /* command is a special shell builtin */
struct cmdentry {
@@ -56,8 +56,12 @@
};
-#define DO_ERR 1 /* find_command prints errors */
-#define DO_ABS 2 /* find_command checks absolute paths */
+/* action to find_command() */
+#define DO_ERR 0x01 /* prints errors */
+#define DO_ABS 0x02 /* checks absolute paths */
+#define DO_NOFUNC 0x04 /* don't return shell functions, for command */
+#define DO_ALTPATH 0x08 /* using alternate path */
+#define DO_ALTBLTIN 0x20 /* %builtin in alt. path */
extern const char *pathopt; /* set by padvance */
Index: jobs.h
===================================================================
RCS file: /cvsroot/basesrc/bin/sh/jobs.h,v
retrieving revision 1.16
diff -u -r1.16 jobs.h
--- jobs.h 2002/11/24 22:35:40 1.16
+++ jobs.h 2002/12/18 23:57:27
@@ -73,12 +73,6 @@
char cmd[MAXCMDTEXT];/* text of command being run */
};
-
-/* states */
-#define JOBSTOPPED 1 /* all procs are stopped */
-#define JOBDONE 2 /* all procs are completed */
-
-
struct job {
struct procstat ps0; /* status of process */
struct procstat *ps; /* status or processes when more than one */
Index: memalloc.c
===================================================================
RCS file: /cvsroot/basesrc/bin/sh/memalloc.c,v
retrieving revision 1.26
diff -u -r1.26 memalloc.c
--- memalloc.c 2002/11/24 22:35:41 1.26
+++ memalloc.c 2002/12/18 23:57:29
@@ -90,7 +90,7 @@
*/
char *
-savestr(char *s)
+savestr(const char *s)
{
char *p;
Index: memalloc.h
===================================================================
RCS file: /cvsroot/basesrc/bin/sh/memalloc.h,v
retrieving revision 1.12
diff -u -r1.12 memalloc.h
--- memalloc.h 2002/11/24 22:35:41 1.12
+++ memalloc.h 2002/12/18 23:57:29
@@ -53,7 +53,7 @@
pointer ckmalloc(int);
pointer ckrealloc(pointer, int);
-char *savestr(char *);
+char *savestr(const char *);
pointer stalloc(int);
void stunalloc(pointer);
void setstackmark(struct stackmark *);
Index: options.h
===================================================================
RCS file: /cvsroot/basesrc/bin/sh/options.h,v
retrieving revision 1.15
diff -u -r1.15 options.h
--- options.h 2002/11/24 22:35:42 1.15
+++ options.h 2002/12/18 23:57:30
@@ -99,9 +99,11 @@
#define qflag optlist[15].val
DEF_OPT( "nolog", 0 ) /* [U] no functon defs in command history */
#define nolog optlist[16].val
+DEF_OPT( "cdprint", 0 ) /* always print result of cd */
+#define cdprint optlist[17].val
#ifdef DEBUG
-DEF_OPT( "debug", 0 )
-#define debug optlist[17].val
+DEF_OPT( "debug", 0 ) /* enable debug prints */
+#define debug optlist[18].val
#endif
#ifdef DEFINE_OPTIONS
Index: shell.h
===================================================================
RCS file: /cvsroot/basesrc/bin/sh/shell.h,v
retrieving revision 1.15
diff -u -r1.15 shell.h
--- shell.h 2002/09/27 18:56:56 1.15
+++ shell.h 2002/12/18 23:57:31
@@ -44,12 +44,12 @@
* SHORTNAMES -> 1 if your linker cannot handle long names.
* define BSD if you are running 4.2 BSD or later.
* define SYSV if you are running under System V.
- * define DEBUG=1 to compile in debugging (set global "debug" to turn on)
+ * define DEBUG=1 to compile in debugging ('set -o debug' to turn on)
* define DEBUG=2 to compile in and turn on debugging.
* define DO_SHAREDVFORK to indicate that vfork(2) shares its address
* with its parent.
*
- * When debugging is on, debugging info will be written to $HOME/trace and
+ * When debugging is on, debugging info will be written to ./trace and
* a quit signal will generate a core dump.
*/
Index: var.c
===================================================================
RCS file: /cvsroot/basesrc/bin/sh/var.c,v
retrieving revision 1.31
diff -u -r1.31 var.c
--- var.c 2002/11/25 12:13:03 1.31
+++ var.c 2002/12/18 23:57:33
@@ -125,7 +125,7 @@
{ &vterm, VSTRFIXED|VTEXTFIXED|VUNSET, "TERM=",
setterm },
#endif
- { &voptind, VSTRFIXED|VTEXTFIXED, "OPTIND=1",
+ { &voptind, VSTRFIXED|VTEXTFIXED|VNOFUNC, "OPTIND=1",
getoptsreset },
{ NULL, 0, NULL,
NULL }
@@ -297,7 +297,7 @@
ckfree(vp->text);
vp->flags &= ~(VTEXTFIXED|VSTACK|VUNSET);
- vp->flags |= flags;
+ vp->flags |= flags & ~VNOFUNC;
vp->text = s;
/*
@@ -314,7 +314,7 @@
if (flags & VNOSET)
return;
vp = ckmalloc(sizeof (*vp));
- vp->flags = flags;
+ vp->flags = flags & ~VNOFUNC;
vp->text = s;
vp->next = *vpp;
vp->func = NULL;
@@ -339,6 +339,14 @@
INTON;
}
+void
+listmklocal(struct strlist *list, int flags)
+{
+ struct strlist *lp;
+
+ for (lp = list ; lp ; lp = lp->next)
+ mklocal(lp->text, flags);
+}
/*
@@ -628,7 +636,7 @@
*/
void
-mklocal(char *name, int flags)
+mklocal(const char *name, int flags)
{
struct localvar *lvp;
struct var **vpp;
@@ -686,6 +694,8 @@
} else if ((lvp->flags & (VUNSET|VSTRFIXED)) == VUNSET) {
(void)unsetvar(vp->text, 0);
} else {
+ if (vp->func && (vp->flags & VNOFUNC) == 0)
+ (*vp->func)(strchr(lvp->text, '=') + 1);
if ((vp->flags & VTEXTFIXED) == 0)
ckfree(vp->text);
vp->flags = lvp->flags;
Index: var.h
===================================================================
RCS file: /cvsroot/basesrc/bin/sh/var.h,v
retrieving revision 1.20
diff -u -r1.20 var.h
--- var.h 2002/11/24 22:35:43 1.20
+++ var.h 2002/12/18 23:57:34
@@ -124,7 +124,8 @@
int showvars(const char *, int, int);
int exportcmd(int, char **);
int localcmd(int, char **);
-void mklocal(char *, int);
+void mklocal(const char *, int);
+void listmklocal(struct strlist *, int);
void poplocalvars(void);
int setvarcmd(int, char **);
int unsetcmd(int, char **);
--
David Laight: david@l8s.co.uk