bug#10284: "Renaming: permission denied" file-error in Windows

[LynX]

Hello Eli,

Thank you for the clarification.

Please review this patch.

--- w32.c.orig  2011-11-26 05:20:20.000000000 +0200
+++ w32.c       2011-12-30 21:22:56.734375000 +0200
@@ -2857,6 +2857,8 @@
 {
   BOOL result;
   char temp[MAX_PATH];
+  int newname_dev;
+  int oldname_dev;

   /* MoveFile on Windows 95 doesn't correctly change the short file name
      alias in a number of circumstances (it is not easy to predict when
@@ -2873,6 +2875,9 @@

   strcpy (temp, map_w32_filename (oldname, NULL));

+  /* volume_info is set indirectly by map_w32_filename */
+  oldname_dev = volume_info.serialnum;
+
   if (os_subtype == OS_WIN95)
     {
       char * o;
@@ -2916,13 +2921,36 @@
      all the permutations of shared or subst'd drives, etc.)  */

   newname = map_w32_filename (newname, NULL);
+
+  /* volume_info is set indirectly by map_w32_filename */
+  newname_dev = volume_info.serialnum;
+
   result = rename (temp, newname);

-  if (result < 0
-      && errno == EEXIST
-      && _chmod (newname, 0666) == 0
-      && _unlink (newname) == 0)
-    result = rename (temp, newname);
+  if (result < 0)
+    {
+
+      if (errno == EACCES
+         && newname_dev != oldname_dev)
+       {
+         /* The implementation of `rename' on Windows does not return
+            errno = EXDEV when you are moving a directory to a different
+            storage device (ex. logical disk). It returns EACCES
+            instead. So here we handle such situations and return EXDEV. */
+         DWORD attributes;
+         if ((attributes = GetFileAttributes(temp)) != -1
+             && attributes & FILE_ATTRIBUTE_DIRECTORY)
+           errno = EXDEV;
+       }
+      else if (errno == EEXIST)
+       {
+         if (_chmod (newname, 0666) != 0)
+           return result;
+         if (_unlink (newname) != 0)
+           return result;
+         result = rename (temp, newname);
+       }
+    }

   return result;
 }

I've added check whether it is a directory or not, and moved all this 
before the unlink operation.

Regards,
LX

29.12.2011 8:18, Eli Zaretskii пишет:
> > Date: Wed, 28 Dec 2011 21:53:39 +0200
> > From: LynX<_LynX@bk.ru>
> >
> >   >  This first removes the target, and only then compares the device
> >   >  numbers.  Wouldn't it be better to do it the other way around, at
> >   >  least when the target is a directory?  That way, the target is left
> >   >  intact if the caller doesn't want to copy over the target, and also
> >   >  the filesystem is left in the same state as on Posix hosts in this
> >   >  case, i.e. the contract of `rename' is preserved on all systems.
> >
> > You mean that before setting errno to EXDEV we also need to check that
> > target is a directory (since files are moved correctly)?
>
> Yes, but that's not the important part of my comment above.  The
> important part is to move the check for different devices _before_ the
> call to _unlink which removes the target file/directory if it exists.
> In other words, we should fail and report EXDEV without risking the
> removal of the target, and let the caller decide what to do with the
> failure.  (If the caller decides to leave things unchanged, it will
> not be The Right Thing for us to remove the target.)

w32.c.patch
Description: Text document