From caf7ab6c71fdc0ed357582b03e93db41c609731e Mon Sep 17 00:00:00 2001
From: Martijn Dekker <martijn@inlv.org>
Date: Tue, 23 Feb 2021 21:35:26 +0000
Subject: [PATCH] Make PATH properly survive a shared-state ${ comsub; }

Reproducer:

$ ksh -c 'v=${ PATH=/dev/null; }; echo $PATH; whence ls'
/dev/null
/bin/ls

The PATH=/dev/null assignment should survive the shared-state
command substitution, and does, yet 'ls' is still found.
The variable became inconsistent with the internal pathlist.

This bugfix is from the 93v- beta.

src/cmd/ksh93/sh/subshell.c: sh_subshell():
- Do not save and restore pathlist for a subshare.
- A few other subshell tweaks from 93v- that made sense:
  . reset shp->subdup (bitmask for dups of 1) after saving it
  . use e_dot instead of "." for consistency
  . retry close(1) if it was interrupted

src/cmd/ksh93/tests/path.sh:
- Add test for this bug.
---
 src/cmd/ksh93/sh/subshell.c | 24 +++++++++++++++++-------
 src/cmd/ksh93/tests/path.sh |  9 +++++++++
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c
index a199e3c37..efc6ce525 100644
--- a/src/cmd/ksh93/sh/subshell.c
+++ b/src/cmd/ksh93/sh/subshell.c
@@ -549,14 +549,14 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
 	sp->options = shp->options;
 	sp->jobs = job_subsave();
 	sp->subdup = shp->subdup;
+	shp->subdup = 0;
 	/* make sure initialization has occurred */ 
 	if(!shp->pathlist)
 	{
 		shp->pathinit = 1;
-		path_get(shp,".");
+		path_get(shp,e_dot);
 		shp->pathinit = 0;
 	}
-	sp->pathlist = path_dup((Pathcomp_t*)shp->pathlist);
 #if _lib_fchdir
 	sp->pwdfd = -1;
 #endif /* _lib_fchdir */
@@ -570,6 +570,8 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
 	sp->subshare = shp->subshare;
 	sp->comsub = shp->comsub;
 	shp->subshare = comsub==2;
+	if(!shp->subshare)
+		sp->pathlist = path_dup((Pathcomp_t*)shp->pathlist);
 	if(comsub)
 	{
 		shp->comsub = comsub;
@@ -590,7 +592,7 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
 		}
 		if(sp->pwdfd<0)
 		{
-			int n = open(".",O_SEARCH);
+			int n = open(e_dot,O_SEARCH);
 			if(n>=0)
 			{
 				sp->pwdfd = n;
@@ -759,11 +761,16 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
 			}
 			sfset(iop,SF_READ,1);
 		}
-		sfswap(sp->saveout,sfstdout);
+		if(sp->saveout)
+			sfswap(sp->saveout,sfstdout);
+		else
+			sfstdout = &_Sfstdout;
 		/*  check if standard output was preserved */
 		if(sp->tmpfd>=0)
 		{
-			close(1);
+			int err=errno;
+			while(close(1)<0 && errno==EINTR)
+				errno = err;
 			if (fcntl(sp->tmpfd,F_DUPFD,1) != 1)
 			{
 				saveerrno = errno;
@@ -773,8 +780,11 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
 		}
 		shp->fdstatus[1] = sp->fdstatus;
 	}
-	path_delete((Pathcomp_t*)shp->pathlist);
-	shp->pathlist = (void*)sp->pathlist;
+	if(!shp->subshare)
+	{
+		path_delete((Pathcomp_t*)shp->pathlist);
+		shp->pathlist = (void*)sp->pathlist;
+	}
 	job_subrestore(sp->jobs);
 	shp->curenv = shp->jobenv = savecurenv;
 	job.curpgid = savejobpgid;
diff --git a/src/cmd/ksh93/tests/path.sh b/src/cmd/ksh93/tests/path.sh
index 2a455f1df..f24ad42d2 100755
--- a/src/cmd/ksh93/tests/path.sh
+++ b/src/cmd/ksh93/tests/path.sh
@@ -731,6 +731,15 @@ got=$({ FPATH=$tmp/fun.$$ "$SHELL" -c self; } 2>&1)
 (((e = $?) == 126)) || err_exit 'Function autoload recursion loop:' \
 	"got status $e$( ((e>128)) && print -n / && kill -l "$e"), $(printf %q "$got")"
 
+# ======
+# If a shared-state ${ command substitution; } changed the value of $PATH, the variable
+# would change but the internal pathlist would not, making path searches inconsistent.
+savePATH=$PATH
+got=${ PATH=/dev/null; }
+got=$(whence ls)
+PATH=$savePATH
+[[ -z $got ]] || err_exit "PATH search inconsistent after changing PATH in subshare (got $(printf %q "$got"))"
+
 # ======
 exit $((Errors<125?Errors:125))