From 08426e1f9f76f137d6958a450b1b5febed9b2ff2 Mon Sep 17 00:00:00 2001 From: Zachary Turner Date: Tue, 12 Jun 2018 17:43:52 +0000 Subject: Refactor ExecuteAndWait to take StringRefs. This simplifies some code which had StringRefs to begin with, and makes other code more complicated which had const char* to begin with. In the end, I think this makes for a more idiomatic and platform agnostic API. Not all platforms launch process with null terminated c-string arrays for the environment pointer and argv, but the api was designed that way because it allowed easy pass-through for posix-based platforms. There's a little additional overhead now since on posix based platforms we'll be takign StringRefs which were constructed from null terminated strings and then copying them to null terminate them again, but from a readability and usability standpoint of the API user, I think this API signature is strictly better. llvm-svn: 334518 --- llvm/unittests/Support/ProgramTest.cpp | 60 +++++++++++++--------------------- 1 file changed, 22 insertions(+), 38 deletions(-) (limited to 'llvm/unittests/Support/ProgramTest.cpp') diff --git a/llvm/unittests/Support/ProgramTest.cpp b/llvm/unittests/Support/ProgramTest.cpp index 7a0676c7a48..ec1c85a9f25 100644 --- a/llvm/unittests/Support/ProgramTest.cpp +++ b/llvm/unittests/Support/ProgramTest.cpp @@ -60,7 +60,7 @@ static cl::opt ProgramTestStringArg2("program-test-string-arg2"); class ProgramEnvTest : public testing::Test { - std::vector EnvTable; + std::vector EnvTable; std::vector EnvStorage; protected: @@ -77,7 +77,7 @@ protected: }(); ASSERT_TRUE(EnvP); - auto prepareEnvVar = [this](decltype(*EnvP) Var) { + auto prepareEnvVar = [this](decltype(*EnvP) Var) -> StringRef { #if defined(_WIN32) // On Windows convert UTF16 encoded variable to UTF8 auto Len = wcslen(Var); @@ -86,10 +86,10 @@ protected: EnvStorage.emplace_back(); auto convStatus = convertUTF16ToUTF8String(Ref, EnvStorage.back()); EXPECT_TRUE(convStatus); - return EnvStorage.back().c_str(); + return EnvStorage.back(); #else (void)this; - return Var; + return StringRef(Var); #endif }; @@ -104,16 +104,9 @@ protected: EnvStorage.clear(); } - void addEnvVar(const char *Var) { - ASSERT_TRUE(EnvTable.empty() || EnvTable.back()) << "Env table sealed"; - EnvTable.emplace_back(Var); - } + void addEnvVar(StringRef Var) { EnvTable.emplace_back(Var); } - const char **getEnviron() { - if (EnvTable.back() != nullptr) - EnvTable.emplace_back(nullptr); // Seal table. - return &EnvTable[0]; - } + ArrayRef getEnviron() const { return EnvTable; } }; #ifdef _WIN32 @@ -129,11 +122,8 @@ TEST_F(ProgramEnvTest, CreateProcessLongPath) { MyExe.append("\\\\?\\"); MyExe.append(MyAbsExe); - const char *ArgV[] = { - MyExe.c_str(), - "--gtest_filter=ProgramEnvTest.CreateProcessLongPath", - nullptr - }; + StringRef ArgV[] = {MyExe, + "--gtest_filter=ProgramEnvTest.CreateProcessLongPath"}; // Add LLVM_PROGRAM_TEST_LONG_PATH to the environment of the child. addEnvVar("LLVM_PROGRAM_TEST_LONG_PATH=1"); @@ -173,13 +163,13 @@ TEST_F(ProgramEnvTest, CreateProcessTrailingSlash) { std::string my_exe = sys::fs::getMainExecutable(TestMainArgv0, &ProgramTestStringArg1); - const char *argv[] = { - my_exe.c_str(), - "--gtest_filter=ProgramEnvTest.CreateProcessTrailingSlash", - "-program-test-string-arg1", "has\\\\ trailing\\", - "-program-test-string-arg2", "has\\\\ trailing\\", - nullptr - }; + StringRef argv[] = { + my_exe, + "--gtest_filter=ProgramEnvTest.CreateProcessTrailingSlash", + "-program-test-string-arg1", + "has\\\\ trailing\\", + "-program-test-string-arg2", + "has\\\\ trailing\\"}; // Add LLVM_PROGRAM_TEST_CHILD to the environment of the child. addEnvVar("LLVM_PROGRAM_TEST_CHILD=1"); @@ -210,11 +200,8 @@ TEST_F(ProgramEnvTest, TestExecuteNoWait) { std::string Executable = sys::fs::getMainExecutable(TestMainArgv0, &ProgramTestStringArg1); - const char *argv[] = { - Executable.c_str(), - "--gtest_filter=ProgramEnvTest.TestExecuteNoWait", - nullptr - }; + StringRef argv[] = {Executable, + "--gtest_filter=ProgramEnvTest.TestExecuteNoWait"}; // Add LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT to the environment of the child. addEnvVar("LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT=1"); @@ -268,11 +255,8 @@ TEST_F(ProgramEnvTest, TestExecuteAndWaitTimeout) { std::string Executable = sys::fs::getMainExecutable(TestMainArgv0, &ProgramTestStringArg1); - const char *argv[] = { - Executable.c_str(), - "--gtest_filter=ProgramEnvTest.TestExecuteAndWaitTimeout", - nullptr - }; + StringRef argv[] = { + Executable, "--gtest_filter=ProgramEnvTest.TestExecuteAndWaitTimeout"}; // Add LLVM_PROGRAM_TEST_TIMEOUT to the environment of the child. addEnvVar("LLVM_PROGRAM_TEST_TIMEOUT=1"); @@ -287,12 +271,12 @@ TEST_F(ProgramEnvTest, TestExecuteAndWaitTimeout) { TEST(ProgramTest, TestExecuteNegative) { std::string Executable = "i_dont_exist"; - const char *argv[] = { Executable.c_str(), nullptr }; + StringRef argv[] = {Executable}; { std::string Error; bool ExecutionFailed; - int RetCode = ExecuteAndWait(Executable, argv, nullptr, {}, 0, 0, &Error, + int RetCode = ExecuteAndWait(Executable, argv, llvm::None, {}, 0, 0, &Error, &ExecutionFailed); ASSERT_TRUE(RetCode < 0) << "On error ExecuteAndWait should return 0 or " "positive value indicating the result code"; @@ -303,7 +287,7 @@ TEST(ProgramTest, TestExecuteNegative) { { std::string Error; bool ExecutionFailed; - ProcessInfo PI = ExecuteNoWait(Executable, argv, nullptr, {}, 0, &Error, + ProcessInfo PI = ExecuteNoWait(Executable, argv, llvm::None, {}, 0, &Error, &ExecutionFailed); ASSERT_EQ(PI.Pid, ProcessInfo::InvalidPid) << "On error ExecuteNoWait should return an invalid ProcessInfo"; -- cgit v1.2.3