From 055b08ff7853c7c9ab791643047b1b163a227013 Mon Sep 17 00:00:00 2001
From: =?utf8?q?P=C3=A1draig=20Brady?= <P@draigBrady.com>
Date: Tue, 22 Nov 2016 01:23:22 +0000
Subject: [PATCH] tests: fix false fails due to passing env vars to returns_

On BSD /bin/sh it was seen that unexported env vars passed to
returns_() would not be propagated to the wrapped command.

* cfg.mk (sc_prohibit_env_returns): Add a syntax check to disallow.
* tests/misc/csplit-io-err.sh: Rearrange to export vars in a subshell.
* tests/rm/rm-readdir-fail.sh: Likewise.
* tests/misc/nohup.sh: Export and unset vars around returns_.
* tests/misc/printenv.sh: Likewise.
Reported by Assaf Gordon
---
 cfg.mk                      | 8 ++++++++
 tests/misc/csplit-io-err.sh | 6 +++---
 tests/misc/nohup.sh         | 4 +++-
 tests/misc/printenv.sh      | 6 ++++--
 tests/rm/rm-readdir-fail.sh | 3 ++-
 5 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index 5d42bf14ef..9e4f8a31a7 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -510,6 +510,14 @@ sc_prohibit_and_fail_1:
 	in_vc_files='^tests/'						\
 	  $(_sc_search_regexp)
 
+# Ensure that env vars are not passed through returns_ as
+# that was seen to fail on FreeBSD /bin/sh at least
+sc_prohibit_env_returns:
+	@prohibit='=[^ ]* returns_ '					\
+	halt='Passing env vars to returns_ is non portable'		\
+	in_vc_files='^tests/'						\
+	  $(_sc_search_regexp)
+
 # The mode part of a setfacl -m option argument must be three bytes long.
 # I.e., an argument of user:bin:rw or user:bin:r will make Solaris 10's
 # setfacl reject it with: "Unrecognized character found in mode field".
diff --git a/tests/misc/csplit-io-err.sh b/tests/misc/csplit-io-err.sh
index 982df161f8..900e38bee7 100755
--- a/tests/misc/csplit-io-err.sh
+++ b/tests/misc/csplit-io-err.sh
@@ -66,9 +66,9 @@ gcc_shared_ k.c k.so \
 # Split the input, and force fwrite() failure -
 # the 'csplit' command should fail with exit code 1
 # (checked with 'returns_ 1 ... || fail=1')
-seq 10 \
-  | LD_PRELOAD=$LD_PRELOAD:./k.so returns_ 1 csplit - 1 4 2>out \
-  || fail=1
+seq 10 |
+(export LD_PRELOAD=$LD_PRELOAD:./k.so
+ returns_ 1 csplit - 1 4 2>out) || fail=1
 
 test -e preloaded || skip_ 'LD_PRELOAD interception failed'
 
diff --git a/tests/misc/nohup.sh b/tests/misc/nohup.sh
index e9c8c389ae..1b43b606f0 100755
--- a/tests/misc/nohup.sh
+++ b/tests/misc/nohup.sh
@@ -118,6 +118,8 @@ EOF
 # Make sure it fails with exit status of 125 when given too few arguments,
 # except that POSIX requires 127 in this case.
 returns_ 125 nohup >/dev/null 2>&1 || fail=1
-POSIXLY_CORRECT=1 returns_ 127 nohup >/dev/null 2>&1 || fail=1
+export POSIXLY_CORRECT=1
+returns_ 127 nohup >/dev/null 2>&1 || fail=1
+unset POSIXLY_CORRECT
 
 Exit $fail
diff --git a/tests/misc/printenv.sh b/tests/misc/printenv.sh
index 2017e5df66..b33c0292c0 100755
--- a/tests/misc/printenv.sh
+++ b/tests/misc/printenv.sh
@@ -59,8 +59,10 @@ EOF
 compare exp out || fail=1
 
 # Exit status reflects missing variable, but remaining arguments processed.
-ENV_TEST1=a returns_ 1 env -- printenv ENV_TEST2 ENV_TEST1 > out || fail=1
-ENV_TEST1=a returns_ 1 env -- printenv ENV_TEST1 ENV_TEST2 >> out || fail=1
+export ENV_TEST1=a
+returns_ 1 env -- printenv ENV_TEST2 ENV_TEST1 > out || fail=1
+returns_ 1 env -- printenv ENV_TEST1 ENV_TEST2 >> out || fail=1
+unset ENV_TEST1
 cat <<EOF > exp || framework_failure_
 a
 a
diff --git a/tests/rm/rm-readdir-fail.sh b/tests/rm/rm-readdir-fail.sh
index d9d8efdb4d..835d3424f8 100755
--- a/tests/rm/rm-readdir-fail.sh
+++ b/tests/rm/rm-readdir-fail.sh
@@ -88,7 +88,8 @@ gcc_shared_ k.c k.so \
 export READDIR_PARTIAL
 for READDIR_PARTIAL in '' '1'; do
   rm -f preloaded
-  (LD_PRELOAD=$LD_PRELOAD:./k.so returns_ 1 rm -Rf dir 2>>err) || fail=1
+  (export LD_PRELOAD=$LD_PRELOAD:./k.so
+   returns_ 1 rm -Rf dir 2>>err) || fail=1
   test -f preloaded ||
     skip_ "internal test failure: maybe LD_PRELOAD doesn't work?"
 done
-- 
2.47.2