From a7301db9f4f7e62e21187ec36bbdae8ba68dcf52 Mon Sep 17 00:00:00 2001 From: Povilas Kanapickas Date: Sat, 19 Dec 2020 22:45:42 +0200 Subject: Remove variadic argument overloads of Process::Start() --- gtest/include/xorg/gtest/xorg-gtest-process.h | 38 ----------------------- gtest/src/process.cpp | 22 -------------- gtest/test/process-test.cpp | 44 +++++++++++++-------------- tests/server/xephyr.cpp | 4 +-- 4 files changed, 24 insertions(+), 84 deletions(-) diff --git a/gtest/include/xorg/gtest/xorg-gtest-process.h b/gtest/include/xorg/gtest/xorg-gtest-process.h index b888d46..a15cab9 100644 --- a/gtest/include/xorg/gtest/xorg-gtest-process.h +++ b/gtest/include/xorg/gtest/xorg-gtest-process.h @@ -152,44 +152,6 @@ class Process { */ void Start(const std::string& program, const std::vector &args); - /** - * Starts a program as a child process. - * - * See 'man execvp' for further information on the variadic argument list. - * - * If Fork() was called previously, Start() may only be called on the child - * process. - * - * @param program The program to start. - * @param args Variadic list of arguments passed to the program. This list - * must end with NULL. - * - * @throws std::runtime_error on failure. - * - * @post If successful: Child process forked and program started. - * @post If successful: Subsequent calls to Pid() return child process pid. - */ - void Start(const std::string& program, va_list args); - - /** - * Starts a program as a child process. - * - * Takes a variadic list of arguments passed to the program. This list - * must end with NULL. - * See 'man execvp' for further information on the variadic argument list. - * - * If Fork() was called previously, Start() may only be called on the child - * process. - * - * @param program The program to start. - * - * @throws std::runtime_error on failure. - * - * @post If successful: Child process forked and program started. - * @post If successful: Subsequent calls to Pid() return child process pid. - */ - void Start(const std::string& program, ...) _X_SENTINEL(0); - /** * Terminates (SIGTERM) this child process and waits a given timeout for * the process to terminate. diff --git a/gtest/src/process.cpp b/gtest/src/process.cpp index ff51b5d..fc74448 100644 --- a/gtest/src/process.cpp +++ b/gtest/src/process.cpp @@ -127,28 +127,6 @@ void xorg::testing::Process::Start(const std::string &program, const std::vector d_->state = RUNNING; } -void xorg::testing::Process::Start(const std::string& program, va_list args) { - std::vector argv; - - if (args) { - char *arg; - do { - arg = va_arg(args, char*); - if (arg) - argv.push_back(std::string(arg)); - } while (arg); - } - - Start(program, argv); -} - -void xorg::testing::Process::Start(const std::string& program, ...) { - va_list list; - va_start(list, program); - Start(program, list); - va_end(list); /* Shouldn't get here */ -} - bool xorg::testing::Process::WaitForExit(unsigned int timeout) { sigset_t sig_mask, old_mask; sigemptyset(&sig_mask); diff --git a/gtest/test/process-test.cpp b/gtest/test/process-test.cpp index e71934a..06610ad 100644 --- a/gtest/test/process-test.cpp +++ b/gtest/test/process-test.cpp @@ -17,7 +17,7 @@ TEST(Process, StartWithNULLArg) { XORG_TESTCASE("invocation of 'ls' with no arguments"); Process p; - p.Start("ls", nullptr); + p.Start("ls", {}); ASSERT_GT(p.Pid(), 0); } @@ -26,7 +26,7 @@ TEST(Process, StartWithNULLTerminatedArg) XORG_TESTCASE("invocation of 'ls' with NULL-terminated argument list"); Process p; - p.Start("ls", "-l", nullptr); + p.Start("ls", {"-l"}); ASSERT_GT(p.Pid(), 0); } @@ -38,7 +38,7 @@ TEST(Process, ExitCodeSuccess) ASSERT_EQ(p.GetState(), Process::NONE); /* Process:Start closes stdout, so we need something that doesn't print */ - p.Start("echo", "-n", nullptr); + p.Start("echo", {"-n"}); ASSERT_GT(p.Pid(), 0); ASSERT_EQ(p.GetState(), Process::RUNNING); @@ -58,7 +58,7 @@ TEST(Process, ExitCodeFailure) /* Process:Start closes stdout, so ls should fail with status 2, if not, * that file is unlikely to exists so we get status 1 */ - p.Start("ls", "asqwerq.aqerqw_rqwe", nullptr); + p.Start("ls", {"asqwerq.aqerqw_rqwe"}); ASSERT_GT(p.Pid(), 0); ASSERT_EQ(p.GetState(), Process::RUNNING); @@ -86,7 +86,7 @@ TEST(Process, ChildTearDown) close(pipefd[0]); Process p; - p.Start("sleep", "1000", nullptr); /* forks another child */ + p.Start("sleep", {"1000"}); /* forks another child */ ASSERT_GT(p.Pid(), 0); char *buffer; @@ -125,7 +125,7 @@ TEST(Process, TerminationFailure) sigaddset(&sig_mask, SIGUSR1); Process p; - p.Start(TEST_BUILD_DIR "process-test-helper", nullptr); + p.Start(TEST_BUILD_DIR "process-test-helper", {}); /* don't check error here, the helper may have sent the signal before we get here */ sigtimedwait(&sig_mask, nullptr, &sig_timeout); @@ -143,7 +143,7 @@ TEST(Process, KillExitStatus) XORG_TESTCASE("a child process killed must have a state of\n" "FINISHED_FAILURE"); Process p; - p.Start(TEST_BUILD_DIR "process-test-helper", nullptr); + p.Start(TEST_BUILD_DIR "process-test-helper", {}); p.Kill(1000); ASSERT_EQ(p.GetState(), Process::FINISHED_FAILURE); } @@ -157,9 +157,9 @@ TEST(Process, DoubleStart) /* Process double-started must fail */ Process p; - p.Start("echo", "-n", nullptr); + p.Start("echo", {"-n"}); try { - p.Start("echo", "-n", nullptr); + p.Start("echo", {"-n"}); FAIL() << "Expected exception"; } catch (std::runtime_error &e) { } @@ -175,7 +175,7 @@ TEST(Process, DoubleStart) /* restart job after a failed one, must succeed */ try { - p.Start("echo", "-n", nullptr); + p.Start("echo", {"-n"}); } catch (std::runtime_error &e) { FAIL(); } @@ -185,7 +185,7 @@ TEST(Process, DoubleStart) /* restart job after successful one, must succeed */ try { - p.Start("echo", "-n", nullptr); + p.Start("echo", {"-n"}); } catch (std::runtime_error &e) { FAIL(); } @@ -195,7 +195,7 @@ TEST(Process, DoubleStart) /* job that must be killed, followed by job */ sigemptyset(&sig_mask); sigaddset(&sig_mask, SIGUSR1); - p.Start(TEST_BUILD_DIR "process-test-helper", nullptr); + p.Start(TEST_BUILD_DIR "process-test-helper", {}); sigtimedwait(&sig_mask, nullptr, &sig_timeout); ASSERT_EQ(p.GetState(), Process::RUNNING); p.Kill(100); @@ -203,7 +203,7 @@ TEST(Process, DoubleStart) /* restart job after successful one, must succeed */ try { - p.Start("echo", "-n", nullptr); + p.Start("echo", {"-n"}); } catch (std::runtime_error &e) { FAIL(); } @@ -215,13 +215,13 @@ TEST(Process, DoubleStart) /* job that fails to terminate, starting another one must fail */ sigemptyset(&sig_mask); sigaddset(&sig_mask, SIGUSR1); - p.Start(TEST_BUILD_DIR "process-test-helper", nullptr); + p.Start(TEST_BUILD_DIR "process-test-helper", {}); sigtimedwait(&sig_mask, nullptr, &sig_timeout); ASSERT_EQ(p.GetState(), Process::RUNNING); ASSERT_FALSE(p.Terminate(100)); try { - p.Start("echo", "-n", nullptr); + p.Start("echo", {"-n"}); FAIL() << "exception expected"; } catch (std::runtime_error &e) { } @@ -239,7 +239,7 @@ TEST(Process, ForkedParentStart) if (p.Fork() > 0) { ASSERT_GT(p.Pid(), 0); ASSERT_EQ(p.GetState(), Process::RUNNING); - ASSERT_THROW({ p.Start("ls", nullptr); }, std::runtime_error); + ASSERT_THROW({ p.Start("ls", {}); }, std::runtime_error); } } @@ -249,7 +249,7 @@ TEST(Process, ForkedChildStart) Process p; if (p.Fork() == 0) { ASSERT_EQ(p.GetState(), Process::RUNNING); - p.Start("ls", nullptr); + p.Start("ls", {}); ASSERT_GT(p.Pid(), 0); } } @@ -260,9 +260,9 @@ TEST(Process, ForkedChildDoubleStart) Process p; if (p.Fork() == 0) { ASSERT_EQ(p.GetState(), Process::RUNNING); - p.Start("ls", nullptr); + p.Start("ls", {}); ASSERT_THROW({ - p.Start("ls", nullptr); + p.Start("ls", {}); }, std::runtime_error); } } @@ -278,7 +278,7 @@ public: Process valgrind; /* check if valgrind actually exists */ - valgrind.Start("valgrind", "--version", nullptr); + valgrind.Start("valgrind", {"--version"}); int status; ASSERT_EQ(waitpid(valgrind.Pid(), &status, 0), valgrind.Pid()); ASSERT_TRUE(WIFEXITED(status)); @@ -295,7 +295,7 @@ TEST_P(ProcessValgrindWrapper, ValgrindWrapper) /* now set the env and fire up valgrind */ setenv("XORG_GTEST_USE_VALGRIND", executable.c_str(), 1); Process p; - p.Start("ls", nullptr); + p.Start("ls", {}); unsetenv("XORG_GTEST_USE_VALGRIND"); /* Check /proc//comm to make sure valgrind @@ -338,7 +338,7 @@ TEST_P(ProcessValgrindArgsWrapper, ValgrindWrapperWithArgs) /* now set the env and fire up valgrind */ setenv("XORG_GTEST_USE_VALGRIND", vargs.c_str(), 1); Process p; - p.Start(TEST_BUILD_DIR "process-test-helper", nullptr); + p.Start(TEST_BUILD_DIR "process-test-helper", {}); unsetenv("XORG_GTEST_USE_VALGRIND"); ASSERT_EQ(p.GetState(), Process::RUNNING); diff --git a/tests/server/xephyr.cpp b/tests/server/xephyr.cpp index 5def28d..639ed28 100644 --- a/tests/server/xephyr.cpp +++ b/tests/server/xephyr.cpp @@ -74,7 +74,7 @@ TEST_F(Xephyr24bppTest, CrashOn24bppHost) Process xephyr; setenv("DISPLAY", server.GetDisplayString().c_str(), 1); - xephyr.Start("Xephyr", ":134", NULL); + xephyr.Start("Xephyr", {":134"}); ASSERT_GT(xephyr.Pid(), 0); ASSERT_EQ(xephyr.GetState(), xorg::testing::Process::RUNNING); @@ -111,7 +111,7 @@ TEST_F(Xephyr8bppTest, No8bppCrash) Process xephyr; setenv("DISPLAY", server.GetDisplayString().c_str(), 1); - xephyr.Start("Xephyr", "-screen", "320x220x8", ":134", NULL); + xephyr.Start("Xephyr", {"-screen", "320x220x8", ":134"}); ASSERT_GT(xephyr.Pid(), 0); ASSERT_EQ(xephyr.GetState(), xorg::testing::Process::RUNNING); -- cgit v1.2.3