Re: [PATCH v2 3/3] compat/mingw: support POSIX semantics for atomic renames

[Johannes Sixt <j6t@kdbg.org>]

Am 27.10.24 um 18:27 schrieb Patrick Steinhardt:
> On Sun, Oct 27, 2024 at 05:31:00PM +0100, Johannes Sixt wrote:
>> Am 27.10.24 um 16:38 schrieb Patrick Steinhardt:
>>> On Sun, Oct 27, 2024 at 02:23:28PM +0100, Johannes Sixt wrote:
>>>> Am 24.10.24 um 13:46 schrieb Patrick Steinhardt:
>>>>> Windows 10 has introduced the `FILE_RENAME_FLAG_POSIX_SEMANTICS` flag
>>>>> that allows us to fix this usecase [1]. When set, it is possible to
>>>>> rename a file over a preexisting file even when the target file still
>>>>> has handles open. Those handles must have been opened with the
>>>>> `FILE_SHARE_DELETE` flag, which we have ensured in the preceding
>>>>> commits.
>>>>>> Careful readers might have noticed that [1] does not mention the above
>>>>> flag, but instead mentions `FILE_RENAME_POSIX_SEMANTICS`. This flag is
>>>>> not for use with `SetFileInformationByHandle()` though, which is what we
>>>>> use. And while the `FILE_RENAME_FLAG_POSIX_SEMANTICS` flag exists, it is
>>>>> not documented on [2] or anywhere else as far as I can tell.
>>>>
>>>> The Windows 10 SDK defines FILE_RENAME_FLAG_REPLACE_IF_EXISTS and
>>>> FILE_RENAME_FLAG_POSIX_SEMANTICS for SetFileInformationByHandle(). That
>>>> the documentation lacks "_FLAG_" in the names must be an error in the
>>>> documentation.
>>>>
>>>> I found the mention of FILE_RENAME_POSIX_SEMANTICS quite distracting,
>>>> because it is a flag to be used with CreateFileW() and basically only
>>>> has to do with case-sensitivity, but nothing with POSIX semantics of
>>>> renaming.
>>>
>>> I'd still prefer to mention this, because otherwise an astute reader
>>> might notice that I'm using a different flag name than what is
>>> documented in the docs and figure out that I defined the wrong flag
>>> name.
>>
>> Ah, I was confused twice here. First, the documentation that you cite[*]
>> mentions FILE_RENAME_POSIX_SEMANTICS, but the name does not exist at
>> all. There does exist FILE_RENAME_FLAG_POSIX_SEMANTICS. The
>> documentation is just wrong. And in my earlier comment I copied the
>> inexistent flag name.
>>
>> But I meant to cite this flag: FILE_FLAG_POSIX_SEMANTICS (no "RENAME").
>> It exists and is for CreateFileW().
>>
>> Perhaps you also meant cite the latter one as the flag that "is not for
>> use with `SetFileInformationByHandle()`"?
>>
>> At any rate, the paragraph as written isn't correct.
> 
> I think I'm missing something. That's what the paragraph says:
> 
>     Careful readers might have noticed that [1] does not mention the above
>     flag, but instead mentions `FILE_RENAME_POSIX_SEMANTICS`. This flag is
>     not for use with `SetFileInformationByHandle()` though, which is what we
>     use. And while the `FILE_RENAME_FLAG_POSIX_SEMANTICS` flag exists, it is
>     not documented on [2] or anywhere else as far as I can tell.
> 
> And I'd claim it is correct.
> 
> `FILE_RENAME_POSIX_SEMANTICS` exists, this it is not a documentation
> error. It is at a lower level than `FILE_RENAME_FLAG_POSIX_SEMANTICS`,
> the documentation at [1] refers to "ntifs.h", which is part of the
> Windows Driver Kit interfaces. So it is not supposed to be used with
> `SetFileInformationByHandle()`, but with `FtlSetInformationFile()` [2],
> which _also_ has a separate `FILE_RENAME_INFO` structure that looks the
> same as `FILE_RENAME_INFO` defined for `SetFileInformationByHandle()`.
> The only difference as far as I can tell is that the flags used for
> these structures have slightly different names.
> 
> Now I totally get your confusion -- I have been extremely confused by
> all of this, as well. It certainly is a documentation error that the
> respective `FILE_RENAME_FLAG_POSIX_SEMANTICS` is undocumented, but
> having proper docs for this is rather important such that the reader
> knows what its behaviour is. So I have no other choice than to link to
> the ntifs interfaces, as it documents the actual behaviour, even though
> it lives in a different part of the Windows APIs.
> > Patrick
> 
> [1]: https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/ntifs/ns-ntifs-_file_rename_information
> [2]: https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/fltkernel/nf-fltkernel-fltsetinformationfile

OK, then let's leave the text as it is.

Thanks,
-- Hannes