Avoid unspecified behaviour around simple commands in general

As mentioned by the previous commit, the Shell Command Language leaves
it unspecified as to whether variable assignments affecting the
execution environment of a simple command charged with executing a
function (that is not the implementation of a standard utility) shall
persist after the completion of the function.

It transpires that modifying gentoo-functions so as to steer clear of
this pitfall isn't particularly difficult so this commit does exactly
that. Most of the changes are in test-functions but functions/rc.sh also
required some minor changes regarding the use of the GENFUN_CALLER
variable.

With this, loksh very nearly passes the test suite. There is one
individual test that continues to fail, although it looks as though that
may be caused by a genuine bug on the part of the shell. That will
require investigating in its own right.

Signed-off-by: Kerin Millar <kfm@plushkava.net>
Signed-off-by: Sam James <sam@gentoo.org>

2 files changed, 35 insertions, 24 deletions

diff --git a/functions/rc.sh b/functions/rc.sh
index 12444d1..0c14035 100644
--- a/functions/rc.sh
+++ b/functions/rc.sh
@@ -49,7 +49,8 @@ ebegin()
 #
 eend()
 {
-	GENFUN_CALLER=${GENFUN_CALLER:-eend} _eend eerror "$@"
+	: "${genfun_caller:=eend}"
+	_eend eerror "$@"
 }
 
 #
@@ -161,7 +162,8 @@ ewarnn()
 #
 ewend()
 {
-	GENFUN_CALLER=${GENFUN_CALLER:-ewend} _eend ewarn "$@"
+	: "${genfun_caller:=ewend}"
+	_eend ewarn "$@"
 }
 
 #
@@ -240,7 +242,8 @@ done
 veend()
 {
 	if yesno "${EINFO_VERBOSE}"; then
-		GENFUN_CALLER=veend eend "$@"
+		genfun_caller=veend
+		eend "$@"
 	elif [ "$#" -gt 0 ] && { ! is_int "$1" || [ "$1" -lt 0 ]; }; then
 		_warn_for_args veend "$1"
 		false
@@ -252,7 +255,8 @@ veend()
 vewend()
 {
 	if yesno "${EINFO_VERBOSE}"; then
-		GENFUN_CALLER=vewend ewend "$@"
+		genfun_caller=vewend
+		ewend "$@"
 	elif [ "$#" -gt 0 ] && { ! is_int "$1" || [ "$1" -lt 0 ]; }; then
 		_warn_for_args vewend "$1"
 		false
@@ -311,7 +315,7 @@ _eend()
 	if [ "$#" -eq 0 ]; then
 		retval=0
 	elif ! is_int "$1" || [ "$1" -lt 0 ]; then
-		_warn_for_args "${GENFUN_CALLER}" "$1"
+		_warn_for_args "${genfun_caller}" "$1"
 		retval=1
 		msg=
 	else
@@ -320,6 +324,8 @@ _eend()
 		msg=$*
 	fi
 
+	genfun_caller=
+
 	if [ "${retval}" -ne 0 ]; then
 		# If a message was given, print it with the specified function.
 		if _is_visible "${msg}"; then
diff --git a/test-functions b/test-functions
index afc56eb..0b0e127 100755
--- a/test-functions
+++ b/test-functions
@@ -46,19 +46,6 @@ test_local() {
 	return "${retval}"
 }
 
-test_simple_command() {
-	f() { :; }
-	LEAKED=
-	LEAKED=1 f
-	retval=0
-	if [ "${LEAKED}" ]; then
-		printf 'not '
-		retval=1
-	fi
-	printf "ok %d - /bin/sh refrains from leaking environmental changes for simple commands\\n" "$((testnum += 1))"
-	return "${retval}"
-}
-
 test_chdir() {
 	set -- \
 		ge  1          ''  \
@@ -76,13 +63,17 @@ test_chdir() {
 	fi
 
 	callback() {
+		local CDPATH var
+
 		shift
 		test_description="chdir $(quote_args "$@")"
 		if [ "$BASH" ]; then
 			# shellcheck disable=3044
 			shopt -s cdable_vars
 		fi
-		CDPATH=child var=child chdir "$@" \
+		CDPATH=child
+		var=child
+		chdir "$@" \
 		&& test "$PWD" != "$OLDPWD" \
 		&& cd - >/dev/null
 	}
@@ -266,7 +257,7 @@ test_esyslog() {
 		should_log=$2
 		shift 2
 		test_description="esyslog $(quote_args "$@")"
-		logged=$(EINFO_LOG=1 esyslog "$@")
+		logged=$(EINFO_LOG=1; esyslog "$@")
 		case $? in
 			0)
 				test "${logged:-0}" -eq "${should_log}"
@@ -680,10 +671,22 @@ test_whenceforth() {
 			whenceforth "$@" >/dev/null
 		else
 			test_description="PATH=${path} whenceforth $(quote_args "$@")"
-			PATH=${path} whenceforth "$@" >/dev/null
+			(
+				# If necessary, conduct the test with a printf
+				# function in effect, duly covering shells that
+				# do not implement it as a builtin. Otherwise,
+				# it could become unavailable on account of the
+				# various values of PATH being tested.
+				case ${printf_cmd} in
+					/*) printf() { "${printf_cmd}" "$@"; }
+				esac
+				PATH=${path}
+				whenceforth "$@" >/dev/null
+			)
 		fi
 	}
 
+	printf_cmd=$(command -v printf)
 	iterate_tests 5 "$@"
 }
 
@@ -923,6 +926,8 @@ test_contains_any() {
 }
 
 test_quote_args() {
+	local POSIXLY_CORRECT
+
 	testnum=$((testnum + 1))
 	retval=0
 	i=0
@@ -930,7 +935,7 @@ test_quote_args() {
 		fmt=$(printf '\\%o' "$i")
 		# shellcheck disable=2059
 		str=$(printf "$fmt.")
-		POSIXLY_CORRECT= quote_args "${str%.}" || break
+		quote_args "${str%.}" || break
 	done | cksum | {
 		read -r cksum _
 		if [ "${cksum}" != "380900690" ]; then
@@ -1016,7 +1021,7 @@ test_update_time() {
 		shift
 		if [ "$1" ]; then
 			test_description="LC_ALL=$1 _update_time"
-			LC_ALL=$1 genfun_time=$(_update_time && printf %s "${genfun_time}")
+			genfun_time=$(LC_ALL=$1; _update_time && printf %s "${genfun_time}")
 		else
 			test_description="_update_time"
 			genfun_time=$(_update_time && printf %s "${genfun_time}")
@@ -1084,7 +1089,7 @@ if [ "${PORTAGE_BIN_PATH}" ] && [ "${S}" ]; then
 	genfun_basedir=${S}
 fi
 
-if ! test_local || ! test_simple_command; then
+if ! test_local; then
 	rc=1
 elif ! GENFUN_MODULES="portage rc" . ./functions.sh; then
 	bailout "Couldn't source ./functions.sh"