Skip to content

Commit

Permalink
Merge pull request att#7 from JohnoKing/fix-special-vars
Browse files Browse the repository at this point in the history
Implement a better fix for unsetting special env vars.

See att#7 for more explanations and ksh source code history :)
McDutchie authored Jun 13, 2020
2 parents 289f56c + 7b994b6 commit a739115
Showing 3 changed files with 29 additions and 14 deletions.
25 changes: 17 additions & 8 deletions src/cmd/ksh93/bltins/typeset.c
Original file line number Diff line number Diff line change
@@ -1208,11 +1208,9 @@ static int unall(int argc, char **argv, register Dt_t *troot, Shell_t* shp)
}

/*
* Check for variables with internal trap/discipline functions (PATH, LANG, LC_*, LINENO, etc.).
* Unsetting these in a virtual/non-forked subshell would cause them to lose their discipline actions,
* so, for example, (unset PATH; PATH=/dev/null; ls) would run 'ls'! Until a fix is found, make the problem
* go away by forking the subshell. To avoid crashing, this must be done before calling sh_pushcontext(),
* so we need to loop through the args separately to check if any variable to unset has a discipline function.
* Unsetting the PATH in a non-forked subshell will cause the parent shell's hash table to be reset,
* so fork to work around the problem. To avoid crashing, this must be done before calling sh_pushcontext(),
* so we need to loop through the args separately and check if any node is equal to PATHNOD.
*/
if(shp->subshell && !shp->subshare && troot==shp->var_tree)
{
@@ -1222,8 +1220,8 @@ static int unall(int argc, char **argv, register Dt_t *troot, Shell_t* shp)
np=nv_open(name,troot,NV_NOADD|nflag);
if(!np)
continue;
/* Has discipline function, and is not a nameref? */
if(np->nvfun && np->nvfun->disc && !nv_isref(np))
/* Are we changing the PATH? */
if(np==PATHNOD)
{
nv_close(np);
sh_subfork();
@@ -1276,7 +1274,18 @@ static int unall(int argc, char **argv, register Dt_t *troot, Shell_t* shp)
}

if(shp->subshell)
np=sh_assignok(np,0);
{
if(!nv_isattr(np,NV_NODISC|NV_ARRAY) && !nv_isvtree(np))
{
/*
* Variables with internal trap/discipline functions (LC_*, LINENO, etc.) need to be
* cloned, as moving them will remove the discipline function.
*/
np=sh_assignok(np,1);
}
else
np=sh_assignok(np,0);
}
}

if(!nv_isnull(np) || nv_size(np) || nv_isattr(np,~(NV_MINIMAL|NV_NOFREE)))
2 changes: 1 addition & 1 deletion src/cmd/ksh93/sh/nvdisc.c
Original file line number Diff line number Diff line change
@@ -962,7 +962,7 @@ int nv_clone(Namval_t *np, Namval_t *mp, int flags)
{
if(nv_isattr(np,NV_INTEGER))
mp->nvalue.ip = np->nvalue.ip;
np->nvfun = 0;
np->nvfun = 0; /* This will remove the discipline function, if there is one */
np->nvalue.cp = 0;
if(!nv_isattr(np,NV_MINIMAL) || nv_isattr(mp,NV_EXPORT))
{
16 changes: 11 additions & 5 deletions src/cmd/ksh93/tests/subshell.sh
Original file line number Diff line number Diff line change
@@ -710,13 +710,19 @@ v=${ eval 'al'; alias al='echo subshare'; } && [[ $v == 'mainalias' && $(eval 'a
|| 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'
check_hash_table()
{
[[ $(hash) ]] || err_exit $'resetting the hash table in a subshell affects the parent shell\'s hash table'
# Ensure the hash table isn't empty before the next test is run
hash -r chmod
}

(hash -r); check_hash_table
(PATH="$PATH"); check_hash_table
(unset PATH); check_hash_table
(nameref PATH_TWO=PATH; unset PATH_TWO); check_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'

0 comments on commit a739115

Please sign in to comment.