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

Fix ${.sh.subshell} counter to actually count level of subshells

This counter is documented as follows:
"The current depth for subshells and command substitution."

But before this commit, the actual behaviour was that the counter
was reset to zero whenever a subshell forked for any reason: a
pipe, background job, running 'ulimit', redirecting stdout in a
command substitution, and more. This behaviour was:

1. Not consistent with the documentation. Non-forked (a.k.a.
   virtual) subshells are an internal implementation detail which
   scripts should not have to be concerned with. The manual page
   doesn't mention them at all.

2. Inherently broken. Since a subshell may fork for any number of
   reasons, even mid-run, and those reasons may change with
   bugfixes and further development, scripts have never actually
   been able to rely on the value of ${.sh.subshell}.

So, this commit fixes the counter to count the levels of all
subshells, both virtual and forked.

src/cmd/ksh93/sh/xec.c: _sh_fork():
- Increase ${.sh.subshell} whenever we fork.

src/cmd/ksh93/sh/subshell.c:
- sh_subfork():
  * Fix comment to properly explain what it does. It doesn't
    "create" a subshell, it forks off an existing virtual subshell.
  * Don't zero ${.sh.subshell}. Instead, since sh_fork() increases
    it but we're forking an existing subshell, undo the increase.
- sh_subshell():
  * Remove 'int16_t subshell' variable. It was unnecessary and
    mostly unused. It was also the wrong type: it was assigned the
    value from shp->subshell which is of type short.
  * Increase and decrease the level of virtual subshells and
    ${.sh.subshell} independently.

src/cmd/ksh93/tests/variables.sh:
- Add regression tests for ${.sh.subshell} in virtual and forked
  subshells of several kinds: comsub, parentheses, pipe, bg job.
- Undo wrong error test count fix from 04b4aef0.

(cherry picked from commit a0e0e29e7e0dbf21e4b3958ae02bde6665fb2696)
This commit is contained in:
Martijn Dekker 2020-06-05 21:39:47 +02:00
parent 6c68b53170
commit 8e97419b0b
4 changed files with 33 additions and 10 deletions

4
NEWS
View file

@ -15,6 +15,10 @@ Any uppercase BUG_* names are modernish shell bug IDs.
macOS, by eliminating an invalid/undefined use of memccpy() on overlapping
buffers in the commonly used sfputr() function.
- Fix the ${.sh.subshell} level counter; it is no longer reset to zero when a
non-forked subshell happens to fork into a separate process for some reason
(an internal implementation detail that should be unnoticeable to scripts).
2020-06-04:
- Fix BUG_KBGPID: the $! special parameter was not set if a background job

View file

@ -171,7 +171,7 @@ void sh_subtmpfile(Shell_t *shp)
/*
* This routine creates a temp file if necessary and creates a subshell.
* This routine creates a temp file if necessary, then forks a virtual subshell into a real subshell.
* The parent routine longjmps back to sh_subshell()
* The child continues possibly with its standard output replaced by temp file
*/
@ -210,10 +210,11 @@ void sh_subfork(void)
subshell_data = 0;
shp->subshell = 0;
shp->comsub = 0;
SH_SUBSHELLNOD->nvalue.s = 0;
sp->subpid=0;
shp->st.trapcom[0] = trap;
shp->savesig = 0;
/* sh_fork() increases ${.sh.subshell} but we forked an existing virtual subshell, so undo */
SH_SUBSHELLNOD->nvalue.s--;
}
}
@ -454,7 +455,6 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
int savecurenv = shp->curenv;
int savejobpgid = job.curpgid;
int *saveexitval = job.exitval;
int16_t subshell;
char *savsig;
Sfio_t *iop=0;
struct checkpt buff;
@ -475,9 +475,8 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
shp->curenv = ++subenv;
savst = shp->st;
sh_pushcontext(shp,&buff,SH_JMPSUB);
subshell = shp->subshell+1;
SH_SUBSHELLNOD->nvalue.s = subshell;
shp->subshell = subshell;
shp->subshell++; /* increase level of virtual subshells */
SH_SUBSHELLNOD->nvalue.s++; /* increase ${.sh.subshell} */
sp->prev = subshell_data;
sp->shp = shp;
sp->sig = 0;
@ -757,8 +756,10 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
shp->coshell = sp->coshell;
#endif /* SHOPT_COSHELL */
if(shp->subshell)
SH_SUBSHELLNOD->nvalue.s = --shp->subshell;
subshell = shp->subshell;
{
shp->subshell--; /* decrease level of virtual subshells */
SH_SUBSHELLNOD->nvalue.s--; /* decrease ${.sh.subshell} */
}
subshell_data = sp->prev;
if(!argsav || argsav->dolrefcnt==argcnt)
sh_argfree(shp,argsav,0);

View file

@ -3195,7 +3195,8 @@ pid_t _sh_fork(Shell_t *shp,register pid_t parent,int flags,int *jobid)
/* except for those `lost' by trap */
if(!(flags&FSHOWME))
sh_sigreset(2);
shp->subshell = 0;
SH_SUBSHELLNOD->nvalue.s++; /* increase ${.sh.subshell} */
shp->subshell = 0; /* zero virtual subshells */
shp->comsub = 0;
shp->spid = 0;
if((flags&FAMP) && shp->coutpipe>1)

View file

@ -621,8 +621,25 @@ function a.set
[[ ${.sh.subshell} == 1 ]] || err_exit '${.sh.subshell} should be 1'
(
[[ ${.sh.subshell} == 2 ]] || err_exit '${.sh.subshell} should be 2'
exit $Errors
)
)
Errors=$? # ensure error count survives subshell
actual=$(
{
(
echo ${.sh.subshell} | cat # left element of pipe should increase ${.sh.subshell}
echo ${.sh.subshell}
ulimit -t unlimited # fork
echo ${.sh.subshell} # should be same after forking existing virtual subshell
)
echo ${.sh.subshell} # a background job should also increase ${.sh.subshell}
} & wait "$!"
echo ${.sh.subshell}
)
expect=$'4\n3\n3\n2\n1'
[[ $actual == "$expect" ]] || err_exit "\${.sh.subshell} failure (expected $(printf %q "$expect"), got $(printf %q "$actual"))"
set -- {1..32768}
(( $# == 32768 )) || err_exit "\$# failed -- expected 32768, got $#"
@ -657,7 +674,7 @@ set --
exit $Errors
)
let "Errors += $?" # ensure error count survives subshell
Errors=$? # ensure error count survives subshell
cd $tmp