rename2 check for existing file
Ken Brown
kbrown@cornell.edu
Wed Jan 9 20:37:00 GMT 2019
Hi Corinna,
On 1/9/2019 9:52 AM, Corinna Vinschen wrote:
> Hi Ken,
>
> your patch to support renameat2(..., RENAME_NOREPLACE),
> commit f665b1cef30f9032877081ac63ea94910825be6a, also
> introduced a new check
>
> + /* Should we replace an existing file? */
> + if ((removepc || dstpc->exists ()) && noreplace)
> + {
> + set_errno (EEXIST);
> + __leave;
> + }
>
> However, the noreplace flag also adds the same check to the actual
> NtSetInformationFile call to rename the file:
>
> - pfri->ReplaceIfExists = TRUE;
> + pfri->ReplaceIfExists = !noreplace;
>
> So, isn't the first check redundant? Can't we just drop it? The
> rename2 function already has so many checks to perform before actually
> calling NtSetInformationFile, every check we can remove is a boon, I
> think.
My recollection is that that we discussed this at the time and decided that
there was a border case where the check was still needed in order to make sure
that EEXIST was set. I'll have to look back at the email thread and see if I
can reconstruct the reason. I may not have a chance to do that until tomorrow.
Ken
More information about the Cygwin-developers
mailing list