[libcxx-commits] [PATCH] D98960: [libcxx] [test] Allow C:\System Volume Information to be missing

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added inline comments.


================
Comment at: libcxx/test/support/filesystem_test_helper.h:691-698
+    // Check that it indeed is inaccessible as expected
+    (void)fs::exists(ent, ec);
+    if (!ec) {
+      fprintf(stderr, "The expected inaccessible directory \"%s\" was found "
+                      "but seems to be accessible, skipping tests "
+                      "regarding it\n", dir.string().c_str());
+      return fs::path();
----------------
Quuxplusone wrote:
> This codepath actually confuses me. It's new (appears on the right-hand side but not the left-hand side of the diff), and it seems to be checking that `ent.exists()` (line 685) and yet `!fs::exists(ent)` (line 692)? This could well be consistent with reality — I mean, I'd expect std::filesystem to do a terrible/weird job of handling visible-but-inaccessible paths — but could you please just manually triple-check that `GetWindowsInaccessibleDir()` does still return a non-empty path //sometimes//, and hasn't been completely nerfed by this patch?
Yep, it does return a non-empty path on regular desktop windows, just re-checked it.

This check is new here, but it's essentially the same check as we had in the fs.op.exists test already; if the directory doesn't fulfill that, we can't use it as intended.

When the tested process is launched from within bash, running on windows server as the administrator user, the directory is fully accessible (for unknown reasons), this case is caught by this clause.

It's a bit non-obvious, but yes, `ent.exists()` is true - when iterating over the parent directory, it does return an entry for the special directory, and the status flags we get from `FindNextFile` (used for iterating a directory) preliminarily indicate that it exists. Checking with `fs::exists()` then instead does the equivalent of `stat()` - in this case opening a handle with `CreateFile(FILE_READ_ATTRIBUTES)` and inspecting it with `GetFileInformationByHandleEx()` - and that fails as we're not allowed to read it (i.e. `fs::exists()` sets the error code or throws an exception, it doesn't signal a clean negative).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98960