Re: three small patches

[Pádraig Brady]

Jim Meyering wrote:
> Subject: [PATCH 1/3] tests: cp/reflink-auto guard against a pathological 
> $TMPDIR
> 
> * tests/cp/reflink-auto: Add quotes.
> 
> diff --git a/tests/cp/reflink-auto b/tests/cp/reflink-auto
> index ff2b1b3..d1f6b2b 100755
> --- a/tests/cp/reflink-auto
> +++ b/tests/cp/reflink-auto
> @@ -26,7 +26,7 @@ fi
>  cleanup_() { rm -rf "$other_partition_tmpdir"; }
>  . "$abs_srcdir/other-fs-tmpdir"
>  a_other="$other_partition_tmpdir/a"
> -rm -f $a_other || framework_failure
> +rm -f "$a_other" || framework_failure

I'd copied that hunk from misc/xattr which had
copied it from elsewhere. So I've fixed those up
also along with another few I noticed.

The attached patch passes all tests.

cheers,
Pádraig.

>From cf91aea37681ea33dd5ba3e13f64522484abfaa0 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <address@hidden>
Date: Sun, 30 Aug 2009 01:27:45 +0100
Subject: [PATCH] tests: don't misbehave if $TMPDIR contains spaces

* tests/misc/xattr: Add quotes.
* tests/mv/acl: Likewise.
* tests/mv/backup-is-src: Likewise.
* tests/mv/hard-link-1: Likewise.
* tests/mv/leak-fd: Likewise.
* tests/mv/mv-special-1: Likewise.
* tests/mv/part-fail: Likewise.
* tests/mv/part-hardlink: Likewise.
* tests/mv/part-rename: Likewise.
* tests/mv/part-symlink: Likewise.
* tests/mv/partition-perm: Likewise.
* tests/mv/sticky-to-xpart: Likewise.
---
 tests/misc/xattr         |   14 +++++++-------
 tests/mv/acl             |   12 ++++++------
 tests/mv/backup-is-src   |    8 ++++----
 tests/mv/hard-link-1     |    4 ++--
 tests/mv/leak-fd         |    2 +-
 tests/mv/mv-special-1    |   10 +++++-----
 tests/mv/part-fail       |    6 +++---
 tests/mv/part-hardlink   |    6 +++---
 tests/mv/part-rename     |    2 +-
 tests/mv/part-symlink    |   10 +++++-----
 tests/mv/partition-perm  |    6 +++---
 tests/mv/sticky-to-xpart |    2 +-
 12 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/tests/misc/xattr b/tests/misc/xattr
index f067ff5..5dd044c 100755
--- a/tests/misc/xattr
+++ b/tests/misc/xattr
@@ -36,7 +36,7 @@ cp --preserve=xattr -n src dest 2>/dev/null \
 cleanup_() { rm -rf "$other_partition_tmpdir"; }
 . "$abs_srcdir/other-fs-tmpdir"
 b_other="$other_partition_tmpdir/b"
-rm -f $b_other || framework_failure
+rm -f "$b_other" || framework_failure
 
 # testing xattr name-value pair
 xattr_name="user.foo"
@@ -98,17 +98,17 @@ EOF
 
 # try to set user xattr on file on other partition
 test_mv=1
-touch $b_other || framework_failure
-setfattr -n "$xattr_name" -v "$xattr_value" $b_other >out_a 2>/dev/null \
+touch "$b_other" || framework_failure
+setfattr -n "$xattr_name" -v "$xattr_value" "$b_other" >out_a 2>/dev/null \
   || test_mv=0
-getfattr -d $b_other >out_b 2>/dev/null || test_mv=0
+getfattr -d "$b_other" >out_b 2>/dev/null || test_mv=0
 grep -F "$xattr_pair" out_b >/dev/null || test_mv=0
-rm -f $b_other || framework_failure
+rm -f "$b_other" || framework_failure
 
 if test $test_mv -eq 1; then
   # mv should preserve xattr when copying content from one partition to another
-  mv b $b_other || fail=1
-  getfattr -d $b_other >out_b 2>/dev/null || skip_test_ "failed to get xattr 
of file"
+  mv b "$b_other" || fail=1
+  getfattr -d "$b_other" >out_b 2>/dev/null || skip_test_ "failed to get xattr 
of file"
   grep -F "$xattr_pair" out_b >/dev/null || fail=1
 else
   cat >&2 <<EOF
diff --git a/tests/mv/acl b/tests/mv/acl
index e4e31dd..5ad8de0 100755
--- a/tests/mv/acl
+++ b/tests/mv/acl
@@ -49,24 +49,24 @@ test $skip_partition != none &&
   skip_test_ "'$skip' is not on a suitable file system for this test"
 
 # move the access acl of a file
-mv file $other_partition_tmpdir || fail=1
-acl2=`cd $other_partition_tmpdir && getfacl file` || framework_failure
+mv file "$other_partition_tmpdir" || fail=1
+acl2=`cd "$other_partition_tmpdir" && getfacl file` || framework_failure
 test "$acl1" = "$acl2" || fail=1
 
 # move the access acl of a directory
 mkdir dir || framework_failure
 setfacl -m user:bin:rw dir || framework_failure
 acl1=`getfacl dir` || framework_failure
-mv dir $other_partition_tmpdir || fail=1
-acl2=`cd $other_partition_tmpdir && getfacl dir` || framework_failure
+mv dir "$other_partition_tmpdir" || fail=1
+acl2=`cd "$other_partition_tmpdir" && getfacl dir` || framework_failure
 test "$acl1" = "$acl2" || fail=1
 
 # move the default acl of a directory
 mkdir dir2 || framework_failure
 setfacl -d -m user:bin:rw dir2 || framework_failure
 acl1=`getfacl dir2` || framework_failure
-mv dir2 $other_partition_tmpdir || fail=1
-acl2=`cd $other_partition_tmpdir && getfacl dir2` || framework_failure
+mv dir2 "$other_partition_tmpdir" || fail=1
+acl2=`cd "$other_partition_tmpdir" && getfacl dir2` || framework_failure
 test "$acl1" = "$acl2" || fail=1
 
 Exit $fail
diff --git a/tests/mv/backup-is-src b/tests/mv/backup-is-src
index 506c149..df6561b 100755
--- a/tests/mv/backup-is-src
+++ b/tests/mv/backup-is-src
@@ -28,12 +28,12 @@ cleanup_() { rm -rf "$other_partition_tmpdir"; }
 a="$other_partition_tmpdir/a"
 a2="$other_partition_tmpdir/a~"
 
-rm -f $a $a2 || framework_failure
-echo a > $a || framework_failure
-echo a2 > $a2 || framework_failure
+rm -f "$a" "$a2" || framework_failure
+echo a > "$a" || framework_failure
+echo a2 > "$a2" || framework_failure
 
 # This mv command should exit nonzero.
-mv --b=simple $a2 $a > out 2>&1 && fail=1
+mv --b=simple "$a2" "$a" > out 2>&1 && fail=1
 
 sed \
    -e "s,mv:,XXX:," \
diff --git a/tests/mv/hard-link-1 b/tests/mv/hard-link-1
index 7bcc049..2df2cf3 100755
--- a/tests/mv/hard-link-1
+++ b/tests/mv/hard-link-1
@@ -32,10 +32,10 @@ mkdir $dir || framework_failure
 > $dir/a || framework_failure
 ln $dir/a $dir/b || framework_failure
 
-mv $dir $other_partition_tmpdir || fail=1
+mv $dir "$other_partition_tmpdir" || fail=1
 
 # Display inode numbers, one per line.
-ls -1i $other_partition_tmpdir/$dir > out || fail=1
+ls -1i "$other_partition_tmpdir/$dir" > out || fail=1
 
 # Make sure the inode numbers are the same.
 a=`sed -n 's/ a$//p' out`
diff --git a/tests/mv/leak-fd b/tests/mv/leak-fd
index 55a4e3a..76a6b43 100755
--- a/tests/mv/leak-fd
+++ b/tests/mv/leak-fd
@@ -50,7 +50,7 @@ test -f $last_file || framework_failure
 
 fail=0
 
-mv * $other_partition_tmpdir || fail=1
+mv * "$other_partition_tmpdir" || fail=1
 test -f $last_file/f && fail=1
 rm .dirs
 
diff --git a/tests/mv/mv-special-1 b/tests/mv/mv-special-1
index 208bc7f..321c54e 100755
--- a/tests/mv/mv-special-1
+++ b/tests/mv/mv-special-1
@@ -42,13 +42,13 @@ touch $dir/a/b/c/file1 $dir/d/e/f/file2 || framework_failure
 # It's not worth making an exception any more.
 
 fail=0
-mv --verbose $null $dir $other_partition_tmpdir > out || fail=1
+mv --verbose $null $dir "$other_partition_tmpdir" > out || fail=1
 # Make sure the files are gone.
 test -p $null && fail=1
 test -d $dir && fail=1
 # Make sure they were moved.
-test -p $other_partition_tmpdir/$null || fail=1
-test -d $other_partition_tmpdir/$dir/a/b/c || fail=1
+test -p "$other_partition_tmpdir/$null" || fail=1
+test -d "$other_partition_tmpdir/$dir/a/b/c" || fail=1
 
 # POSIX says rename (A, B) can succeed if A and B are on different file 
systems,
 # so ignore chatter about when files are removed and copied rather than 
renamed.
@@ -72,7 +72,7 @@ EOF
 
 compare out2 exp || fail=1
 
-# cd $other_partition_tmpdir
-# ls -l -A -R $other_partition_tmpdir
+# cd "$other_partition_tmpdir"
+# ls -l -A -R "$other_partition_tmpdir"
 
 Exit $fail
diff --git a/tests/mv/part-fail b/tests/mv/part-fail
index b1048e9..290864d 100755
--- a/tests/mv/part-fail
+++ b/tests/mv/part-fail
@@ -29,12 +29,12 @@ skip_if_root_
 cleanup_() { t=$other_partition_tmpdir; chmod -R 700 "$t"; rm -rf "$t"; }
 . "$abs_srcdir/other-fs-tmpdir"
 
-touch k $other_partition_tmpdir/k || framework_failure
-chmod u-w $other_partition_tmpdir || framework_failure
+touch k "$other_partition_tmpdir/k" || framework_failure
+chmod u-w "$other_partition_tmpdir" || framework_failure
 
 fail=0
 
-mv -f k $other_partition_tmpdir 2> out && fail=1
+mv -f k "$other_partition_tmpdir" 2> out && fail=1
 cat <<EOF > exp
 mv: inter-device move failed: \`k' to \`$other_partition_tmpdir/k'; unable to 
remove target: Permission denied
 EOF
diff --git a/tests/mv/part-hardlink b/tests/mv/part-hardlink
index 32d58d7..51dea20 100755
--- a/tests/mv/part-hardlink
+++ b/tests/mv/part-hardlink
@@ -36,10 +36,10 @@ ln a/1 b/1 || framework_failure
 
 fail=0
 
-mv f g $other_partition_tmpdir || fail=1
-mv a b $other_partition_tmpdir || fail=1
+mv f g "$other_partition_tmpdir" || fail=1
+mv a b "$other_partition_tmpdir" || fail=1
 
-cd $other_partition_tmpdir
+cd "$other_partition_tmpdir"
 set `ls -Ci f g`
 test $1 = $3 || fail=1
 set `ls -Ci a/1 b/1`
diff --git a/tests/mv/part-rename b/tests/mv/part-rename
index 0337220..45b8615 100755
--- a/tests/mv/part-rename
+++ b/tests/mv/part-rename
@@ -31,6 +31,6 @@ mkdir foo || framework_failure
 
 fail=0
 
-mv foo/ $other_partition_tmpdir/bar || fail=1
+mv foo/ "$other_partition_tmpdir/bar" || fail=1
 
 Exit $fail
diff --git a/tests/mv/part-symlink b/tests/mv/part-symlink
index 71788b9..a12a9f5 100755
--- a/tests/mv/part-symlink
+++ b/tests/mv/part-symlink
@@ -69,7 +69,7 @@ for copy in cp mv; do
     for options in '' --rem '--rem -d' '--rem -b' -b -bd -d; do
       case "$options" in *d*|*--rem*) test $copy = mv && continue;; esac
       rm -rf dir || fail=1
-      rm -f $other_partition_tmpdir/* || fail=1
+      rm -f "$other_partition_tmpdir"/* || fail=1
       mkdir dir || fail=1
       cd dir || fail=1
       case "$args" in *loc_reg*) reg_abs="`pwd`/$loc_reg" ;; esac
@@ -94,7 +94,7 @@ for copy in cp mv; do
           # and put brackets around the output.
           test -s .err && {
             echo '[' | tr -d '\n'
-            sed 's/^[^:][^:]*\(..\):/\1:/;s,'$other_partition_tmpdir/,, .err
+            sed 's/^[^:][^:]*\(..\):/\1:/;s,'"$other_partition_tmpdir/,," .err
             echo ']' | tr -d '\n'
             }
           # Strip off all but the file names.
@@ -102,13 +102,13 @@ for copy in cp mv; do
           ls=`ls -gG --ignore=.err . \
               | sed \
                   -e '/^total /d' \
-                  -e s,$other_partition_tmpdir/,, \
+                  -e "s,$other_partition_tmpdir/,," \
                   -e "s,$pwd_tmp/,," \
                   -e 's/^[^ ]*  *[^ ]*  *[^ ]*  *[^ ]*  *[^ ]*  *[^ ]*  *//'`
-          ls2=`cd $other_partition_tmpdir && ls -gG --ignore=.err . \
+          ls2=`cd "$other_partition_tmpdir" && ls -gG --ignore=.err . \
               | sed \
                   -e '/^total /d' \
-                  -e s,$other_partition_tmpdir/,, \
+                  -e "s,$other_partition_tmpdir/,," \
                   -e "s,$pwd_tmp/,," \
                   -e 's/^[^ ]*  *[^ ]*  *[^ ]*  *[^ ]*  *[^ ]*  *[^ ]*  *//'`
           echo "($ls) ($ls2)"
diff --git a/tests/mv/partition-perm b/tests/mv/partition-perm
index ee7d6a9..41a193c 100755
--- a/tests/mv/partition-perm
+++ b/tests/mv/partition-perm
@@ -29,14 +29,14 @@ cleanup_() { rm -rf "$other_partition_tmpdir"; }
 chmod a=rwx file
 
 umask 077
-mv file $other_partition_tmpdir
+mv file "$other_partition_tmpdir"
 
 fail=0
 test -f file && fail=1
-test -f $other_partition_tmpdir/file || fail=1
+test -f "$other_partition_tmpdir/file" || fail=1
 
 # This would have failed with the mv from fileutils-4.0i.
-mode=`ls -l $other_partition_tmpdir/file|cut -b-10`
+mode=`ls -l "$other_partition_tmpdir/file" | cut -b-10`
 test "$mode" = "-rwxrwxrwx" || fail=1
 
 Exit $fail
diff --git a/tests/mv/sticky-to-xpart b/tests/mv/sticky-to-xpart
index a4ab3dd..f0f9f95 100755
--- a/tests/mv/sticky-to-xpart
+++ b/tests/mv/sticky-to-xpart
@@ -52,7 +52,7 @@ case $version in
 esac
 
 setuidgid $NON_ROOT_USERNAME env PATH="$PATH" \
-  mv t/root-owned $other_partition_tmpdir 2> out-t && fail=1
+  mv t/root-owned "$other_partition_tmpdir" 2> out-t && fail=1
 
 # On some systems, we get `Not owner'.  Convert it.
 # On other systems (HPUX), we get `Permission denied'.  Convert it, too.
-- 
1.6.2.5