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