From 090b65e79bd9e54edd2a6bfc67e3706850a5b5f9 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Mon, 26 Apr 2021 18:09:36 +0100 Subject: [PATCH] Fix fork after redirecting stdout in subshare (re: 500757d7) Previously, command substitutions executed as virtual subshells were always forked if any command was run within them that redireceted standard output, even if the redirection was local to that command. Commit 500757d7 removed the check for a shared-state command substitution (subshare), so introduced a bug where even that would fork, causing it to stop sharing its state. We can further improve on that fix by only forking if the redirection is permanent as with `exec` or `redirect`. There should be no need to do that if the redirection is local to a command run within the command substitution, as the file descriptor is restored when that command finishes, which is still within the command substitution. src/cmd/ksh93/sh/io.c: sh_redirect(): - Only fork upon redirecting stdout if the virtual subshell is a command substitution, and if the redirection is permanent (flag==1 or flag==2). --- NEWS | 6 ++++++ src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh/io.c | 12 +++++++----- src/cmd/ksh93/tests/subshell.sh | 8 ++++++++ 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index 4893f0953..e7bb8927b 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,12 @@ For full details, see the git log at: https://github.com/ksh93/ksh Any uppercase BUG_* names are modernish shell bug IDs. +2021-04-26: + +- Fixed a bug introduced on 2021-02-20 in which a shared-state command + substitution stopped sharing its state with the calling shell environment + if it executed a command that locally redirected standard outpuut. + 2021-04-22: - shcomp (the shell bytecode compiler) was fixed to correctly compile process diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index 979d8bb6f..3de355fd9 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -21,7 +21,7 @@ #define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */ #define SH_RELEASE_SVER "1.0.0-alpha" /* semantic version number: https://semver.org */ -#define SH_RELEASE_DATE "2021-04-22" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2021-04-26" /* must be in this format for $((.sh.version)) */ #define SH_RELEASE_CPYR "(c) 2020-2021 Contributors to ksh " SH_RELEASE_FORK /* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */ diff --git a/src/cmd/ksh93/sh/io.c b/src/cmd/ksh93/sh/io.c index 303655be9..30b687d73 100644 --- a/src/cmd/ksh93/sh/io.c +++ b/src/cmd/ksh93/sh/io.c @@ -1147,13 +1147,15 @@ int sh_redirect(Shell_t *shp,struct ionod *iop, int flag) fn = (iof&IOUFD); if(fn==1) { - if(shp->subshare && flag==2) + if(shp->subshell && shp->comsub && (flag==1 || flag==2)) { - errormsg(SH_DICT,ERROR_exit(1),"cannot redirect stdout inside shared-state comsub"); - UNREACHABLE(); - } - if(shp->subshell && (flag==2 || isstring)) + if(shp->subshare) + { + errormsg(SH_DICT,ERROR_exit(1),"cannot redirect stdout inside shared-state comsub"); + UNREACHABLE(); + } sh_subfork(); + } } if(shp->redir0 && fn==0 && !(iof&IOMOV)) shp->redir0 = 2; diff --git a/src/cmd/ksh93/tests/subshell.sh b/src/cmd/ksh93/tests/subshell.sh index 19b9e61d9..e5eeee171 100755 --- a/src/cmd/ksh93/tests/subshell.sh +++ b/src/cmd/ksh93/tests/subshell.sh @@ -1010,5 +1010,13 @@ unalias a [[ $got == "$exp" ]] || err_exit 'backtick comsub with alias:' \ "expected $(printf %q "$exp"), got $(printf %q "$got")" +# ====== +# Redirecting standard output for a single command should not cause a subshare to fork +exp='good' +got='bad' +dummy=${ : >&2; got='good'; } +[[ $got == "$exp" ]] || err_exit 'subshare stopped sharing state after command that redirects stdout' \ + "(expected '$exp', got '$got')" + # ====== exit $((Errors<125?Errors:125))