From: Adam Simpkins <simpkins@fb.com>
Date: Thu, 26 May 2016 19:50:38 +0000 (-0700)
Subject: add a default Subprocess constructor
X-Git-Tag: 2016.07.26~199
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=c59d30324f8901974800d70df3fba4d09d17ec1e;p=folly.git

add a default Subprocess constructor

Summary:
The default constructor creates the Subprocess in an uninitialized state.  This
makes it possible to default-construct a Subprocess, but only initialize it
later using the move assignment operator.

Even before this diff, it was possible to have Subprocess objects in
uninitialized states after using the move assignment operator to move the data
out of a Subprocess object.

Reviewed By: yfeldblum

Differential Revision: D3348490

fbshipit-source-id: aa6acef9be770de8f0ee118da294cb134f04a466
---

diff --git a/folly/Subprocess.cpp b/folly/Subprocess.cpp
index 8190c470..ad2c077d 100644
--- a/folly/Subprocess.cpp
+++ b/folly/Subprocess.cpp
@@ -162,13 +162,13 @@ Subprocess::Options& Subprocess::Options::fd(int fd, int action) {
   return *this;
 }
 
+Subprocess::Subprocess() {}
+
 Subprocess::Subprocess(
     const std::vector<std::string>& argv,
     const Options& options,
     const char* executable,
-    const std::vector<std::string>* env)
-  : pid_(-1),
-    returnCode_(RV_NOT_STARTED) {
+    const std::vector<std::string>* env) {
   if (argv.empty()) {
     throw std::invalid_argument("argv must not be empty");
   }
@@ -179,9 +179,7 @@ Subprocess::Subprocess(
 Subprocess::Subprocess(
     const std::string& cmd,
     const Options& options,
-    const std::vector<std::string>* env)
-  : pid_(-1),
-    returnCode_(RV_NOT_STARTED) {
+    const std::vector<std::string>* env) {
   if (options.usePath_) {
     throw std::invalid_argument("usePath() not allowed when running in shell");
   }
diff --git a/folly/Subprocess.h b/folly/Subprocess.h
index 7598ee46..8dabfc09 100644
--- a/folly/Subprocess.h
+++ b/folly/Subprocess.h
@@ -426,6 +426,14 @@ class Subprocess {
   Subprocess(Subprocess&&) = default;
   Subprocess& operator=(Subprocess&&) = default;
 
+  /**
+   * Create an uninitialized subprocess.
+   *
+   * In this state it can only be destroyed, or assigned to using the move
+   * assignment operator.
+   */
+  Subprocess();
+
   /**
    * Create a subprocess from the given arguments.  argv[0] must be listed.
    * If not-null, executable must be the actual executable
@@ -821,9 +829,8 @@ class Subprocess {
   // Returns an index into pipes_. Throws std::invalid_argument if not found.
   size_t findByChildFd(const int childFd) const;
 
-
-  pid_t pid_;
-  ProcessReturnCode returnCode_;
+  pid_t pid_{-1};
+  ProcessReturnCode returnCode_{RV_NOT_STARTED};
 
   /**
    * Represents a pipe between this process, and the child process (or its
diff --git a/folly/test/SubprocessTest.cpp b/folly/test/SubprocessTest.cpp
index 0b19e1b3..817245e3 100644
--- a/folly/test/SubprocessTest.cpp
+++ b/folly/test/SubprocessTest.cpp
@@ -71,6 +71,19 @@ TEST(SimpleSubprocessTest, MoveSubprocess) {
   // Now old_proc is destroyed, but we don't crash.
 }
 
+TEST(SimpleSubprocessTest, DefaultConstructor) {
+  Subprocess proc;
+  EXPECT_TRUE(proc.returnCode().notStarted());
+
+  {
+    auto p1 = Subprocess(std::vector<std::string>{"/bin/true"});
+    proc = std::move(p1);
+  }
+
+  EXPECT_TRUE(proc.returnCode().running());
+  EXPECT_EQ(0, proc.wait().exitStatus());
+}
+
 #define EXPECT_SPAWN_ERROR(err, errMsg, cmd, ...) \
   do { \
     try { \