rename, fchdir: don't ignore chdir failure

Although we just checked that chdir(cwd) worked, there is a
race where it could disappear while we are temporarily away.
If that happens, forcefully give up rather than proceeding
in the wrong directory.

* lib/fchdir.c (get_name): Abort on unexpected chdir failure.
* lib/rename.c (rpl_rename) [W32]: Likewise.
(rpl_rename) [RENAME_DEST_EXISTS_BUG]: Avoid one case of losing
an empty destination directory if source cannot be renamed,
although there is still possibility for failure.
* doc/posix-functions/rename.texi (rename): Document the race.
Reported by Jim Meyering.

Signed-off-by: Eric Blake <ebb9@byu.net>

diff --git a/ChangeLog b/ChangeLog
index a8e9cf23a9af6949f7ef224ee468c224cf177679..43b4bfeb04799ac32a529fa729d6a533272102d3 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2009-10-02  Eric Blake  <ebb9@byu.net>
 
+       rename, fchdir: don't ignore chdir failure
+       * lib/fchdir.c (get_name): Abort on unexpected chdir failure.
+       * lib/rename.c (rpl_rename) [W32]: Likewise.
+       (rpl_rename) [RENAME_DEST_EXISTS_BUG]: Avoid one case of losing
+       an empty destination directory if source cannot be renamed,
+       although there is still possibility for failure.
+       * doc/posix-functions/rename.texi (rename): Document the race.
+       Reported by Jim Meyering.
+
        maint: cleanup whitespace in recent commits
        * lib/rename.c (rpl_rename): Remove tabs.
        * tests/test-link.h (test_link): Likewise.

diff --git a/doc/posix-functions/rename.texi b/doc/posix-functions/rename.texi
index 65981db9d2c19e863fc0b6908e19a945ad532242..be997aae1e683b3b128939b449e23ce2a6392771 100644 (file)
--- a/doc/posix-functions/rename.texi
+++ b/doc/posix-functions/rename.texi
@@ -27,7 +27,9 @@ NetBSD 1.6, Cygwin 1.5.x.
 @item
 This function will not always replace an existing destination on some
 platforms:
-mingw.
+Cygwin 1.5.x, mingw.
+However, the replacement is not atomic for directories, and may end up
+losing the empty destination if the source could not be renamed.
 @item
 This function mistakenly allows names ending in @samp{.} or @samp{..}
 on some platforms:

diff --git a/lib/fchdir.c b/lib/fchdir.c
index 54bbc623dbc18e8fdfd7b0362c14823d056661b7..e55ef6ce961ebc7c1f09a0fc786c72d034bc53bd 100644 (file)
--- a/lib/fchdir.c
+++ b/lib/fchdir.c
@@ -106,7 +106,8 @@ get_name (char const *dir)
        return NULL;
       result = chdir (dir) ? NULL : getcwd (NULL, 0);
       saved_errno = errno;
-      chdir (cwd);
+      if (chdir (cwd))
+       abort ();
       free (cwd);
       errno = saved_errno;
     }

diff --git a/lib/rename.c b/lib/rename.c
index 44c9e75c2c1b1919a40ee2835491c640089f6c01..71b80321a8adb1758d746048acbdc55f80ff3bbb 100644 (file)
--- a/lib/rename.c
+++ b/lib/rename.c
@@ -113,13 +113,16 @@ rpl_rename (char const *src, char const *dst)
      replace an existing empty directory, so we have to help it out.
      And canonicalize_file_name is not yet ported to mingw; however,
      for directories, getcwd works as a viable alternative.  Ensure
-     that we can get back to where we started before using it.  */
+     that we can get back to where we started before using it; later
+     attempts to return are fatal.  Note that we can end up losing a
+     directory if rename then fails, but it was empty, so not much
+     damage was done.  */
   if (dst_exists && S_ISDIR (dst_st.st_mode))
     {
       char *cwd = getcwd (NULL, 0);
       char *src_temp;
       char *dst_temp;
-      if (chdir (cwd))
+      if (!cwd || chdir (cwd))
         return -1;
       if (IS_ABSOLUTE_FILE_NAME (src))
         {
@@ -129,11 +132,12 @@ rpl_rename (char const *src, char const *dst)
       else
         {
           src_temp = chdir (src) ? NULL : getcwd (NULL, 0);
-          if (!IS_ABSOLUTE_FILE_NAME (dst))
-            chdir (cwd);
+          if (!IS_ABSOLUTE_FILE_NAME (dst) && chdir (cwd))
+            abort ();
           dst_temp = chdir (dst) ? NULL : getcwd (NULL, 0);
         }
-      chdir (cwd);
+      if (chdir (cwd))
+        abort ();
       free (cwd);
       if (!src_temp || !dst_temp)
         {
@@ -421,9 +425,16 @@ rpl_rename (char const *src, char const *dst)
      directory on top of an empty one (the old directory name can
      reappear if the new directory tree is removed).  Work around this
      by removing the target first, but don't remove the target if it
-     is a subdirectory of the source.  */
+     is a subdirectory of the source.  Note that we can end up losing
+     a directory if rename then fails, but it was empty, so not much
+     damage was done.  */
   if (dst_exists && S_ISDIR (dst_st.st_mode))
     {
+      if (src_st.st_dev != dst_st.st_dev)
+        {
+          rename_errno = EXDEV;
+          goto out;
+        }
       if (src_temp != src)
         free (src_temp);
       src_temp = canonicalize_file_name (src);