From 7e6bbf85b635a884dc48a7c7cca8123e2a2f2257 Mon Sep 17 00:00:00 2001
From: Martijn Dekker <martijn@inlv.org>
Date: Sat, 26 Sep 2020 02:36:43 +0200
Subject: [PATCH] Fix another comsub regression (rhbz#1116508) (re: 970069a6)

Another Red Hat patch of a patch. With the new comsub mechanism,
functions could sometimes return the wrong exit status when invoked
from a command substitution.

Original patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-fununset.patch

I have determined that the extra setexit() in the Red Hat patch,
which copies the current exit status to $?, is not needed, as the
code for running functions already sets $? on termination. I've
added extra regression tests to prove this.

    By the way, the setexit() macro is defined like this in defs.h:

            #define exitset()	(sh.savexit=sh.exitval)

    That's more evidence (see also 3654ee73) that it does not
    matter whether you address the shell's status struct via a
    pointer. That macro is used in places that use shp pointers.
    But, that aside...

src/cmd/ksh93/sh/subshell.c: sh_subshell():
- When waiting within a command substitution for a forked process
  to end, save & restore sh.exitval (the exit status of the command
  currently being run) so that job_wait() cannot override it.

src/cmd/ksh93/tests/functions.sh:
- Add tests based in part on the reproducer from rhbz#1116508.
---
 src/cmd/ksh93/sh/subshell.c      |  2 ++
 src/cmd/ksh93/tests/functions.sh | 31 +++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c
index d18a7aa62250..64d1100f8f35 100644
--- a/src/cmd/ksh93/sh/subshell.c
+++ b/src/cmd/ksh93/sh/subshell.c
@@ -662,7 +662,9 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
 			job.bktick_waitall = 0;
 			if(comsub!=1 && shp->spid)
 			{
+				int e = shp->exitval;
 				job_wait(shp->spid);
+				shp->exitval = e;
 				if(shp->pipepid==shp->spid)
 					shp->spid = 0;
 				shp->pipepid = 0;
diff --git a/src/cmd/ksh93/tests/functions.sh b/src/cmd/ksh93/tests/functions.sh
index 63f6450d4b50..535985f1576a 100755
--- a/src/cmd/ksh93/tests/functions.sh
+++ b/src/cmd/ksh93/tests/functions.sh
@@ -1275,5 +1275,36 @@ $SHELL <<- \EOF
 EOF
 [[ $? == 2 ]] && err_exit 'Over-shifting in a POSIX function does not terminate the script if the function call has a redirection'
 
+# ======
+# https://bugzilla.redhat.com/1116508
+expect=$'13/37/1/1'
+actual=$(
+	foo() {
+		print foo | read
+		return 13
+	}
+	out=$(foo)
+	print -n $?/
+	foo() {
+		print foo | read
+		ls /dev/null
+		return 37
+	}
+	out=$(foo)
+	print -n $?/
+	foo() {
+		print foo | ! read
+	}
+	out=$(foo)
+	print -n $?/
+	foo() {
+		! true
+	}
+	out=$(foo)
+	print -n $?
+)
+[[ $actual == "$expect" ]] || err_exit "wrong exit status from function invoked by command substitution" \
+	"(expected $(printf %q "$expect"), got $(printf %q "$actual"))"
+
 # ======
 exit $((Errors<125?Errors:125))