mirror of
git://git.code.sf.net/p/cdesktopenv/code
synced 2025-03-09 15:50:02 +00:00
This commit fixes what are hopefully the two final aspects of #153: 1. If the present working directory does not exist (was moved or deleted) upon entering a virtual subshell, no PWD directory path is saved. Since restoring the state after exiting a virtual subshell is contingent on a previous PWD path existing, this resulted in entire aspects of the virtual subshell, such as the subshell function tree, not being cleaned up. 2. A separate problem is that 'cd ..' does not update PWD or OLDPWD when run from a nonexistent directory. A reproducer exposing both problems is: $ mkdir test $ cd test $ ksh -c '(subfn() { BAD; }; cd ..; echo subPWD==$PWD); typeset -f subfn; echo mainPWD==$PWD' subPWD==/usr/local/src/ksh93/ksh/test subfn() { BAD; };mainPWD==/usr/local/src/ksh93/ksh/test Expected output: subPWD==/usr/local/src/ksh93/ksh mainPWD==/usr/local/src/ksh93/ksh/test src/cmd/ksh93/bltins/cd_pwd.c: - If path_pwd() fails to get the PWD (usually it no longer exists), don't set $OLDPWD to '.' as that is pointless; use $PWD instead. After cd'ing from a nonexistent directory, 'cd -' *should* fail and should not be equivalent to 'cd .'. - Remove a redundant check for (!oldpwd) where it is always set. - Do not prematurely return without setting PWD or OLDPWD if pathcanon() fails to canonicalise a nonexistent directory. Instead, fall back to setting PWD to the result of getcwd(3). src/cmd/ksh93/sh/subshell.c: - Minor stylistic adjustment. Some NULL macros sneaked in. This historic code base does not use them (yet); change to NIL(type*). - sh_subshell(): Fix logic for determining whether to save/restore subshell state. 1. When saving, 'if(!comsub || !shp->subshare)' is redundant; 'if(!shp->subshare)' should be enough. If we're not in a subshare, state should be saved. 2. When restoring, 'if(sp->shpwd)' is just nonsense as there is no guarantee that the PWD exists upon entering a subshell. Simply use the same 'if(!shp->subshare)'. Add an extra check for sp->pwd to avoid a possible segfault. Always restore the PWD on subshell exit and not only if shp->pwd is set. - sh_subshell(): Issue fatal errors in libast's "panic" format. src/cmd/ksh93/tests/builtins.sh: - Adjust a relevant test to run err_exit() outside of the subshell so that any error is counted in the main shell. - Add test for problem 2 described at the top. src/cmd/ksh93/tests/subshell.sh: - Add test for problems 1 and 2 based on reproducer above. Resolves: https://github.com/ksh93/ksh/issues/153
This commit is contained in:
parent
b0a6c1bde5
commit
feaf718f16
4 changed files with 67 additions and 38 deletions
|
|
@ -219,7 +219,7 @@ void sh_subfork(void)
|
|||
shp->subshell = 0;
|
||||
shp->comsub = 0;
|
||||
sp->subpid=0;
|
||||
shp->st.trapcom[0] = (comsub==2 ? NULL : trap);
|
||||
shp->st.trapcom[0] = (comsub==2 ? NIL(char*) : trap);
|
||||
shp->savesig = 0;
|
||||
/* sh_fork() increases ${.sh.subshell} but we forked an existing virtual subshell, so undo */
|
||||
shgd->realsubshell--;
|
||||
|
|
@ -581,7 +581,7 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
|
|||
shp->comsub = comsub;
|
||||
job.bktick_waitall = (comsub==1);
|
||||
}
|
||||
if(!comsub || !shp->subshare)
|
||||
if(!shp->subshare)
|
||||
{
|
||||
struct subshell *xp;
|
||||
sp->shpwd = shp->pwd;
|
||||
|
|
@ -634,9 +634,9 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
|
|||
else if(shp->st.trapcom[isig])
|
||||
savsig[isig] = sh_strdup(shp->st.trapcom[isig]);
|
||||
else
|
||||
savsig[isig] = NULL;
|
||||
savsig[isig] = NIL(char*);
|
||||
}
|
||||
/* this nonsense needed for $(trap) */
|
||||
/* this is needed for var=$(trap) */
|
||||
shp->st.otrapcom = (char**)savsig;
|
||||
}
|
||||
sp->cpid = shp->cpid;
|
||||
|
|
@ -794,7 +794,7 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
|
|||
job.curpgid = savejobpgid;
|
||||
job.exitval = saveexitval;
|
||||
shp->bckpid = sp->bckpid;
|
||||
if(sp->shpwd) /* restore environment if saved */
|
||||
if(!shp->subshare) /* restore environment if saved */
|
||||
{
|
||||
int n;
|
||||
shp->options = sp->options;
|
||||
|
|
@ -842,24 +842,21 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
|
|||
free((void*)savsig);
|
||||
}
|
||||
shp->options = sp->options;
|
||||
if(!shp->pwd || strcmp(sp->pwd,shp->pwd))
|
||||
if(shp->pwd != sp->pwd && (!shp->pwd || !sp->pwd || strcmp(sp->pwd,shp->pwd)))
|
||||
{
|
||||
/* restore PWDNOD */
|
||||
/* restore the present working directory */
|
||||
Namval_t *pwdnod = sh_scoped(shp,PWDNOD);
|
||||
if(shp->pwd)
|
||||
{
|
||||
#if _lib_fchdir
|
||||
if(fchdir(sp->pwdfd) < 0)
|
||||
if(fchdir(sp->pwdfd) < 0)
|
||||
#else
|
||||
if(chdir(sp->pwd) < 0)
|
||||
if(!sp->pwd || chdir(sp->pwd) < 0)
|
||||
#endif /* _lib_fchdir */
|
||||
{
|
||||
saveerrno = errno;
|
||||
fatalerror = 2;
|
||||
}
|
||||
shp->pwd=sp->pwd;
|
||||
path_newdir(shp,shp->pathlist);
|
||||
{
|
||||
saveerrno = errno;
|
||||
fatalerror = 2;
|
||||
}
|
||||
shp->pwd=sp->pwd;
|
||||
path_newdir(shp,shp->pathlist);
|
||||
if(nv_isattr(pwdnod,NV_NOFREE))
|
||||
pwdnod->nvalue.cp = (const char*)sp->pwd;
|
||||
}
|
||||
|
|
@ -939,17 +936,17 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
|
|||
case 1:
|
||||
shp->toomany = 1;
|
||||
errno = saveerrno;
|
||||
errormsg(SH_DICT,ERROR_system(1),e_redirect);
|
||||
errormsg(SH_DICT,ERROR_SYSTEM|ERROR_PANIC,e_redirect);
|
||||
UNREACHABLE();
|
||||
case 2:
|
||||
/* reinit PWD as it will be wrong */
|
||||
shp->pwd = NULL;
|
||||
shp->pwd = NIL(const char*);
|
||||
path_pwd(shp,0);
|
||||
errno = saveerrno;
|
||||
errormsg(SH_DICT,ERROR_system(1),"Failed to restore PWD upon exiting subshell");
|
||||
errormsg(SH_DICT,ERROR_SYSTEM|ERROR_PANIC,"Failed to restore PWD upon exiting subshell");
|
||||
UNREACHABLE();
|
||||
default:
|
||||
errormsg(SH_DICT,ERROR_system(1),"Subshell error %d",fatalerror);
|
||||
errormsg(SH_DICT,ERROR_SYSTEM|ERROR_PANIC,"Subshell error %d",fatalerror);
|
||||
UNREACHABLE();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue