[libcxx-commits] [PATCH] D98166: [libcxx] Test accessing a directory on windows that gives "access denied" errors

Mon Mar 8 04:51:52 PST 2021

mstorsjo added a comment.

In D98166#2610860 <https://reviews.llvm.org/D98166#2610860>, @curdeius wrote:

> We certainly need to factor it out into some helper in `support/filesystem_test_helper.h` as you suggested.
> Also, I'm not really at ease with:
>
>   if (!found)
>     return;
>
> I think that we should check it as a precondition of a test. Just returning if the dir is not found may lead to some stupid errors and dead tests.
> Is there any case where this path would be inaccessible? MinGW? I mean apart from having a very old OS or no C: partition?

I guess there could be odd installation setups where it isn't available, but as you say, it seems to be available on most setups, so it might be best to just require that it is, and clearly safer against silently missing testcases. And if someone actually runs these tests on windows and fails due to this, they can of course speak up.

================
Comment at: libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.exists/exists.pass.cpp:84
+#else
+    // Actually a directory, not a file
+    const path file("C:\\System Volume Information");
----------------
curdeius wrote:
> FYI. If you really need a file instead of a directory, there are system files in C:\ (usually, but they can be moved): `hiberfil.sys`, `pagefile.sys`, `swapfile.sys`.
Yeah. For this case it doesn't matter if it's a file or directory, but perhaps it's clearer to rename file to path here.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98166