From 102868f8507e649970e2a7f58e05ab22d309cf0b Mon Sep 17 00:00:00 2001 From: Johnothan King Date: Wed, 10 Jun 2020 04:00:35 -0700 Subject: [PATCH] Replace the hash alias with a proper builtin This commit replaces the old hash alias with a proper builtin. I based this builtin off of the code alias uses for handling `alias -t --`, but with the hack for `--` removed as it has no use in the new builtin. `alias -t --` will no longer work, that hack is now gone. While I was testing this builtin, I found a bug with hash tables in non-forking subshells. If the hash table of a non-forking subshell is changed, the parent shell's hash table is also changed. As an example, running `(hash -r)` was resetting the parent shell's hash table. The workaround is to force the subshell to fork if the hash table will be changed. src/cmd/ksh93/bltins/typeset.c: - Move the code for hash out of the alias builtin into a dedicated hash builtin. `alias -t --` is no longer supported. src/cmd/ksh93/data/aliases.c: - Remove the old alias for hash from the table of predefined aliases. src/cmd/ksh93/data/builtins.c: - Fix the broken entry for the hash builtin and add a man page for the new builtin. src/cmd/ksh93/sh.1: - Replace the entry for the hash alias with a more detailed entry for the hash builtin. src/cmd/ksh93/sh/name.c: - Force non-forking subshells to fork if the PATH is being reset to workaround a bug with the hash tree. src/cmd/ksh93/tests/alias.sh: - Add a regression test for resetting a hash table, then adding a utility to the refreshed hash table. src/cmd/ksh93/tests/subshell.sh: - Add regression tests for changing the hash table in subshells. (cherry picked from commit d8428a833afe9270b61745ba3d6df355fe1d5499) --- NEWS | 10 +++++ TODO | 5 --- src/cmd/ksh93/bltins/typeset.c | 65 +++++++++++++++++++++----------- src/cmd/ksh93/data/aliases.c | 1 - src/cmd/ksh93/data/builtins.c | 25 +++++++++++- src/cmd/ksh93/include/builtins.h | 2 + src/cmd/ksh93/sh.1 | 17 ++++++++- src/cmd/ksh93/sh/name.c | 12 +++++- src/cmd/ksh93/tests/alias.sh | 5 +++ src/cmd/ksh93/tests/subshell.sh | 11 ++++++ 10 files changed, 121 insertions(+), 32 deletions(-) diff --git a/NEWS b/NEWS index 5090b44a5..284856f85 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,16 @@ For full details, see the git log at: Any uppercase BUG_* names are modernish shell bug IDs. +2020-06-10: + +- The 'hash' utility is now a regular builtin instead of an alias to + 'alias -t --'. The functionality of the old command has been removed + from the alias builtin. + +- Changing the hash table in a subshell will no longer affect the parent + shell's hash table. This fix applies to the hash utility and when the + PATH is reset manually. + 2020-06-09: - The 'unalias' builtin will now return a non-zero status if it tries diff --git a/TODO b/TODO index 11d643256..bedce718f 100644 --- a/TODO +++ b/TODO @@ -20,11 +20,6 @@ Fix build system: ______ Fix or remove broken or misguided default aliases: -- Make a proper POSIX 'hash' builtin command and remove the builtin alias - hash='alias -t --', which, when removed, exposes a b0rken 'hash' builtin. - To work with this alias, the 'alias' builtin parses the '-r' for 'hash' - *after* '--' ('alias -t -- -r'), which is utterly bogus and violates POSIX. - - Make a proper builtin out of the redirect='command exec' alias. It should really only parse redirections. Currently, if an unwitting user notices this alias and tries out something like 'redirect ls >file', it does 'exec ls diff --git a/src/cmd/ksh93/bltins/typeset.c b/src/cmd/ksh93/bltins/typeset.c index d8623e3f3..b22365650 100644 --- a/src/cmd/ksh93/bltins/typeset.c +++ b/src/cmd/ksh93/bltins/typeset.c @@ -24,6 +24,7 @@ * typeset [options] [arg...] * alias [-ptx] [arg...] * unalias [arg...] + * hash [-r] [utility...] * builtin [-sd] [-f file] [name...] * set [options] [name...] * unset [-fnv] [name...] @@ -132,6 +133,49 @@ int b_readonly(int argc,char *argv[],Shbltin_t *context) return(setall(argv,flag,tdata.sh->var_tree, &tdata)); } +int b_hash(int argc,char *argv[],Shbltin_t *context) +{ + register unsigned flag = NV_NOARRAY | NV_NOSCOPE | NV_ASSIGN | NV_TAGGED; + register Dt_t *troot; + register int rflag=0, n; + struct tdata tdata; + NOT_USED(argc); + memset((void *)&tdata, 0, sizeof(tdata)); + tdata.sh = context->shp; + troot = tdata.sh->alias_tree; + + /* The hash utility should not be recognized as the alias builtin */ + tdata.aflag = '-'; + + while((n = optget(argv,sh_opthash))) switch(n) + { + case 'r': + rflag = 1; + break; + case ':': + error(2,"%s",opt_info.arg); + break; + case '?': + error(ERROR_usage(0),"%s",opt_info.arg); + return 2; + } + if (error_info.errors) + errormsg(SH_DICT,ERROR_usage(2),"%s",optusage(NULL)); + argv += (opt_info.index-1); + + /* Handle -r */ + if (rflag) + { + Namval_t *np = nv_search((char *)PATHNOD, tdata.sh->var_tree, HASH_BUCKET); + nv_putval(np, nv_getval(np), NV_RDONLY); + } + + troot = tdata.sh->track_tree; + /* We must fork in subshells to avoid polluting the parent shell's hash table. */ + if(tdata.sh->subshell && !tdata.sh->subshare) + sh_subfork(); + return setall(argv, flag, troot, &tdata); +} int b_alias(int argc,register char *argv[],Shbltin_t *context) { @@ -174,28 +218,7 @@ int b_alias(int argc,register char *argv[],Shbltin_t *context) errormsg(SH_DICT,ERROR_usage(2),"%s",optusage(NIL(char*))); argv += (opt_info.index-1); if(flag&NV_TAGGED) - { - /* hacks to handle hash -r | -- */ - if(argv[1] && argv[1][0]=='-') - { - if(argv[1][1]=='r' && argv[1][2]==0) - { - Namval_t *np = nv_search((char*)PATHNOD,tdata.sh->var_tree,HASH_BUCKET); - nv_putval(np,nv_getval(np),NV_RDONLY); - argv++; - if(!argv[1]) - return(0); - } - if(argv[1][0]=='-') - { - if(argv[1][1]=='-' && argv[1][2]==0) - argv++; - else - errormsg(SH_DICT, ERROR_exit(1), e_option, argv[1]); - } - } troot = tdata.sh->track_tree; - } } if(context->shp->subshell && !context->shp->subshare) sh_subfork(); diff --git a/src/cmd/ksh93/data/aliases.c b/src/cmd/ksh93/data/aliases.c index a081834ca..f76be4114 100644 --- a/src/cmd/ksh93/data/aliases.c +++ b/src/cmd/ksh93/data/aliases.c @@ -33,7 +33,6 @@ const struct shtable2 shtab_aliases[] = "compound", NV_NOFREE|BLT_DCL, "typeset -C", "float", NV_NOFREE|BLT_DCL, "typeset -lE", "functions", NV_NOFREE, "typeset -f", - "hash", NV_NOFREE, "alias -t --", "history", NV_NOFREE, "hist -l", "integer", NV_NOFREE|BLT_DCL, "typeset -li", "nameref", NV_NOFREE|BLT_DCL, "typeset -n", diff --git a/src/cmd/ksh93/data/builtins.c b/src/cmd/ksh93/data/builtins.c index ef64e90fe..39c8724e8 100644 --- a/src/cmd/ksh93/data/builtins.c +++ b/src/cmd/ksh93/data/builtins.c @@ -79,7 +79,7 @@ const struct shtable3 shtab_builtins[] = "newgrp", NV_BLTIN|BLT_ENV|BLT_SPC, Bltin(login), #endif /* _bin_newgrp || _usr_bin_newgrp */ "alias", NV_BLTIN|BLT_SPC, bltin(alias), - "hash", NV_BLTIN|BLT_SPC, bltin(alias), + "hash", NV_BLTIN, bltin(hash), "enum", NV_BLTIN|BLT_ENV|BLT_SPC|BLT_DCL,bltin(enum), "eval", NV_BLTIN|BLT_ENV|BLT_SPC|BLT_EXIT,bltin(eval), "exit", NV_BLTIN|BLT_ENV|BLT_SPC, bltin(return), @@ -377,7 +377,7 @@ USAGE_LICENSE "definition, or an error occurred.]" "}" -"[+SEE ALSO?\bsh\b(1), \bunalias\b(1)]" +"[+SEE ALSO?\bsh\b(1), \bhash(1), \bunalias\b(1)]" ; const char sh_optbuiltin[] = @@ -929,6 +929,27 @@ _JOB_ "[+SEE ALSO?\bwait\b(1), \bps\b(1), \bfg\b(1), \bbg\b(1)]" ; +const char sh_opthash[] = +"[-1c?\n@(#)$Id: hash (ksh community) 2020-06-10 $\n]" +"[+NAME?hash - display the locations of recently used programs]" +"[+DESCRIPTION?The \bhash\b utility is used to display or modify " + "the hash table, which contains the locations of " + "recently used programs. When \bhash\b is given no arguments, " + "it will list all commands in the hash table. If \autility\a is " + "supplied, \bhash\b will add that utility to the hash table.]" +"[r?This option will empty the hash table. The effect of " + "this flag can also be achieved by resetting the value of \bPATH\b.]" +"\n" +"\n[utility...]\n" +"\n" +"[+EXIT STATUS?]{" + "[+0?Successful completion.]" + "[+>0?An error occured.]" +"}" + +"[+SEE ALSO?\bsh\b(1), \balias\b(1)]" +; + const char sh_opthist[] = "[-1cn?@(#)$Id: hist (AT&T Research) 2000-04-02 $\n]" USAGE_LICENSE diff --git a/src/cmd/ksh93/include/builtins.h b/src/cmd/ksh93/include/builtins.h index 72f84e2f9..a93cc9e70 100644 --- a/src/cmd/ksh93/include/builtins.h +++ b/src/cmd/ksh93/include/builtins.h @@ -85,6 +85,7 @@ extern int b_builtin(int, char*[],Shbltin_t*); extern int b_cd(int, char*[],Shbltin_t*); extern int b_command(int, char*[],Shbltin_t*); extern int b_getopts(int, char*[],Shbltin_t*); +extern int b_hash(int, char*[],Shbltin_t*); extern int b_hist(int, char*[],Shbltin_t*); extern int b_let(int, char*[],Shbltin_t*); extern int b_read(int, char*[],Shbltin_t*); @@ -159,6 +160,7 @@ extern const char sh_optgetopts[]; extern const char sh_optbg[]; extern const char sh_optdisown[]; extern const char sh_optfg[]; +extern const char sh_opthash[]; extern const char sh_opthist[]; extern const char sh_optjobs[]; extern const char sh_optkill[]; diff --git a/src/cmd/ksh93/sh.1 b/src/cmd/ksh93/sh.1 index 3732af0be..3136f0b3a 100644 --- a/src/cmd/ksh93/sh.1 +++ b/src/cmd/ksh93/sh.1 @@ -796,8 +796,6 @@ but can be unset or redefined: .TP .B "functions=\(fmtypeset \-f\(fm" .TP -.B "hash=\(fmalias \-t \-\^\-\(fm" -.TP .B "history=\(fmhist \-l\(fm" .TP .B "integer=\(fmtypeset \-li\(fm" @@ -6126,6 +6124,21 @@ The option .B # can only be specified as the first option. .TP +\f3hash\fP \*(OK \f3\-r\fP \f2\^\fP\*(CK \*(OK \f3utility\^\fP \*(CK +The +.BR hash +command can be used to display or modify the hash table. +The hash table is a list of the most recently used programs +and their locations. +If +.I utility\^ +is supplied, +.BR hash +will add that utility to the hash table. +If +.B \-r +is selected, the hash table is reset. +.TP .PD 0 \f3hist\fP \*(OK \f3\-e\fP \f2ename\^\fP \ \*(CK \*(OK \f3\-nlr\^\fP \*(CK \*(OK \f2first\^\fP \*(OK \f2last\^\fP \*(CK \*(CK .TP diff --git a/src/cmd/ksh93/sh/name.c b/src/cmd/ksh93/sh/name.c index 5a70c2503..c7d40542d 100644 --- a/src/cmd/ksh93/sh/name.c +++ b/src/cmd/ksh93/sh/name.c @@ -1605,12 +1605,22 @@ void nv_putval(register Namval_t *np, const char *string, int flags) #endif /* SHOPT_FIXEDARRAY */ if(!(flags&NV_RDONLY) && nv_isattr (np, NV_RDONLY)) errormsg(SH_DICT,ERROR_exit(1),e_readonly, nv_name(np)); - /* The following could cause the shell to fork if assignment + + /* + * Resetting the PATH in a non-forking subshell will reset the parent shell's + * hash table, so work around the problem by forking before sh_assignok + */ + if(shp->subshell && !shp->subshare && np == PATHNOD) + sh_subfork(); + + /* + * The following could cause the shell to fork if assignment * would cause a side effect */ shp->argaddr = 0; if(shp->subshell && !nv_local && !(flags&NV_RDONLY)) np = sh_assignok(np,1); + if(np->nvfun && np->nvfun->disc && !(flags&NV_NODISC) && !nv_isref(np)) { /* This function contains disc */ diff --git a/src/cmd/ksh93/tests/alias.sh b/src/cmd/ksh93/tests/alias.sh index 642034eb9..dab8003c9 100755 --- a/src/cmd/ksh93/tests/alias.sh +++ b/src/cmd/ksh93/tests/alias.sh @@ -105,6 +105,11 @@ fi unalias no_such_alias && err_exit 'unalias should return non-zero for unknown alias' +# ====== +# Adding a utility after resetting the hash table should work +hash -r chmod +[[ $(hash) == "chmod=$(whence -p chmod)" ]] || err_exit $'resetting the hash table with `hash -r \'utility\'` doesn\'t work correctly' + # ====== # Attempting to unalias a previously set alias twice should be an error alias foo=bar diff --git a/src/cmd/ksh93/tests/subshell.sh b/src/cmd/ksh93/tests/subshell.sh index 8187ca933..3162f0699 100755 --- a/src/cmd/ksh93/tests/subshell.sh +++ b/src/cmd/ksh93/tests/subshell.sh @@ -709,5 +709,16 @@ v=$(alias al='echo sub'; eval 'al') && [[ $v == sub ]] || err_exit 'alias fails v=${ eval 'al'; alias al='echo subshare'; } && [[ $v == 'mainalias' && $(eval 'al') == subshare ]] \ || err_exit 'alias redefinition fails to survive ${ ...; }' +# Resetting a subshell's hash table should not affect the parent shell +(hash -r) +[[ $(hash) ]] || err_exit $'resetting the hash table in a subshell affects the parent shell\'s hash table' +(PATH="$PATH") +[[ $(hash) ]] || err_exit $'resetting the PATH in a subshell affects the parent shell\'s hash table' + +# Adding a utility to a subshell's hash table should not affect the parent shell +hash -r chmod +(hash cat) +[[ $(hash) == "chmod=$(whence -p chmod)" ]] || err_exit $'changes to a subshell\'s hash table affect the parent shell' + # ====== exit $((Errors<125?Errors:125))