mirror of
git://git.code.sf.net/p/cdesktopenv/code
synced 2025-02-15 04:32:24 +00:00
Implement a better fix for unsetting special env vars
The regression this commit fixes was first introduced in ksh93t
2008-07-25. It was previously worked around in 6f0e008c
by forking
subshells if any special environment variable is unset.
The reason why this problem doesn't occur in ksh93s+ is because in
that version of ksh sh_assignok never moves nodes, it only clones
them. The second argument doesn't set NV_MOVE, which makes
`sh_assignok(np,0)` is similar to `sh_assignok(np,1)`. In ksh93t and
higher, setting the second argument to zero causes the node to be moved
with NV_MOVE, which causes the discipline function associated with
the variable node to be removed when `np->nvfun` is set to zero (i.e.
NULL). This is why a command like `(unset LC_NUMERIC; LC_NUMERIC=invalid)`
doesn't print a diagnostic, as it looses its discipline function.
This patch fixes the problem by cloning the node with sh_assignok
if it is a special variable with a discipline function. This allows
special variables to work as expected in virtual subshells. The
original workaround has been kept for the $PATH variable only, as
hash tables are still broken in virtual subshells. It has been updated
accordingly to only fork subshells if it detects the variable node
for PATH. I have added two more regression tests for changing the
PATH in subshells to make sure hash tables continue working as
expected with this fix.
src/cmd/ksh93/bltins/typeset.c:
- Only fork virtual subshells if the PATH will be changed. If a
variable is a special variable with a discipline function, it
should be just be cloned, not moved.
src/cmd/ksh93/sh/nvdisc.c:
- Add a comment to clarify that NV_MOVE will delete the discipline
function associated with the node.
src/cmd/ksh93/tests/subshells.sh:
- Add two more regression tests for unsetting the PATH in subshells,
one for if PATH is being pointed to by a nameref. Condense the
hash table tests by moving the main test into a single function.
This commit is contained in:
parent
289f56cd4c
commit
7b994b6a7e
3 changed files with 29 additions and 14 deletions
|
@ -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 the PATH in a non-forked subshell will cause the parent shell's hash table to be reset,
|
||||||
* Unsetting these in a virtual/non-forked subshell would cause them to lose their discipline actions,
|
* so fork to work around the problem. To avoid crashing, this must be done before calling sh_pushcontext(),
|
||||||
* so, for example, (unset PATH; PATH=/dev/null; ls) would run 'ls'! Until a fix is found, make the problem
|
* so we need to loop through the args separately and check if any node is equal to PATHNOD.
|
||||||
* 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.
|
|
||||||
*/
|
*/
|
||||||
if(shp->subshell && !shp->subshare && troot==shp->var_tree)
|
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);
|
np=nv_open(name,troot,NV_NOADD|nflag);
|
||||||
if(!np)
|
if(!np)
|
||||||
continue;
|
continue;
|
||||||
/* Has discipline function, and is not a nameref? */
|
/* Are we changing the PATH? */
|
||||||
if(np->nvfun && np->nvfun->disc && !nv_isref(np))
|
if(np==PATHNOD)
|
||||||
{
|
{
|
||||||
nv_close(np);
|
nv_close(np);
|
||||||
sh_subfork();
|
sh_subfork();
|
||||||
|
@ -1276,7 +1274,18 @@ static int unall(int argc, char **argv, register Dt_t *troot, Shell_t* shp)
|
||||||
}
|
}
|
||||||
|
|
||||||
if(shp->subshell)
|
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)))
|
if(!nv_isnull(np) || nv_size(np) || nv_isattr(np,~(NV_MINIMAL|NV_NOFREE)))
|
||||||
|
|
|
@ -962,7 +962,7 @@ int nv_clone(Namval_t *np, Namval_t *mp, int flags)
|
||||||
{
|
{
|
||||||
if(nv_isattr(np,NV_INTEGER))
|
if(nv_isattr(np,NV_INTEGER))
|
||||||
mp->nvalue.ip = np->nvalue.ip;
|
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;
|
np->nvalue.cp = 0;
|
||||||
if(!nv_isattr(np,NV_MINIMAL) || nv_isattr(mp,NV_EXPORT))
|
if(!nv_isattr(np,NV_MINIMAL) || nv_isattr(mp,NV_EXPORT))
|
||||||
{
|
{
|
||||||
|
|
|
@ -710,13 +710,19 @@ v=${ eval 'al'; alias al='echo subshare'; } && [[ $v == 'mainalias' && $(eval 'a
|
||||||
|| err_exit 'alias redefinition fails to survive ${ ...; }'
|
|| err_exit 'alias redefinition fails to survive ${ ...; }'
|
||||||
|
|
||||||
# Resetting a subshell's hash table should not affect the parent shell
|
# Resetting a subshell's hash table should not affect the parent shell
|
||||||
(hash -r)
|
check_hash_table()
|
||||||
[[ $(hash) ]] || err_exit $'resetting the hash table in a subshell affects the parent shell\'s hash table'
|
{
|
||||||
(PATH="$PATH")
|
[[ $(hash) ]] || err_exit $'resetting the hash table in a subshell affects the parent shell\'s hash table'
|
||||||
[[ $(hash) ]] || err_exit $'resetting the PATH 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
|
# Adding a utility to a subshell's hash table should not affect the parent shell
|
||||||
hash -r chmod
|
|
||||||
(hash cat)
|
(hash cat)
|
||||||
[[ $(hash) == "chmod=$(whence -p chmod)" ]] || err_exit $'changes to a subshell\'s hash table affect the parent shell'
|
[[ $(hash) == "chmod=$(whence -p chmod)" ]] || err_exit $'changes to a subshell\'s hash table affect the parent shell'
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue