Re: [PATCH 0/7] contents of topic/libposix for merge to master

[Bruno Haible]

Hello Eric,

Gary reported:
>>    ix86 RHEL 5       gcc 4.1.2      (rename, renameat)
>>    x86_64 RHEL 5     gcc 4.1.2      (dprintf-posix2.sh, fprintf-posix3.sh
>>                                      rename, renameat)

I'm seeing the failures on Linux 2.6.18 systems:

  test-rename.h:119: assertion failed
  FAIL: test-rename
  test-rename.h:119: assertion failed
  FAIL: test-renameat

The cause is that the test happens on an NFS file system. After the line 117
has been executed (a rename of a non-empty directory over an empty directory),
for 30 seconds
  - the test whether BASE "dir" exists, via a readdir() loop, will tell
    that it is nonexistent,
  - but  stat (BASE "dir", &st)  will return 0, telling that the directory
    still exists.

sleep(1) and sync() don't help about this situation; only waiting 30 seconds
helps.

What should we do?
  a) Patch the test so that it uses a readdir() loop to detect the absence of
     the file even when stat() pretends it's still present. Or
  b) Use an rpl_rename override that will make the unit test work.

I think this behaviour of the NFS cache could also affect subsequent code
inside the coreutils 'mv' program, or even later commands in users' shell
scripts, therefore I would favour b).

For a), find attached a draft patch to the test (in its old, unreadable form).

For b), the fix is to enable the lib/rename.c replacement, with
  #define RENAME_DEST_EXISTS_BUG 1
  #define RENAME_HARD_LINK_BUG 1
Yes, both are needed. The first one fixes
  test-rename.h:119: assertion failed
The second one then fixes
  test-rename.h:191: assertion failed

But this will require an additional test in m4/rename.m4.

Which way you prefer?

Bruno


*** test-rename.h    Fri Jan  1 09:50:44 2010
--- test-rename.h       Fri Nov 12 23:33:25 2010
***************
*** 14,19 ****
--- 14,53 ----
     You should have received a copy of the GNU General Public License
     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
  
+ #include <dirent.h>
+ 
+ static int dentry_exists (const char *filename)
+ {
+   int exists = 0;
+   DIR *dir = opendir (".");
+   ASSERT (dir != NULL);
+   for (;;)
+     {
+       struct dirent *d = readdir (dir);
+       if (d == NULL)
+         break;
+       if (strcmp (filename, d->d_name) == 0)
+         {
+           exists = 1;
+           break;
+         }
+     }
+   closedir (dir);
+   return exists;
+ }
+ 
+ static void assert_nonexistent (const char *filename)
+ {
+   struct stat st;
+ 
+   errno = 0;
+   if (stat (filename, &st) == -1)
+     ASSERT (errno == ENOENT);
+   else
+     /* Deal with NFS cache that keeps info about files present for 30 
seconds.  */
+     ASSERT (!dentry_exists (filename));
+ }
+ 
  /* This file is designed to test both rename(a,b) and
     renameat(AT_FDCWD,a,AT_FDCWD,b).  FUNC is the function to test.
     Assumes that BASE and ASSERT are already defined, and that
***************
*** 115,123 ****
    ASSERT (func (BASE "dir2", BASE "dir/") == -1);
    ASSERT (errno == EEXIST || errno == ENOTEMPTY);
    ASSERT (func (BASE "dir", BASE "dir2") == 0); /* Full onto empty.  */
!   errno = 0;
!   ASSERT (stat (BASE "dir", &st) == -1);
!   ASSERT (errno == ENOENT);
    ASSERT (stat (BASE "dir2/file", &st) == 0);
    ASSERT (mkdir (BASE "dir", 0700) == 0);
    ASSERT (func (BASE "dir2/", BASE "dir") == 0);
--- 149,155 ----
    ASSERT (func (BASE "dir2", BASE "dir/") == -1);
    ASSERT (errno == EEXIST || errno == ENOTEMPTY);
    ASSERT (func (BASE "dir", BASE "dir2") == 0); /* Full onto empty.  */
!   assert_nonexistent (BASE "dir");
    ASSERT (stat (BASE "dir2/file", &st) == 0);
    ASSERT (mkdir (BASE "dir", 0700) == 0);
    ASSERT (func (BASE "dir2/", BASE "dir") == 0);
***************
*** 127,135 ****
    ASSERT (errno == ENOENT);
    ASSERT (mkdir (BASE "dir2", 0700) == 0);
    ASSERT (func (BASE "dir", BASE "dir2/") == 0);
!   errno = 0;
!   ASSERT (stat (BASE "dir", &st) == -1);
!   ASSERT (errno == ENOENT);
    ASSERT (stat (BASE "dir2/file", &st) == 0);
    ASSERT (unlink (BASE "dir2/file") == 0);
    errno = 0; /* Reject trailing dot.  */
--- 159,165 ----
    ASSERT (errno == ENOENT);
    ASSERT (mkdir (BASE "dir2", 0700) == 0);
    ASSERT (func (BASE "dir", BASE "dir2/") == 0);
!   assert_nonexistent (BASE "dir");
    ASSERT (stat (BASE "dir2/file", &st) == 0);
    ASSERT (unlink (BASE "dir2/file") == 0);
    errno = 0; /* Reject trailing dot.  */