This is the mail archive of the cygwin@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Fixes to rename and unlink. Cygwin 1.3.1. Failed SAMBA deletes?


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]