tests: add extra protection against unexpected exits

Many tests use `program ... && fail=1` to ensure expected
error situations are indicated.  However that would mask
an unexpected exit (like a crash).  Therefore explicitly
check the expected exit code.
Note where error messages are also verified, the extra
protection is not added.

* tests/init.sh (returns_): A new helper function to
check the return code of a command, and used
throughout the tests.
* cfg.mk (sc_prohibit_and_fail_1): Add a syntax check
to avoid new instances of this issue.

9 files changed, 18 insertions, 15 deletions

diff --git a/tests/cp/cp-HL.sh b/tests/cp/cp-HL.sh
index fa9832537..08a72b13a 100755
--- a/tests/cp/cp-HL.sh
+++ b/tests/cp/cp-HL.sh
@@ -33,7 +33,7 @@ test -d dest-dir/src-dir || fail=1
 cat dest-dir/slink > /dev/null 2>&1 || fail=1
 
 # Expect this to fail since *this* slink is a dangling symlink.
-cat dest-dir/src-dir/slink > /dev/null 2>&1 && fail=1
+returns_ 1 cat dest-dir/src-dir/slink >/dev/null 2>&1 || fail=1
 
 # FIXME: test -L, too.
 
diff --git a/tests/cp/cp-a-selinux.sh b/tests/cp/cp-a-selinux.sh
index 2ecbd2386..d000ee8f1 100755
--- a/tests/cp/cp-a-selinux.sh
+++ b/tests/cp/cp-a-selinux.sh
@@ -49,14 +49,16 @@ rm -f f
 # in the destination, so SELinux contexts should be updated too.
 chmod o+rw restore/existing_dir
 mkdir -p backup/existing_dir/ || framework_failure_
-ls -Zd backup/existing_dir | grep $ctx && framework_failure_
+ls -Zd backup/existing_dir > ed_ctx || fail=1
+grep $ctx ed_ctx && framework_failure_
 touch backup/existing_dir/file || framework_failure_
 chcon $ctx backup/existing_dir/file || framework_failure_
 # Set the dir context to ensure it is reset
 mkdir -p --context="$ctx" restore/existing_dir || framework_failure_
 # Copy and ensure existing directories updated
 cp -a backup/. restore/
-ls -Zd restore/existing_dir | grep $ctx &&
+ls -Zd restore/existing_dir > ed_ctx || fail=1
+grep $ctx ed_ctx &&
   { ls -lZd restore/existing_dir; fail=1; }
 
 # Check restorecon (-Z) functionality for file and directory
@@ -175,6 +177,7 @@ for no_g_cmd in '' 'rm -f g'; do
   # restorecon equivalent.  Note even though the context
   # returned from matchpathcon() will not match $ctx
   # the resulting ENOTSUP warning will be suppressed.
+
    # With absolute path
   $no_g_cmd
   cp -Z ../f $(realpath g) || fail=1
@@ -186,7 +189,7 @@ for no_g_cmd in '' 'rm -f g'; do
   cp -Z -a ../f g || fail=1
    # -Z doesn't take an arg
   $no_g_cmd
-  cp -Z "$ctx" ../f g && fail=1
+  returns_ 1 cp -Z "$ctx" ../f g || fail=1
 
   # Explicit context
   $no_g_cmd
@@ -198,8 +201,8 @@ for no_g_cmd in '' 'rm -f g'; do
 done
 
 # Mutually exclusive options
-cp -Z --preserve=context ../f g && fail=1
-cp --preserve=context -Z ../f g && fail=1
-cp --preserve=context --context="$ctx" ../f g && fail=1
+returns_ 1 cp -Z --preserve=context ../f g || fail=1
+returns_ 1 cp --preserve=context -Z ../f g || fail=1
+returns_ 1 cp --preserve=context --context="$ctx" ../f g || fail=1
 
 Exit $fail
diff --git a/tests/cp/cp-i.sh b/tests/cp/cp-i.sh
index d9d805116..e74141307 100755
--- a/tests/cp/cp-i.sh
+++ b/tests/cp/cp-i.sh
@@ -60,6 +60,6 @@ echo n | cp -vnf c d 2>/dev/null > out7 || fail=1
 compare out7 out_empty || fail=1
 
 # options --backup and --no-clobber are mutually exclusive
-cp -bn c d 2>/dev/null && fail=1
+returns_ 1 cp -bn c d 2>/dev/null || fail=1
 
 Exit $fail
diff --git a/tests/cp/cp-parents.sh b/tests/cp/cp-parents.sh
index 4814d6557..f0454f714 100755
--- a/tests/cp/cp-parents.sh
+++ b/tests/cp/cp-parents.sh
@@ -42,7 +42,7 @@ test -d d/a/b/c || fail=1
 
 # With 6.7 and earlier, cp --parents f/g d would mistakenly create a
 # directory d/f, even though f is a regular file.
-cp --parents f/g d 2>/dev/null && fail=1
+returns_ 1 cp --parents f/g d 2>/dev/null || fail=1
 test -d d/f && fail=1
 
 # Check that re_protect works.
diff --git a/tests/cp/dir-vs-file.sh b/tests/cp/dir-vs-file.sh
index 826d9bce0..8ac159c04 100755
--- a/tests/cp/dir-vs-file.sh
+++ b/tests/cp/dir-vs-file.sh
@@ -24,7 +24,7 @@ touch file || framework_failure_
 
 
 # In 4.0.35, this cp invocation silently succeeded.
-cp -R dir file 2>/dev/null && fail=1
+returns_ 1 cp -R dir file 2>/dev/null || fail=1
 
 # Make sure file is not replaced with a directory.
 # In 4.0.35, it was.
diff --git a/tests/cp/fail-perm.sh b/tests/cp/fail-perm.sh
index abc5e4ebe..6a9cc8804 100755
--- a/tests/cp/fail-perm.sh
+++ b/tests/cp/fail-perm.sh
@@ -27,7 +27,7 @@ chmod u=rx,go=,-st D || framework_failure_
 
 
 # This is expected to exit non-zero, because it can't read D/a.
-cp -pR D DD > /dev/null 2>&1 && fail=1
+returns_ 1 cp -pR D DD > /dev/null 2>&1 || fail=1
 
 # Permissions on DD must be 'dr-x------'
 
diff --git a/tests/cp/no-ctx.sh b/tests/cp/no-ctx.sh
index 3cae58314..19d68b8ff 100755
--- a/tests/cp/no-ctx.sh
+++ b/tests/cp/no-ctx.sh
@@ -57,7 +57,7 @@ LD_PRELOAD=./k.so cp -a file_src file_dst || fail=1
 
 # ENODATA should give an immediate error when required to preserve ctx
 # This is debatable, and maybe we should not fail when no context available?
-LD_PRELOAD=./k.so cp --preserve=context file_src file_dst && fail=1
+LD_PRELOAD=./k.so returns_ 1 cp --preserve=context file_src file_dst || fail=1
 
 test -e preloaded || skip_ 'LD_PRELOAD interception failed'
 
diff --git a/tests/cp/reflink-auto.sh b/tests/cp/reflink-auto.sh
index 666620879..b0464a2eb 100755
--- a/tests/cp/reflink-auto.sh
+++ b/tests/cp/reflink-auto.sh
@@ -27,7 +27,7 @@ rm -f "$a_other" || framework_failure_
 echo non_zero_size > "$a_other" || framework_failure_
 
 # we shouldn't be able to reflink() files on separate partitions
-cp --reflink      "$a_other" b && fail=1
+returns_ 1 cp --reflink "$a_other" b || fail=1
 
 # --reflink=auto should fall back to a normal copy
 cp --reflink=auto "$a_other" b || fail=1
diff --git a/tests/cp/sparse.sh b/tests/cp/sparse.sh
index 3d6fb5876..958de8306 100755
--- a/tests/cp/sparse.sh
+++ b/tests/cp/sparse.sh
@@ -34,8 +34,8 @@ cp --sparse=always sparse copy || fail=1
 test $(stat --printf %b copy) -le $(stat --printf %b sparse) || fail=1
 
 # Ensure that --sparse={always,never} with --reflink fail.
-cp --sparse=always --reflink sparse copy && fail=1
-cp --sparse=never --reflink sparse copy && fail=1
+returns_ 1 cp --sparse=always --reflink sparse copy || fail=1
+returns_ 1 cp --sparse=never --reflink sparse copy || fail=1
 
 
 # Ensure we handle sparse/non-sparse transitions correctly