1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-02-13 03:32:24 +00:00

Fix unsetting special vars in subshells (re: efa31503, 8b7f8f9b)

This fixes (or at least works around) a bug that caused special
variables such as PATH, LANG, LC_ALL, LINENO, etc. to lose their
effect after being unset in a subshell.

For example:
(unset PATH; PATH=/dev/null; ls); : wrongly ran 'ls'
(unset LC_ALL; LC_ALL=badlocale); : failed to print a diagnostic

This is yet another problem with non-forking/virtual subshells. If
you forced the subshell to fork (one way of doing this is using the
'ulimit' builtin, e.g. ulimit -t unlimited) before unsetting the
special variable, the problem vanished.

I've tried to localise the problem. I suspect the sh_assignok()
function, which is called from unall(), is to blame. This function
is supposed to make a copy of a variable node in the virtual
subshell's variable tree. Apparently, it fails to copy the
associated permanent discipline function settings (stored in the
np->nvfun->disc pointer) that gave these variables their special
effect, and which survive unset. However, my attempts to fix that
have been unsuccessful. If anyone can figure out a fix, please send
a patch/pull request!
Data point: This bug existed in 93u 2011-02-08, but did not yet
exist in M-1993-12-28-s+. So it is a regression.

Meanwhile, pending a proper fix, this commit adds a safe
workaround: it forces a non-forked subshell to fork before
unsetting such a special variable.

src/cmd/ksh93/bltins/typeset.c: unall():
- If we're in a non-forked, non-${ ...; } subshell, then before
  unsetting any variable, check for variables with internal
  trap/discipline functions, and call sh_subfork() if any are
  found. To avoid crashing, this must be done before calling
  sh_pushcontext(), so we need to loop through the args separately.

src/cmd/ksh93/tests/variables.sh:
- Remove the 'ulimit' that forced the fork; we do this in C now.

(cherry picked from commit 21b1a67156582e3cbd36936f4af908bb45211a4b)
This commit is contained in:
Martijn Dekker 2020-06-05 15:31:26 +02:00
parent 04722950bb
commit 6f0e008cf7
4 changed files with 35 additions and 4 deletions

7
NEWS
View file

@ -4,6 +4,13 @@ For full details, see the git log at:
Any uppercase BUG_* names are modernish shell bug IDs.
2020-06-05:
- Fix a bug that caused special variables such as PATH, LANG, LC_ALL,
etc. to lose their effect after being unset in a subshell. For example:
(unset PATH; PATH=/dev/null; ls); : wrongly ran 'ls'
(unset LC_ALL; LC_ALL=badlocale); : failed to print a diagnostic
2020-06-04:
- Fix BUG_KBGPID: the $! special parameter was not set if a background job

View file

@ -1209,6 +1209,33 @@ static int unall(int argc, char **argv, register Dt_t *troot, Shell_t* shp)
dtclear(troot);
return(r);
}
/*
* 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.
*/
if(shp->subshell && !shp->subshare && troot==shp->var_tree)
{
char **argv_tmp = argv;
while(name = *argv_tmp++)
{
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))
{
nv_close(np);
sh_subfork();
break;
}
nv_close(np);
}
}
sh_pushcontext(shp,&buff,1);
while(name = *argv++)
{

View file

@ -17,4 +17,4 @@
* David Korn <dgk@research.att.com> *
* *
***********************************************************************/
#define SH_RELEASE "93u+m 2020-06-04"
#define SH_RELEASE "93u+m 2020-06-05"

View file

@ -642,9 +642,6 @@ set --
fi
done
# TODO: changing the locale is somehow broken in non-forked/virtual subshells.
# For now, fork it using ulimit; remove the ulimit to expose the test failures.
ulimit -t unlimited
x=x
for v in LC_ALL LC_CTYPE LC_MESSAGES LC_COLLATE LC_NUMERIC
do nameref r=$v