libs/newlib-cygwin
libs
/
newlib-cygwin
mirror of git://sourceware.org/git/newlib-cygwin.git
4
0
Fork 0
Code Issues Projects Releases Wiki Activity

rename: Refactor "new file already exists and rename fails" case 

If newfile already exists and is in use, trying to overwrite it with
NtSetInformationFile(FileRenameInformation) fails exactly as if we
don't have the permissions to delete it.  Unfortunately the return code
is the same STATUS_ACCESS_DENIED, so we have no way to distinguish
these cases.  What we do here so far is to start a transaction to delete
newfile.  If this open fails with a transactional error we stop the
transaction and retry opening the file without transaction.

But, here's the problem: If newfile is in use, NtOpenFile(oldfile)
naturally does NOT fail with a transactional error.  Rather, the
subsequent call to unlink_nt(newfile) does, because there's another
handle open to newfile outside a transaction.  However, the code does
not check if unlink_nt fails with a transactional error and so fails
to retry without transaction.

This patch recifies the problem and checks unlink_nt's status as well.

Refactor code to get rid of goto into another code block.

Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
This commit is contained in:
Corinna Vinschen 2017-01-12 22:42:11 +01:00
parent e5cadbfdcd
commit 6ed4753e77
1 changed files with 29 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
winsup/cygwin/syscalls.cc
View File

@ -2509,9 +2509,10 @@ rename (const char *oldpath, const char *newpath)
if (status == STATUS_ACCESS_DENIED && dstpc->exists ()
&& !dstpc->isdir ())
{
bool need_open = false;
if ((dstpc->fs_flags () & FILE_SUPPORTS_TRANSACTIONS) && !trans)
{
start_transaction (old_trans, trans);
/* As mentioned earlier, opening the file must be part of the
transaction. Therefore we have to reopen the file here if the
transaction hasn't been started already. Unfortunately we
@ -2521,28 +2522,34 @@ rename (const char *oldpath, const char *newpath)
re-open it. Fortunately nothing has happened yet, so the
atomicity of the rename functionality is not spoiled. */
NtClose (fh);
retry_reopen:
start_transaction (old_trans, trans);
need_open = true;
}
while (true)
{
status = STATUS_SUCCESS;
if (need_open)
status = NtOpenFile (&fh, DELETE,
oldpc.get_object_attr (attr, sec_none_nih),
&io, FILE_SHARE_VALID_FLAGS,
FILE_OPEN_FOR_BACKUP_INTENT
| (oldpc.is_rep_symlink ()
? FILE_OPEN_REPARSE_POINT : 0));
if (!NT_SUCCESS (status))
if (NT_SUCCESS (status))
{
if (NT_TRANSACTIONAL_ERROR (status) && trans)
{
/* If NtOpenFile fails due to transactional problems,
stop transaction and go ahead without. */
status = unlink_nt (*dstpc);
if (NT_SUCCESS (status))
break;
}
if (!NT_TRANSACTIONAL_ERROR (status) || !trans)
break;
/* If NtOpenFile or unlink_nt fail due to transactional problems,
stop transaction and retry without. */
NtClose (fh);
stop_transaction (status, old_trans, trans);
debug_printf ("Transaction failure. Retry open.");
goto retry_reopen;
debug_printf ("Transaction failure %y. Retry open.", status);
}
__seterrno_from_nt_status (status);
__leave;
}
}
if (NT_SUCCESS (status = unlink_nt (*dstpc)))
if (NT_SUCCESS (status))
status = NtSetInformationFile (fh, &io, pfri,
sizeof *pfri + pfri->FileNameLength,
FileRenameInformation);