[PATCH] D144172: [Support] [Windows] Don't check file access time in equivalent(file_status, file_status)

Hans Wennborg via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 16 05:35:51 PST 2023


hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

In D144172#4131724 <https://reviews.llvm.org/D144172#4131724>, @mstorsjo wrote:

> In D144172#4131662 <https://reviews.llvm.org/D144172#4131662>, @hans wrote:
>
>> Does the same issue also affect the LastWriteTime fields? I.e., if someone `touch`es a file between calls?
>
> I guess it could do... But that's also probably a quite fuzzy distinction to try to make; if a file is touched, it's clearly still the same file. But if it is rewritten (but with the same contents) - is it still the same? That probably depends on whether whoever writes it opens/truncates the existing file or writes a new one, replacing the old one (possibly with an atomic rename). In the latter cases, it's clearly no longer the same file.
>
>> From https://learn.microsoft.com/en-us/windows/win32/api/fileapi/ns-fileapi-by_handle_file_information it sounds like just comparing the LastWriteTime and VolumeSerialNumber should be enough to check file identities. Perhaps we should just do that?
>
> You mean `FileIndex`, not `LastWriteTime`? I guess that could be reasonable - but that also feels like a more risky change.

Oops, yes I meant `FileIndex`.

> In that case I'd prefer to do it in two steps; first this, to get rid of the known-wrong reliance on last access (which should be quite safe), then secondly stripping out the other checks that are documented to not be necessary (which also probably is fine, but which is a bigger change).

Sounds good to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144172/new/

https://reviews.llvm.org/D144172



More information about the llvm-commits mailing list