Re: patch for a race condition and a related bug in cp

[Paul Eggert]

Jim Meyering <address@hidden> writes:

> What I'd really like is a test case to exercise (and prevent
> reintroduction of) the race condition bug.  Even if it has to
> involve use of a debugger and/or sleeping in the test harness.

Well, as you know I'm a bit leery of debuggers and/or sleeps, but I
had forgotten that we have a way to test for this race condition
without resorting to those things.  Here's a patch that adds a test
case.

2007-02-02  Paul Eggert  <address@hidden>

        * tests/cp/parent-perm-race: Test for cp --preserve=mode race.

* tests/cp/parent-perm-race: Test for cp --preserve=mode race.
diff --git a/tests/cp/parent-perm-race b/tests/cp/parent-perm-race
index 80c95a6..d2870bc 100755
--- a/tests/cp/parent-perm-race
+++ b/tests/cp/parent-perm-race
@@ -1,7 +1,7 @@
 #!/bin/sh
 # Make sure cp -pR --parents isn't too generous with parent permissions.

-# Copyright (C) 2006 Free Software Foundation, Inc.
+# Copyright (C) 2006, 2007 Free Software Foundation, Inc.

 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -32,37 +32,47 @@ framework_failure=0
 mkdir -p $tmp || framework_failure=1
 cd $tmp || framework_failure=1

-umask 022
-mkdir -p a d || framework_failure=1
-mkfifo a/fifo || {
-  echo "$0: fifos not supported; skipping this test." 1>&2
-  (exit 77); exit 77
-}
-
-# Copy a fifo's contents.  That way, we can examine d/a's
-# state while cp is running.
-cp -p -R --copy-contents --parents a d &
-cp_pid=$!
-
-(
-  # Now 'cp' is reading the fifo.
-  # Check the permissions of the temporary destination
-  # directory that 'cp' has made.
-  ls -ld d/a >d/ls.out
-
-  # Close the fifo so that "cp" can continue.  But output first,
-  # before exiting, otherwise some shells would optimize away the file
-  # descriptor that holds the fifo open.
-  echo foo
-) >a/fifo
-
-case `cat d/ls.out` in
-d???--[-S]--[-S]*)
-  fail=0;;
-*)
-  fail=1;;
-esac
-
-wait $cp_pid || fail=1
+umask 002
+mkdir mode ownership d || framework_failure=1
+chmod g+s d 2>/dev/null # The cp test is valid either way.
+
+fail=0
+
+for attr in mode ownership
+do
+  mkfifo $attr/fifo || {
+    echo "$0: fifos not supported; skipping this test." 1>&2
+    (exit 77); exit 77
+  }
+
+  # Copy a fifo's contents.  That way, we can examine d/$attr's
+  # state while cp is running.
+  cp --preserve=$attr -R --copy-contents --parents $attr d &
+  cp_pid=$!
+
+  (
+    # Now 'cp' is reading the fifo.
+    # Check the permissions of the temporary destination
+    # directory that 'cp' has made.
+    ls -ld d/$attr >d/$attr.ls
+
+    # Close the fifo so that "cp" can continue.  But output first,
+    # before exiting, otherwise some shells would optimize away the file
+    # descriptor that holds the fifo open.
+    echo foo
+  ) >$attr/fifo
+
+  ls_output=`cat d/$attr.ls` || fail=1
+  case $attr,$ls_output in
+  ownership,d???--[-S]--[-S]* | \
+  mode,d????-??-?* | \
+  mode,d??[-x]?w[-x]?-[-x]* )
+    ;;
+  *)
+    fail=1;;
+  esac
+
+  wait $cp_pid || fail=1
+done

 (exit $fail); exit $fail
M ChangeLog
M tests/cp/parent-perm-race
Committed as 961c142f537f7b5b92bfe3379033ce211318ffa7