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.) > >