From 790e8d91fffa01c1ae5a72d31615f20d78f20404 Mon Sep 17 00:00:00 2001 From: Pawel Bylica Date: Tue, 17 Nov 2015 16:54:32 +0000 Subject: [PATCH 1/1] [Support] Tweak path::system_temp_directory() on Windows. Summary: This patch changes the behavior of path::system_temp_directory() on Windows to be closer to GetTempPath Windows API call. Enforces path separator to be the native one, makes path absolute, etc. GetTempPath is not used directly because of limitations/implementation bugs on Windows 7. Windows specific unit tests are added. Most of them runs in separated process with modified environment variables. This change fixes FileSystemTest.CreateDir unittest that had been failing when run from Unix-like shell on Windows (Unix-like path separator (/) used in env variables). Reviewers: chapuni, rafael, aaron.ballman Subscribers: rafael, llvm-commits Differential Revision: http://reviews.llvm.org/D14231 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@253345 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Support/Windows/Path.inc | 31 ++++++++-------- unittests/Support/Path.cpp | 70 ++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 16 deletions(-) diff --git a/lib/Support/Windows/Path.inc b/lib/Support/Windows/Path.inc index 49910af55a7..4e4841231ef 100644 --- a/lib/Support/Windows/Path.inc +++ b/lib/Support/Windows/Path.inc @@ -773,17 +773,12 @@ bool home_directory(SmallVectorImpl &result) { return getKnownFolderPath(FOLDERID_Profile, result); } -static bool getTempDirEnvVar(const char *Var, SmallVectorImpl &Res) { - SmallVector NameUTF16; - if (windows::UTF8ToUTF16(Var, NameUTF16)) - return false; - +static bool getTempDirEnvVar(const wchar_t *Var, SmallVectorImpl &Res) { SmallVector Buf; size_t Size = 1024; do { Buf.reserve(Size); - Size = - GetEnvironmentVariableW(NameUTF16.data(), Buf.data(), Buf.capacity()); + Size = GetEnvironmentVariableW(Var, Buf.data(), Buf.capacity()); if (Size == 0) return false; @@ -791,14 +786,12 @@ static bool getTempDirEnvVar(const char *Var, SmallVectorImpl &Res) { } while (Size > Buf.capacity()); Buf.set_size(Size); - if (windows::UTF16ToUTF8(Buf.data(), Size, Res)) - return false; - return true; + return !windows::UTF16ToUTF8(Buf.data(), Size, Res); } static bool getTempDirEnvVar(SmallVectorImpl &Res) { - const char *EnvironmentVariables[] = {"TMP", "TEMP", "USERPROFILE"}; - for (const char *Env : EnvironmentVariables) { + const wchar_t *EnvironmentVariables[] = {L"TMP", L"TEMP", L"USERPROFILE"}; + for (auto *Env : EnvironmentVariables) { if (getTempDirEnvVar(Env, Res)) return true; } @@ -809,13 +802,19 @@ void system_temp_directory(bool ErasedOnReboot, SmallVectorImpl &Result) { (void)ErasedOnReboot; Result.clear(); - // Check whether the temporary directory is specified by an environment - // variable. - if (getTempDirEnvVar(Result)) + // Check whether the temporary directory is specified by an environment var. + // This matches GetTempPath logic to some degree. GetTempPath is not used + // directly as it cannot handle evn var longer than 130 chars on Windows 7 + // (fixed on Windows 8). + if (getTempDirEnvVar(Result)) { + assert(!Result.empty() && "Unexpected empty path"); + native(Result); // Some Unix-like shells use Unix path separator in $TMP. + fs::make_absolute(Result); // Make it absolute if not already. return; + } // Fall back to a system default. - const char *DefaultResult = "C:\\TEMP"; + const char *DefaultResult = "C:\\Temp"; Result.append(DefaultResult, DefaultResult + strlen(DefaultResult)); } } // end namespace path diff --git a/unittests/Support/Path.cpp b/unittests/Support/Path.cpp index 9b7cd2fbdbb..e325f9a1c65 100644 --- a/unittests/Support/Path.cpp +++ b/unittests/Support/Path.cpp @@ -351,6 +351,76 @@ TEST(Support, UserCacheDirectory) { } } +TEST(Support, TempDirectory) { + SmallString<32> TempDir; + path::system_temp_directory(false, TempDir); + EXPECT_TRUE(!TempDir.empty()); + TempDir.clear(); + path::system_temp_directory(true, TempDir); + EXPECT_TRUE(!TempDir.empty()); +} + +static std::string path2regex(std::string Path) { + size_t Pos = 0; + while ((Pos = Path.find('\\', Pos)) != std::string::npos) { + Path.replace(Pos, 1, "\\\\"); + Pos += 2; + } + return Path; +} + +/// Helper for running temp dir test in separated process. See below. +#define EXPECT_TEMP_DIR(prepare, expected) \ + EXPECT_EXIT( \ + { \ + prepare; \ + SmallString<300> TempDir; \ + path::system_temp_directory(true, TempDir); \ + raw_os_ostream(std::cerr) << TempDir; \ + std::exit(0); \ + }, \ + ::testing::ExitedWithCode(0), path2regex(expected)) + +#ifdef LLVM_ON_WIN32 +TEST(SupportDeathTest, TempDirectoryOnWindows) { + // In this test we want to check how system_temp_directory responds to + // different values of specific env vars. To prevent corrupting env vars of + // the current process all checks are done in separated processes. + EXPECT_TEMP_DIR(_wputenv_s(L"TMP", L"C:\\OtherFolder"), "C:\\OtherFolder"); + EXPECT_TEMP_DIR(_wputenv_s(L"TMP", L"C:/Unix/Path/Seperators"), + "C:\\Unix\\Path\\Seperators"); + EXPECT_TEMP_DIR(_wputenv_s(L"TMP", L"Local Path"), ".+\\Local Path$"); + EXPECT_TEMP_DIR(_wputenv_s(L"TMP", L"F:\\TrailingSep\\"), "F:\\TrailingSep"); + EXPECT_TEMP_DIR( + _wputenv_s(L"TMP", L"C:\\2\x03C0r-\x00B5\x00B3\\\x2135\x2080"), + "C:\\2\xCF\x80r-\xC2\xB5\xC2\xB3\\\xE2\x84\xB5\xE2\x82\x80"); + + // Test $TMP empty, $TEMP set. + EXPECT_TEMP_DIR( + { + _wputenv_s(L"TMP", L""); + _wputenv_s(L"TEMP", L"C:\\Valid\\Path"); + }, + "C:\\Valid\\Path"); + + // All related env vars empty + EXPECT_TEMP_DIR( + { + _wputenv_s(L"TMP", L""); + _wputenv_s(L"TEMP", L""); + _wputenv_s(L"USERPROFILE", L""); + }, + "C:\\Temp"); + + // Test evn var / path with 260 chars. + SmallString<270> Expected{"C:\\Temp\\AB\\123456789"}; + while (Expected.size() < 260) + Expected.append("\\DirNameWith19Charss"); + ASSERT_EQ(260, Expected.size()); + EXPECT_TEMP_DIR(_putenv_s("TMP", Expected.c_str()), Expected.c_str()); +} +#endif + class FileSystemTest : public testing::Test { protected: /// Unique temporary directory in which all created filesystem entities must -- 2.34.1