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

From: Patrick Steinhardt <ps@pks.im>
Date: Oct 27, 2024, 17:27 UTC
Message-ID: <Zx54B6FdGprtH0HV@pks.im>
In-Reply-To: <49619b52-0fea-4179-a829-7ec4a6945055@kdbg.org>

On Sun, Oct 27, 2024 at 05:31:00PM +0100, Johannes Sixt wrote:

Show 43 quoted lines

> 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