process: split Fork() and Start() into (optionally) two calls

Process::Start() will fork() and execvp() the child process. For use-cases
where the child process must have specific signal masks or other properties
before the execvp() call this is unfeasable, the caller cannot control the
properties of the child between forking and execvp.

Split the fork() call out into Process::Fork(), making it optional. Start()
will fork on demand if it hasn't been called before. Behaviour stays the
same for callers of Start(), only those that call Fork() first need to pay
attention to details.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Chase Douglas <chase.douglas@ubuntu.com>

3 files changed, 76 insertions, 3 deletions

diff --git a/include/xorg/gtest/xorg-gtest-process.h b/include/xorg/gtest/xorg-gtest-process.h
index 8c581db..30fe30b 100644
--- a/include/xorg/gtest/xorg-gtest-process.h
+++ b/include/xorg/gtest/xorg-gtest-process.h
@@ -112,11 +112,32 @@ class Process {
   Process();
 
   /**
+   * Fork manually. Usually, fork() is called as part of Start() but for
+   * use-cases where the parent process and the child process need special
+   * processing before the child is replaced by an execvp call Fork() may be
+   * called manually.
+   *
+   * A process may only be forked once.
+   *
+   * The state of both the parent and the child after a Fork() is
+   * Process::RUNNING. If Fork() is called directly, Start() may only be
+   * called on the child process.
+   *
+   * @throws std::runtime_error on failure.
+   *
+   * @return The pid of the child, or 0 for the child process.
+   */
+  pid_t Fork();
+
+  /**
    * Starts a program as a child process.
    *
    * See 'man execvp' for further information on the elements in
    * the vector.
    *
+   * If Fork() was called previously, Start() may only be called on the child
+   * process.
+   *
    * @param program The program to start.
    * @param args Vector of arguments passed to the program.
    *
@@ -132,6 +153,9 @@ class 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.
@@ -150,6 +174,9 @@ class Process {
    * 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.
diff --git a/src/process.cpp b/src/process.cpp
index 7f7d330..a743346 100644
--- a/src/process.cpp
+++ b/src/process.cpp
@@ -66,12 +66,11 @@ enum xorg::testing::Process::State xorg::testing::Process::GetState() {
   return d_->state;
 }
 
-void xorg::testing::Process::Start(const std::string &program, const std::vector<std::string> &argv) {
+pid_t xorg::testing::Process::Fork() {
   if (d_->pid != -1)
-    throw std::runtime_error("Attempting to start an already started process");
+    throw std::runtime_error("A process may only be forked once");
 
   d_->pid = fork();
-
   if (d_->pid == -1) {
     d_->state = ERROR;
     throw std::runtime_error("Failed to fork child process");
@@ -85,7 +84,19 @@ void xorg::testing::Process::Start(const std::string &program, const std::vector
 #ifdef __linux
     prctl(PR_SET_PDEATHSIG, SIGTERM);
 #endif
+  }
+
+  d_->state = RUNNING;
+  return d_->pid;
+}
+
+void xorg::testing::Process::Start(const std::string &program, const std::vector<std::string> &argv) {
+  if (d_->pid == -1)
+    d_->pid = Fork();
+  else if (d_->pid > 0)
+    throw std::runtime_error("Start() may only be called on the child process");
 
+  if (d_->pid == 0) { /* Child */
     std::vector<char*> args;
     std::vector<std::string>::const_iterator it;
 
diff --git a/test/process-test.cpp b/test/process-test.cpp
index 725663b..59f96c2 100644
--- a/test/process-test.cpp
+++ b/test/process-test.cpp
@@ -229,6 +229,41 @@ TEST(Process, DoubleStart)
   sigprocmask(SIG_UNBLOCK, &sig_mask, 0);
 }
 
+TEST(Process, ForkedParentStart)
+{
+  XORG_TESTCASE("Fork() and calling Start() on the parent causes an exception");
+  Process p;
+  if (p.Fork() > 0) {
+    ASSERT_GT(p.Pid(), 0);
+    ASSERT_EQ(p.GetState(), Process::RUNNING);
+    ASSERT_THROW({ p.Start("ls", NULL); }, std::runtime_error);
+  }
+}
+
+TEST(Process, ForkedChildStart)
+{
+  XORG_TESTCASE("Fork() and calling Start() executes the process");
+  Process p;
+  if (p.Fork() == 0) {
+    ASSERT_EQ(p.GetState(), Process::RUNNING);
+    p.Start("ls", NULL);
+    ASSERT_GT(p.Pid(), 0);
+  }
+}
+
+TEST(Process, ForkedChildDoubleStart)
+{
+  XORG_TESTCASE("Fork() and calling Start() twice causes an exception");
+  Process p;
+  if (p.Fork() == 0) {
+    ASSERT_EQ(p.GetState(), Process::RUNNING);
+    p.Start("ls", NULL);
+    ASSERT_THROW({
+        p.Start("ls", NULL);
+    }, std::runtime_error);
+  }
+}
+
 int main(int argc, char *argv[]) {
   testing::InitGoogleTest(&argc, argv);
   return RUN_ALL_TESTS();