tests: avoid the :> construct which can hide errors

On most shells `:>file || framework_failure_` will not evaluate the framework_failure_ even if there was an error writing the file. shells which do evaluate the failure are ksh 93u+ and bash 4.2, while shells wich don't include bash 4.3, solaris, freebsd, dash. Furthermore this construct is problematic on Solaris 10 sh, which will try to optimize away a `:' command in a loop after the first iteration, even if it is redirected. * tests/cp/link-deref.sh: Remove the leading colon on redirections. * tests/cp/reflink-perm.sh: Likewise. * tests/id/zero.sh: Likewise. * tests/install/install-C.sh: Likewise. * tests/misc/env.sh: Likewise. * tests/misc/md5sum-bsd.sh: Likewise. * tests/misc/runcon-no-reorder.sh: Likewise. * tests/mv/partition-perm.sh: Likewise. * tests/rm/r-root.sh: Likewise. * tests/split/l-chunk.sh: Likewise. * tests/split/line-bytes.sh: Likewise. * tests/tail-2/inotify-rotate.sh: Likewise. * tests/tail-2/retry.sh: Likewise. * tests/tail-2/symlink.sh: Likewise. * tests/tail-2/wait.sh: Likewise. * tests/touch/read-only.sh: Likewise. + cfg.mk (sc_prohibit_colon_redirection): A new syntax check to avoid further instances of this creeping in.
2025-08-10 05:43:02 +03:00 · 2014-03-05 18:41:16 +00:00
parent 5dce6bdfaf
commit 056d6de78f
17 changed files with 33 additions and 26 deletions
--- a/cfg.mk
+++ b/cfg.mk
@@ -123,6 +123,13 @@ sc_tests_executable:
 	  | sed -e "s/^/$(ME): Please make test executable: /" | grep . \
 	    && exit 1; :

+# Avoid :>file which doesn't propagate errors
+sc_prohibit_colon_redirection:
+	@cd $(srcdir)/tests && GIT_PAGER= git grep -n ': *>.*||' \
+	  && { echo '$(ME): '"The leading colon in :> will hide errors" 1>&2; \
+	       exit 1; }  \
+	  || :
+
 # Create a list of regular expressions matching the names
 # of files included from system.h.  Exclude a couple.
 .re-list:
--- a/tests/cp/link-deref.sh
+++ b/tests/cp/link-deref.sh
@@ -31,7 +31,7 @@ if grep '^#define HAVE_LINKAT 1' "$CONFIG_HEADER" > /dev/null \
 fi

 mkdir dir              || framework_failure_
-: > file               || framework_failure_
+> file                 || framework_failure_
 ln -s dir     dirlink  || framework_failure_
 ln -s file    filelink || framework_failure_
 ln -s nowhere danglink || framework_failure_
--- a/tests/cp/reflink-perm.sh
+++ b/tests/cp/reflink-perm.sh
@@ -20,8 +20,8 @@
 print_ver_ cp


-: > time_check
-: > file
+> time_check
+> file
 ts='2009-08-28 19:00'
 touch -d "$ts" file || framework_failure_
 test time_check -nt file || skip_ "The system clock is wrong"
--- a/tests/id/zero.sh
+++ b/tests/id/zero.sh
@@ -42,7 +42,7 @@ printf '%s\n' $users '' >> users || framework_failure_

 # Exercise "id -z" with various options.
 printf '\n' > exp || framework_failure_
-:> out || framework_failure_
+> out || framework_failure_

 while read u ; do
  for o in g gr G Gr u ur ; do
--- a/tests/install/install-C.sh
+++ b/tests/install/install-C.sh
@@ -54,7 +54,7 @@ echo test > a || framework_failure_
 echo "'a' -> 'b'" > out_installed_first || framework_failure_
 echo "removed 'b'
 'a' -> 'b'" > out_installed_second || framework_failure_
-: > out_empty || framework_failure_
+> out_empty || framework_failure_

 # destination file does not exist
 ginstall -Cv -m$mode1 a b > out || fail=1
--- a/tests/misc/env.sh
+++ b/tests/misc/env.sh
@@ -62,7 +62,7 @@ fi

 ENV_TEST1=a
 export ENV_TEST1
-: >out || framework_failure_
+>out || framework_failure_
 env ENV_TEST2= > all || fail=1
 grep '^ENV_TEST' all | LC_ALL=C sort >> out || framework_failure_
 env -u ENV_TEST1 ENV_TEST3=c > all || fail=1
--- a/tests/misc/md5sum-bsd.sh
+++ b/tests/misc/md5sum-bsd.sh
@@ -70,7 +70,7 @@ nl='
 tab='	'
 rm check.md5
 for i in 'a\b' 'a\' "a${nl}b" "a${tab}b"; do
-  :> "$i"
+  > "$i"
  md5sum --tag "$i" >> check.md5
 done
 md5sum --strict -c check.md5 || fail=1
--- a/tests/misc/runcon-no-reorder.sh
+++ b/tests/misc/runcon-no-reorder.sh
@@ -27,7 +27,7 @@ echo "$diag" > exp || framework_failure_
 # On such a system it fails with the above diagnostic, which is fine.
 # Before the no-reorder change, it would have failed with a diagnostic
 # about -j being an invalid option.
-runcon $(id -Z) true -j 2> out && : > exp
+runcon $(id -Z) true -j 2> out && > exp

 # When run on a system with no /selinux/context (i.e., in a chroot),
 # it chcon fails with this: "runcon: invalid context: \
--- a/tests/mv/partition-perm.sh
+++ b/tests/mv/partition-perm.sh
@@ -21,11 +21,11 @@ print_ver_ mv
 cleanup_() { rm -rf "$other_partition_tmpdir"; }
 . "$abs_srcdir/tests/other-fs-tmpdir"

-: > file
-chmod a=rwx file
+> file || framework_failure_
+chmod a=rwx file || framework_failure_

 umask 077
-mv file "$other_partition_tmpdir"
+mv file "$other_partition_tmpdir" || framework_failure_

 test -f file && fail=1
 test -f "$other_partition_tmpdir/file" || fail=1
--- a/tests/rm/r-root.sh
+++ b/tests/rm/r-root.sh
@@ -104,7 +104,7 @@ test -d dir && framework_failure_
 # rm(1) must succeed as before, but this time both the evidence file "x"
 # and the test file / directory must still exist afterward.
 mkdir dir || framework_failure_
-: > file  || framework_failure_
+> file    || framework_failure_

 skip=
 for file in dir file ; do
@@ -168,8 +168,8 @@ done
 # Exercise "rm -r file1 / file2".
 # Expect a non-Zero exit status representing failure to remove "/",
 # yet 'file1' and 'file2' should be removed.
-: > file1 || framework_failure_
-: > file2 || framework_failure_
+> file1 || framework_failure_
+> file2 || framework_failure_

 # Now that we know that 'rm' won't call the unlinkat() system function for "/",
 # we could probably execute it without the LD_PRELOAD'ed safety net.
--- a/tests/split/l-chunk.sh
+++ b/tests/split/l-chunk.sh
@@ -71,7 +71,7 @@ DEBUGGING=
 test "$DEBUGGING" && test "$VERBOSE" && set +x
 for ELIDE_EMPTY in '' '-e'; do
  for IO_BLKSIZE in 1 2 5 10 80 100; do
-    : > out
+    > out
    test "$DEBUGGING" && printf "\n---io-blk-size=$IO_BLKSIZE $ELIDE_EMPTY\n"
    for N in 6 8 12 15 22; do
      rm -f x*
@@ -119,15 +119,15 @@ test "$DEBUGGING" && test "$VERBOSE" && set -x


 # Check extraction of particular chunks
-: > out
+> out
 printf '1\n12345\n' > exp
 split -n l/13/15 in > out
 compare exp out || fail=1
-: > out
+> out
 printf '' > exp
 split -n l/14/15 in > out
 compare exp out || fail=1
-: > out
+> out
 printf '1\n12345\n1\n' > exp
 split -n l/15/15 in > out
 compare exp out || fail=1
--- a/tests/split/line-bytes.sh
+++ b/tests/split/line-bytes.sh
@@ -63,8 +63,8 @@ cat <<\EOF > no_eol_splits_exp
 EOF

 for b in $(seq 10); do
-  : > splits
-  : > no_eol_splits
+  > splits
+  > no_eol_splits
  for s in $(seq 11); do
    rm x??
    split ---io=$b -C$s in || fail=1
--- a/tests/tail-2/inotify-rotate.sh
+++ b/tests/tail-2/inotify-rotate.sh
@@ -47,7 +47,7 @@ for i in $(seq 50); do
    # Normally less than a second is required here, but with heavy load
    # and a lot of disk activity, even 20 seconds is insufficient, which
    # leads to this timeout killing tail before the "ok" is written below.
-    :>k && :>x || framework_failure_ failed to initialize files
+    >k && >x || framework_failure_ failed to initialize files
    timeout 40 tail -F k > out 2>&1 &
    pid=$!
    sleep .1
--- a/tests/tail-2/retry.sh
+++ b/tests/tail-2/retry.sh
@@ -45,7 +45,7 @@ grep -F 'tail: warning: --retry ignored' out || fail=1
 # === Test:
 # Ensure that "tail --retry --follow=name" waits for the file to appear.
 # Clear 'out' so that we can check its contents without races
-:>out                           || framework_failure_
+>out                            || framework_failure_
 timeout 10 tail -s.1 --follow=name --retry missing >out 2>&1 & pid=$!
 retry_delay_ wait4lines_ .1 6 1 || fail=1  # Wait for "cannot open" error.
 echo "X" > missing              || fail=1
--- a/tests/tail-2/symlink.sh
+++ b/tests/tail-2/symlink.sh
@@ -32,7 +32,7 @@ wait4lines_ ()
 # Ensure changing targets of cli specified symlinks are handled.
 # Prior to v8.22, inotify would fail to recognize changes in the targets.
 # Clear 'out' so that we can check its contents without races.
-:>out                           || framework_failure_
+>out                            || framework_failure_
 ln -nsf target symlink          || framework_failure_
 timeout 10 tail -s.1 -F symlink >out 2>&1 & pid=$!
 retry_delay_ wait4lines_ .1 6 1 || fail=1  # Wait for "cannot open..."
@@ -50,7 +50,7 @@ rm -f target out           || framework_failure_
 # Ensure we correctly handle the source symlink itself changing.
 # I.E. that we don't operate solely on the targets.
 # Clear 'out' so that we can check its contents without races.
-:>out                           || framework_failure_
+>out                            || framework_failure_
 echo "X1" > target1             || framework_failure_
 ln -nsf target1 symlink         || framework_failure_
 timeout 10 tail -s.1 -F symlink >out 2>&1 & pid=$!
--- a/tests/tail-2/wait.sh
+++ b/tests/tail-2/wait.sh
@@ -52,7 +52,7 @@ for inotify in ---disable-inotify ''; do
  grep -Ev 'inotify (resources exhausted|cannot be used)' tail.err > x
  mv x tail.err
  test -s tail.err && fail=1
-  :>tail.err
+  >tail.err

  tail_F()
  {
--- a/tests/touch/read-only.sh
+++ b/tests/touch/read-only.sh
@@ -20,7 +20,7 @@
 print_ver_ touch
 skip_if_root_

-: > read-only || framework_failure_
+> read-only || framework_failure_
 chmod 444 read-only || framework_failure_