[PATCH] D14231: [Support] Use GetTempDir to get the temporary dir path on Windows.

[Paweł Bylica via llvm-commits]

chfast added inline comments.

================
Comment at: unittests/Support/Path.cpp:366
@@ +365,3 @@
+
+  _wputenv_s(L"TMP", L"C:\\OtherFolder");
+  path::system_temp_directory(true, TempDir);
----------------
aaron.ballman wrote:
> I am slightly worried about this for systems that don't mount a C drive, but I suspect those are passingly rare. ;-)
> 
> What has me way more worried is that failing these tests will not restore the TMP (and TEMP, below) environment variables. So if any of these tests fail, we leave the user's machine in a broken state. This can be solved with some RAII wrappers.
> 
> Further, changing the temp directory out from under the user affects *every* application on their machine. This means there's a non-zero chance that temp files start going to incorrect locations for other applications. I don't think there's a way to solve this, and I'm really not comfortable with that because it can lead to hard-to-trace problems for developers. Ideas most certainly welcome.
https://msdn.microsoft.com/en-us/library/83zh4e6k.aspx
> _putenv and _wputenv affect only the environment that is local to the current process; you cannot use them to modify the command-level environment

So, changing env vars can affect unit tests running in other threads. RAII for restoring original values is worth considering.

================
Comment at: unittests/Support/Path.cpp:386
@@ +385,3 @@
+  _putenv_s("TMP", Expected.c_str());
+  path::system_temp_directory(true, TempDir);
+  EXPECT_EQ(Expected, TempDir);
----------------
Here is the test that checks system_temp_directory() in case $TMP is 260 chars long.


Repository:
  rL LLVM

http://reviews.llvm.org/D14231