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

[Patrick Steinhardt <ps@pks.im>]

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