rename2 check for existing file

[Ken Brown]

On 1/9/2019 3:37 PM, Ken Brown wrote:
> 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 think there were three cases in which the check was needed:

   - removepc is non-NULL
   - dstpc points to an existing directory
   - dstpc points to an existing file with FILE_ATTRIBUTE_READONLY.

But now that you've introduced use_posix_semantics, maybe the check is only 
needed in the pre-W10 case.

Ken