Mailing-List: contact cygwin-help AT sourceware DOT cygnus DOT com; run by ezmlm List-Subscribe: List-Archive: List-Post: List-Help: , Sender: cygwin-owner AT sources DOT redhat DOT com Delivered-To: mailing list cygwin AT sources DOT redhat DOT com From: "Neil Erskine" To: Subject: Fixes to rename and unlink. Cygwin 1.3.1. Failed SAMBA deletes? Date: Tue, 22 May 2001 21:43:15 -0300 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit X-Priority: 3 (Normal) X-MSMail-Priority: Normal X-Mailer: Microsoft Outlook IMO, Build 9.0.2416 (9.0.2911.0) Importance: Normal X-MimeOLE: Produced By Microsoft MimeOLE V5.50.4133.2400 A recent note referenced clever code utilizing the DeleteOnClose flag in reference to failed SAMBA deletes. Note that the clever code in unlink doesn't work with novell shares either, although for different reasons. Fixes to unlink and rename are part of this message. I fixed remove (unlink) from cygwin1.dll version 1.3.1 in the following, which adds a lot of debugging lines and comments, but differs from the old code only in removing one instance of "os_being_run == winNT" at the point where Novell is mentioned. I doubt that this will fix the SAMBA issue, but if anyone is looking at fixing remove for SAMBA, they might incorporate this fix too. I haven't tested it on anything other than NTFS and Novell filesystems accessed from Windows 2000. Stylisticly, the loop in unlink doesn't belong here. Unlink's current form is: for (i=0; i < 2; i++) { Try basic algorithm for delete. Hacky bit with Create/Open If this is the second time through loop, and delete didn't work break. Change the mode of the file to enable writes } It would be easier to read if the basic delete algorithm was put in a function and unlink written: TryBasicAlgorithm(); If that didn't work { /* Hacky stuff here, or in basic algorithm */ Change the mode; TryBasicAlgorithm(); } I haven't done that as I can't tell why the Create/Close should work (and doesn't on Novell), so am not sure whether it belongs as part of the basic algorithm, or inside the if statement. The loop in unlink is not nearly as bad as the potentially infinate loop in rename. I rewrote that loop to be at least bounded (fix below also), again removing a check for WinNT that seemed unnecessary. However, I have no idea why renaming a read-only file on Novell ends up with the file being read-only: the "Novell" hack referenced in rename's strace debugging lines should make such files publicly writable. However, even though the line including "permissions mangled" does come out, the chmod seems to have no effect but to make the following MoveFile call work as expected! The "hack" doesn't happen on NTFS volumes as the initial MoveFile call works, so rename works there. I have not tried it with FAT, on on other OS platforms. I added a sloughof comments to rename. Hope all this is of assistance to someone. With the attached changes to unlink and rename, cvs seems to work on Novell shares. Revised source to unlink, works on Novell and NTFS with Win 2000. ------------------------------------------ extern "C" int _unlink (const char *ourname) { int res = -1; sigframe thisframe (mainthread); path_conv win32_name (ourname, PC_SYM_NOFOLLOW | PC_FULL); if (win32_name.error) { set_errno (win32_name.error); goto done; } syscall_printf ("_unlink (%s)", win32_name.get_win32 ()); DWORD atts; atts = win32_name.file_attributes (); if (atts != 0xffffffff && atts & FILE_ATTRIBUTE_DIRECTORY) { syscall_printf ("unlinking a directory"); set_errno (EPERM); goto done; } /* Windows won't check the directory mode, so we do that ourselves. */ if (!writable_directory (win32_name)) { syscall_printf ("non-writable directory"); goto done; } /* Check for shortcut as symlink condition. */ syscall_printf ("atts = %x", atts); if (atts != 0xffffffff && atts & FILE_ATTRIBUTE_READONLY) { int len = strlen (win32_name.get_win32 ()); syscall_printf ("read only"); if (len > 4 && strcasematch (win32_name.get_win32 () + len - 4, ".lnk")) { SetFileAttributes (win32_name.get_win32 (), win32_name.file_attributes () & ~FILE_ATTRIBUTE_READONLY); syscall_printf ("atts after = %x", win32_name.file_attributes ()); } } for (int i = 0; i < 2; i++) { if (DeleteFile (win32_name)) { syscall_printf ("DeleteFile succeeded"); res = 0; break; } DWORD lasterr; lasterr = GetLastError (); syscall_printf ("DeleteFile failed with error %x", lasterr); /* FIXME: There's a race here. */ HANDLE h = CreateFile (win32_name, GENERIC_READ, FILE_SHARE_READ, &sec_none_nih, OPEN_EXISTING, FILE_FLAG_DELETE_ON_CLOSE, 0); if (h != INVALID_HANDLE_VALUE) { CloseHandle (h); syscall_printf ("CreateFile/CloseHandle succeeded"); /* Novell fix here. Removed check for running NT. */ if (GetFileAttributes (win32_name) == (DWORD) -1) { syscall_printf ("Good bye, all well"); res = 0; break; } } else { syscall_printf ( "CreateFile failed with error %x", GetLastError() ); } if (i > 0) { DWORD dtype; if (os_being_run == winNT || lasterr != ERROR_ACCESS_DENIED) { syscall_printf ("Good bye, error"); goto err; } char root[MAX_PATH]; strcpy (root, win32_name); dtype = GetDriveType (rootdir (root)); if (dtype & DRIVE_REMOTE) { syscall_printf ("access denied on remote drive"); goto err; /* Can't detect this, unfortunately */ } lasterr = ERROR_SHARING_VIOLATION; } syscall_printf ("i %d, couldn't delete file, %E", i); /* If we get ERROR_SHARING_VIOLATION, the file may still be open - Windows NT doesn't support deleting a file while it's open. */ if (lasterr == ERROR_SHARING_VIOLATION) { syscall_printf ("Sharing violation"); cygwin_shared->delqueue.queue_file (win32_name); res = 0; break; } /* if access denied, chmod to be writable in case it is not and try again */ /* FIXME: Should check whether ourname is directory or file and only try again if permissions are not sufficient */ if (lasterr == ERROR_ACCESS_DENIED && chmod (win32_name, 0777) == 0) continue; err: __seterrno (); res = -1; break; } done: syscall_printf ("%d = unlink (%s)", res, ourname); return res; } --------------------- Fixed version of rename: -------------------------------- extern "C" int _rename (const char *oldpath, const char *newpath) { sigframe thisframe (mainthread); int res = 0; path_conv real_old (oldpath, PC_SYM_NOFOLLOW); if (real_old.error) { syscall_printf ("-1 = rename (%s, %s): real_old.error", oldpath, newpath); set_errno (real_old.error); return -1; } path_conv real_new (newpath, PC_SYM_NOFOLLOW); /* Shortcut hack. */ char new_lnk_buf[MAX_PATH + 5]; if (real_old.issymlink () && !real_new.error && !real_new.case_clash) { int len_old = strlen (real_old.get_win32 ()); if (strcasematch (real_old.get_win32 () + len_old - 4, ".lnk")) { strcpy (new_lnk_buf, newpath); strcat (new_lnk_buf, ".lnk"); newpath = new_lnk_buf; real_new.check (newpath, PC_SYM_NOFOLLOW); } } if (real_new.error || real_new.case_clash) { syscall_printf ("-1 = rename (%s, %s): real_new.error", oldpath, newpath); set_errno (real_new.case_clash ? ECASECLASH : real_new.error); return -1; } if (!writable_directory (real_old.get_win32 ()) || !writable_directory (real_new.get_win32 ())) { syscall_printf ("-1 = rename (%s, %s): not writable", oldpath, newpath); set_errno (EACCES); return -1; } if (real_old.file_attributes () == (DWORD) -1) /* source file doesn't exist*/ { syscall_printf ("file to move doesn't exist"); set_errno (ENOENT); return (-1); } if (real_new.file_attributes () != (DWORD) -1 && real_new.file_attributes () & FILE_ATTRIBUTE_READONLY) { /* Destination file exists and is read only, change that or else the rename won't work. */ syscall_printf ("Destination read-only"); SetFileAttributesA (real_new.get_win32 (), real_new.file_attributes () & ~ FILE_ATTRIBUTE_READONLY); } if (MoveFile (real_old.get_win32 (), real_new.get_win32 ())) { syscall_printf ( "MoveFile worked" ); } else { int HackWorked = (1 == 0); DWORD LastError = GetLastError(); syscall_printf ( "MoveFile failed %x", LastError ); /* Normal approach didn't work. Try some hacks that might. */ switch ( LastError ) { case ERROR_ALREADY_EXISTS: case ERROR_FILE_EXISTS: if (os_being_run == winNT) { /* Previous code seemed to suggest that MoveFileEx sometime * worked even when it shouldn't be necessary. Try that for * WinNT systems (the call doesn't exist on non-WinNT * systems, so it isn't safe to try there). */ if (MoveFileEx (real_old.get_win32 (), real_new.get_win32 (), MOVEFILE_REPLACE_EXISTING)) { syscall_printf ( "MoveFileEx worked" ); HackWorked = (1 == 1); } else { syscall_printf ( "MoveFileEx failed %x", GetLastError() ); } } break; case ERROR_ACCESS_DENIED: chmod ( real_old.get_win32 (), 0777 ); if (MoveFile (real_old.get_win32 (), real_new.get_win32 ())) { syscall_printf ("Novell hack succeeded, permissions mangled."); HackWorked = (1 == 1); } break; default: syscall_printf ( "No NT specific solution for problem" ); break; } /* Previous code seemed to suggest that on non-WinNT systems the * following bizarre loop would sometimes work (no reason given as * to why). Try it too. I took out the restriction that it wasn't * tried on WinNT, as I couldn't see any danger in it. The worst * that can happen is it fails (very slowly). */ if ( ! HackWorked ) { int Done = 0; int Attempt; syscall_printf ("win95 hack"); /* Warning. I have converted a potentially infinite loop to a * bounded loop. I have no information to indicate that the * loop cycle limit I selected is a reasonable upper * limit. Neither do I have any information to indicate that * this loop ever accomplishes anything. */ for ( Attempt=0; Done == 0 && Attempt < 100; Attempt++ ) { syscall_printf ("Try %d", Attempt ); if (!DeleteFileA (real_new.get_win32 ()) && GetLastError () != ERROR_FILE_NOT_FOUND) { syscall_printf ("deleting %s to be paranoid", real_new.get_win32 ()); res = -1; Done = 1; } else { if (MoveFile (real_old.get_win32 (), real_new.get_win32 ())) { syscall_printf ( "Win95 hack worked" ); Done = 1; } } } if ( ! Done ) { syscall_printf ( "Win95 hack didn't work" ); res = -1; } } } if (res) __seterrno (); if (res == 0) { /* make the new file have the permissions of the old one */ SetFileAttributesA (real_new.get_win32 (), real_old.file_attributes ()); } syscall_printf ("%d = rename (%s, %s) return", res, real_old.get_win32 (), real_new.get_win32 ()); return res; } -------------------------------- Neil Erskine voice 902 423 7727 ext. 230 fax 902 422 8108 -- Want to unsubscribe from this list? Check out: http://cygwin.com/ml/#unsubscribe-simple