Skip to content

Commit

Permalink
Multiple 'whence' and path search fixes
Browse files Browse the repository at this point in the history
Hopefully this doesn't introduce new bugs, but it does fix at
least the following:

1. When whence -v/-a found an "undefined" (i.e. autoloadable)
   function in $FPATH, it actually loaded the function as a side
   effect of reporting on its existence (!). Now it only reports.

2. 'whence' will now canonicalise paths properly. Examples:
	$ whence ///usr/lib/../bin//./env
	/usr/bin/env
	$ (cd /; whence -v dev/../usr/bin//./env)
	dev/../usr/bin//./env is /usr/bin/env

3. 'whence' no longer prefixes a spurious double slash when doing
   something like 'cd / && whence bin/echo'. On Cygwin, an initial
   double slash denotes a network server, so this was not just a
   cosmetic problem.

4. 'whence -a' now reports a "tracked alias" (a.k.a. hash table
   entry, i.e. cached $PATH search) even if an actual alias by the
   same name exists. This needed fixing because in fact the hash
   table entry continues to be used when bypassing the alias.
   Aliases and "tracked aliases" are not remotely the same thing;
   confusing nomenclature is not a reason to report wrong results.

5. When using 'hash' or 'alias -t' on a command that is also a
   builtin to force caching a $PATH search for the external
   command, 'whence -a' double-reported the path:
	$ hash printf; whence -a printf
	printf is a shell builtin
	printf is /usr/bin/printf
	printf is a tracked alias for /usr/bin/printf
   This is now fixed so that the second output line is gone.
   Plus, if there were multiple versions of the command on $PATH,
   the tracked alias was reported at the end, which is the wrong
   order. This is also fixed.

src/cmd/ksh93/bltins/whence.c: whence():
- Refactor the do...while loop that handles whence -v/-a for path
  searches in such a way that the code actually makes sense and
  stops looking like higher esotericism. Just doing this fixed #2,
  #4 and #5 above (the latter two before I even noticed them). For
  instance, the path_fullname() call to canonicalise paths was
  already there; it was just never used.
- Remove broken 'notrack' flaggery for deciding whether to report a
  hash table entry a.k.a. "tracked alias"; instead, check the hash
  table (shp->track_tree).

src/cmd/ksh93/sh/path.c:
- path_search(): Re #3: When prefixing the PWD, first check if
  we're in '/' and if so, don't prefix it; otherwise, adding the
  next slash causes an initial double slash. (Since '/' is the only
  valid single-character absolute path, all we need to do is check
  if the second character pwd[1] is non-null.)
- path_search(): Re #1: Stop autoloading when called by 'whence':
  * The 'flag==2' check to avoid autoloading a function was
    broken. The flag value is 2 on the first whence() loop
    iteration, but 3 on subsequent ones. Change to 'flag >= 2'.
  * However, this only fixes it if the function file does not have
    the x permission bit, as executable files are handled by
    path_absolute() which unconditionally autoloads functions!
    So, pass on our flag parameter when callling path_absolute().
- path_absolute(): Re #1: Add flag parameter. Do not autoload
  functions if flag >= 2.

src/cmd/ksh93/include/path.h,
src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/sh/main.c,
src/cmd/ksh93/sh/xec.c:
- Re #1: Update path_absolute() calls, adding a 0 flag parameter.

src/cmd/ksh93/include/name.h:
- Remove now-unused pathcomp member from union Value. It was
  introduced in 9906535 to allow examining the value of a tracked
  alias. This commit uses nv_getval() instead.

src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/path.sh:
- Add and tweak various related tests.

Fixes: #84
  • Loading branch information
McDutchie committed Sep 20, 2020
1 parent 95fc175 commit a329c22
Show file tree
Hide file tree
Showing 11 changed files with 192 additions and 106 deletions.
10 changes: 10 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,16 @@ For full details, see the git log at: https://github.com/ksh93/ksh

Any uppercase BUG_* names are modernish shell bug IDs.

2020-09-20:

- Bugfix: when whence -v/-a found an "undefined" (i.e. autoloadable) function
in $FPATH, it actually loaded the function as a side effect of reporting on
its existence. Now it only reports, as documented.

- 'whence' will now canonicalise paths properly, resolving '.' and '..'
elements in paths given to it. It also no longer prefixes a spurious
double slash when doing something like 'cd / && whence bin/echo'.

2020-09-18:

- Setting the 'posix' option now turns off the 'braceexpand' option, as brace
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/ksh93/bltins/typeset.c
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ static int setall(char **argv,register int flag,Dt_t *troot,struct tdata *tp
if(troot==shp->track_tree && tp->aflag=='-')
{
np = nv_search(name,troot,NV_ADD);
path_alias(np,path_absolute(shp,nv_name(np),NIL(Pathcomp_t*)));
path_alias(np,path_absolute(shp,nv_name(np),NIL(Pathcomp_t*),0));
continue;
}
np = nv_open(name,troot,nvflags|((nvflags&NV_ASSIGN)?0:NV_ARRAY)|((iarray|(nvflags&(NV_REF|NV_NOADD)==NV_REF))?NV_FARRAY:0));
Expand Down Expand Up @@ -990,7 +990,7 @@ int b_builtin(int argc,char *argv[],Shbltin_t *context)
tdata.sh = context->shp;
stkp = tdata.sh->stk;
if(!tdata.sh->pathlist)
path_absolute(tdata.sh,argv[0],NIL(Pathcomp_t*));
path_absolute(tdata.sh,argv[0],NIL(Pathcomp_t*),0);
while (n = optget(argv,sh_optbuiltin)) switch (n)
{
case 's':
Expand Down
88 changes: 44 additions & 44 deletions src/cmd/ksh93/bltins/whence.c
Original file line number Diff line number Diff line change
Expand Up @@ -137,16 +137,13 @@ static int whence(Shell_t *shp,char **argv, register int flags)
register const char *cp;
register int aflag,r=0;
register const char *msg;
int tofree;
Namval_t *nq;
char *notused;
Pathcomp_t *pp=0;
int notrack = 1;
if(flags&Q_FLAG)
flags &= ~A_FLAG;
while(name= *argv++)
{
tofree=0;
aflag = ((flags&A_FLAG)!=0);
cp = 0;
np = 0;
Expand All @@ -164,7 +161,7 @@ static int whence(Shell_t *shp,char **argv, register int flags)
}
/* non-tracked aliases */
if((np=nv_search(name,shp->alias_tree,0))
&& !nv_isnull(np) && !(notrack=nv_isattr(np,NV_TAGGED))
&& !nv_isnull(np) && !nv_isattr(np,NV_TAGGED)
&& (cp=nv_getval(np)))
{
if(flags&V_FLAG)
Expand Down Expand Up @@ -213,81 +210,84 @@ static int whence(Shell_t *shp,char **argv, register int flags)
aflag++;
}
search:
if(sh_isstate(SH_DEFPATH))
{
cp=0;
notrack=1;
}
do
{
if(path_search(shp,name,&pp,2+(aflag>1)))
int maybe_undef_fn = 0; /* flag for possible undefined (i.e. autoloadable) function */
/*
* See comments in sh/path.c for info on what path_search()'s true/false return values mean
*/
if(path_search(shp, name, &pp, aflag>1 ? 3 : 2))
{
cp = name;
if((flags&P_FLAG) && *cp!='/')
cp = 0;
if(*cp!='/')
{
if(flags&P_FLAG)
cp = 0;
else
maybe_undef_fn = 1;
}
}
else
{
cp = stakptr(PATH_OFFSET);
if(*cp==0)
cp = 0;
else if(*cp!='/')
{
cp = path_fullname(shp,cp);
tofree=1;
}
}
if(flags&Q_FLAG)
{
pp = 0;
r |= !cp;
}
else if(maybe_undef_fn)
{
/* Skip defined function or builtin (already done above) */
if(!nv_search(cp,shp->fun_tree,0))
{
/* Undefined/autoloadable function on FPATH */
sfputr(sfstdout,sh_fmtq(cp),-1);
if(flags&V_FLAG)
sfputr(sfstdout,sh_translate(is_ufunction),-1);
sfputc(sfstdout,'\n');
}
}
else if(cp)
{
cp = path_fullname(shp,cp); /* resolve '.' & '..' */
if(flags&V_FLAG)
{
if(*cp!= '/')
{
if(!np && (np=nv_search(name,shp->track_tree,0)))
{
const char *command_path = np->nvalue.pathcomp->name;
sfprintf(sfstdout,"%s %s %s/%s\n",name,sh_translate(is_talias),command_path,cp);
}
continue;
}
sfputr(sfstdout,sh_fmtq(name),' ');
/* built-in version of program */
if(*cp=='/' && (np=nv_search(cp,shp->bltin_tree,0)))
if(nv_search(cp,shp->bltin_tree,0))
msg = sh_translate(is_builtver);
/* tracked aliases next */
else if(aflag>1 || !notrack || strchr(name,'/'))
msg = sh_translate("is");
else
else if(!sh_isstate(SH_DEFPATH)
&& (np = nv_search(name,shp->track_tree,0))
&& !nv_isattr(np,NV_NOALIAS)
&& strcmp(cp,nv_getval(np))==0)
msg = sh_translate(is_talias);
else
msg = sh_translate("is");
sfputr(sfstdout,msg,' ');
}
sfputr(sfstdout,sh_fmtq(cp),'\n');
if(aflag)
{
if(aflag<=1)
aflag++;
if (pp)
pp = pp->next;
}
else
pp = 0;
if(tofree)
{
free((char*)cp);
tofree = 0;
}
free((char*)cp);
}
else if(aflag<=1)
{
r |= 1;
if(flags&V_FLAG)
errormsg(SH_DICT,ERROR_exit(0),e_found,sh_fmtq(name));
}
/* If -a given, continue with next result */
if(aflag)
{
if(aflag<=1)
aflag++;
if(pp)
pp = pp->next;
}
else
pp = 0;
} while(pp);
}
return(r);
Expand Down
2 changes: 0 additions & 2 deletions src/cmd/ksh93/include/name.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
#include <cdt.h>

typedef int (*Nambfp_f)(int, char**, void*);
struct pathcomp;

/* Nodes can have all kinds of values */
union Value
Expand All @@ -55,7 +54,6 @@ union Value
struct Namfun *funp; /* discipline pointer */
struct Namref *nrp; /* name reference */
Nambfp_f bfp; /* builtin entry point function pointer */
struct pathcomp *pathcomp;
};

#include "nval.h"
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/include/path.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ extern Pathcomp_t *path_addpath(Shell_t*,Pathcomp_t*,const char*,int);
extern Pathcomp_t *path_dup(Pathcomp_t*);
extern void path_delete(Pathcomp_t*);
extern void path_alias(Namval_t*,Pathcomp_t*);
extern Pathcomp_t *path_absolute(Shell_t*, const char*, Pathcomp_t*);
extern Pathcomp_t *path_absolute(Shell_t*, const char*, Pathcomp_t*, int);
extern char *path_basename(const char*);
extern char *path_fullname(Shell_t*,const char*);
extern int path_expand(Shell_t*,const char*, struct argnod**);
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/include/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@
* David Korn <[email protected]> *
* *
***********************************************************************/
#define SH_RELEASE "93u+m 2020-09-18"
#define SH_RELEASE "93u+m 2020-09-20"
2 changes: 1 addition & 1 deletion src/cmd/ksh93/sh/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ int sh_main(int ac, char *av[], Shinit_f userinit)
if(fdin < 0 && !strchr(name,'/'))
{
#ifdef PATH_BFPATH
if(path_absolute(shp,name,NIL(Pathcomp_t*)))
if(path_absolute(shp,name,NIL(Pathcomp_t*),0))
sp = stakptr(PATH_OFFSET);
#else
sp = path_absolute(shp,name,NIL(char*));
Expand Down
45 changes: 33 additions & 12 deletions src/cmd/ksh93/sh/path.c
Original file line number Diff line number Diff line change
Expand Up @@ -643,12 +643,26 @@ static void funload(Shell_t *shp,int fno, const char *name)

/*
* do a path search and track alias if requested
* if flag is 0, or if name not found, then try autoloading function
* if flag==2 or 3, returns 1 if name found on FPATH
* if flag==3 no tracked alias will be set
* returns 1, if function was autoloaded.
*
* If flag is 0, or if name not found, then try autoloading function and return 1 if successful.
* If flag is >=1, do a regular path search. If it yields an autoloadable function, load it.
* If flag is 2 or 3, never autoload a function but return 1 if name found on FPATH.
* If flag is 3, no tracked alias will be set (IOW, the result won't be cached in the hash table).
* If oldpp is not NULL, it will contain a pointer to the path component
* where it was found.
*
* path_search() returns 1/true if:
* - the given absolute path was found executable
* - the given name is a function or non-path-bound builtin, and a path search found nothing external
* - the given name matched an autoloadable function on FPATH
*
* path_search() returns 0/false if:
* - the given relative path was found executable; the PWD is prefixed to make it absolute
* - a tracked alias (a.k.a. hash table entry) was found and used
* - the given name was found on PATH as an executable command or path-bound builtin (irrespective of
* whether it exists as a function or normal builtin); its full path is written on the stack, except
* if the matching $PATH entry is '.' or empty, the simple name is written without prefixing the PWD
* - nothing executable was found
*/

int path_search(Shell_t *shp,register const char *name,Pathcomp_t **oldpp, int flag)
Expand All @@ -658,6 +672,7 @@ int path_search(Shell_t *shp,register const char *name,Pathcomp_t **oldpp, int f
Pathcomp_t *pp=0;
if(name && strchr(name,'/'))
{
char *pwd;
stakseek(PATH_OFFSET);
stakputs(name);
if(canexecute(shp,stakptr(PATH_OFFSET),0)<0)
Expand All @@ -668,7 +683,9 @@ int path_search(Shell_t *shp,register const char *name,Pathcomp_t **oldpp, int f
if(*name=='/')
return(1);
stakseek(PATH_OFFSET);
stakputs(path_pwd(shp,1));
pwd = path_pwd(shp,1);
if(pwd[1]) /* if pwd=="/", avoid starting with "//" */
stakputs(pwd);
stakputc('/');
stakputs(name);
stakputc(0);
Expand Down Expand Up @@ -697,7 +714,7 @@ int path_search(Shell_t *shp,register const char *name,Pathcomp_t **oldpp, int f
stakputc(0);
return(0);
}
pp = path_absolute(shp,name,oldpp?*oldpp:NIL(Pathcomp_t*));
pp = path_absolute(shp,name,oldpp?*oldpp:NIL(Pathcomp_t*),flag);
if(oldpp)
*oldpp = pp;
if(!pp && (np=nv_search(name,shp->fun_tree,0))&&np->nvalue.ip)
Expand All @@ -711,7 +728,7 @@ int path_search(Shell_t *shp,register const char *name,Pathcomp_t **oldpp, int f
pp=sh_isstate(SH_DEFPATH)?shp->defpathlist:shp->pathlist;
if(pp && strmatch(name,e_alphanum) && (fno=path_opentype(shp,name,pp,1))>=0)
{
if(flag==2)
if(flag >= 2)
{
sh_close(fno);
return(1);
Expand All @@ -732,8 +749,10 @@ int path_search(Shell_t *shp,register const char *name,Pathcomp_t **oldpp, int f

/*
* do a path search and find the full pathname of file name
*
* If flag >= 2, do not autoload functions (cf. path_search()).
*/
Pathcomp_t *path_absolute(Shell_t *shp,register const char *name, Pathcomp_t *pp)
Pathcomp_t *path_absolute(Shell_t *shp,register const char *name, Pathcomp_t *pp, int flag)
{
register int f,isfun;
int noexec=0;
Expand Down Expand Up @@ -874,10 +893,12 @@ Pathcomp_t *path_absolute(Shell_t *shp,register const char *name, Pathcomp_t *pp
}
if(isfun && f>=0)
{
nv_onattr(nv_open(name,sh_subfuntree(1),NV_NOARRAY|NV_IDENT|NV_NOSCOPE),NV_LTOU|NV_FUNCTION);
funload(shp,f,name);
close(f);
f = -1;
if(flag < 2)
{
nv_onattr(nv_open(name,sh_subfuntree(1),NV_NOARRAY|NV_IDENT|NV_NOSCOPE),NV_LTOU|NV_FUNCTION);
funload(shp,f,name);
close(f);
}
return(0);
}
else if(f>=0 && (oldpp->flags & PATH_STD_DIR))
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/sh/xec.c
Original file line number Diff line number Diff line change
Expand Up @@ -3619,7 +3619,7 @@ static pid_t sh_ntfork(Shell_t *shp,const Shnode_t *t,char *argv[],int *jobid,in
&& !nv_isattr(np,NV_NOALIAS)
&& np->nvalue.cp)
path = nv_getval(np);
else if(path_absolute(shp,path,NIL(Pathcomp_t*)))
else if(path_absolute(shp,path,NIL(Pathcomp_t*),0))
{
path = stkptr(shp->stk,PATH_OFFSET);
stkfreeze(shp->stk,0);
Expand Down
Loading

0 comments on commit a329c22

Please sign in to comment.