[libcxx-commits] [PATCH] D137131: [libc++] Android temp dir is /data/local/tmp, enable Windows test

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added inline comments.


================
Comment at: libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.temp_dir_path/temp_directory_path.pass.cpp:144
+        TEST_CHECK(win_dir_sz > 0 && win_dir_sz < MAX_PATH);
+        TEST_CHECK(win_dir[win_dir_sz-1] != L'\\');
+        TEST_CHECK(ret == win_dir);
----------------
rprichard wrote:
> mstorsjo wrote:
> > I don't quite see why this specific line is needed here (I do kinda get what it hints at, based on the docs for `GetWindowsDirectoryW` though), but this should be effectively covered by the `ret == win_dir` check anyway, right? (I.e. the purpose of this test is to test the stdlib's temp path generation, not check `GetWindowsDirectoryW`.
> I suppose I was trying to verify that ret and win_dir both do not end with a backslash. I can remove this line and rely on the MSDN docs if you'd like.
> 
> Aside, the MSDN docs contradict themselves in the (hypothetical) case that the Windows directory is the root:
> 
> https://learn.microsoft.com/en-us/windows/win32/api/sysinfoapi/nf-sysinfoapi-getwindowsdirectoryw:
> > This path does not end with a backslash unless the Windows directory is the root directory. For example, if the Windows directory is named Windows on drive C, the path of the Windows directory retrieved by this function is C:\Windows. If the system was installed in the root directory of drive C, the path retrieved is C:.
> 
> Realistically, I think that never happens?
> 
Ah, now I see the angle where you're coming from, that it'd be good to verify that the returned path has a specific form.

With that in mind, I guess it's fine to keep the test - it probably doesn't matter much either way.

Yeah I noted that contradiction in the docs - I would believe it's a simple typo and just was meant to say "If the system was installed in the root directory of drive C, the path retrieved is C:\."



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137131