Remove variadic argument overloads of Process::Start()

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
@@ -153,44 +153,6 @@ class Process {
   void Start(const std::string& program, const std::vector<std::string> &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<std::string> 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/<pid>/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);