GNU bug report logs - #10284
23.2; "Renaming: permission denied" file-error in Windows

Previous Next

Full log

View this message in rfc822 format

From: LynX <_LynX <at> bk.ru>
To: 10284 <at> debbugs.gnu.org
Subject: bug#10284: "Renaming: permission denied" file-error in Windows
Date: Fri, 30 Dec 2011 21:31:26 +0200

[Message part 1 (text/plain, inline)]

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 <at> 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 (text/plain, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.