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 8190c47063b64737b1a277ba579caab20b5cf3ae..ad2c077dcc9d2729d4337ed5f3a5a5c4fbc8ba89 100644 (file)
--- 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 7598ee462665ff890c29722919ea75597c132b34..8dabfc09af326047ae766ca1824e9e017f3a56cc 100644 (file)
--- 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 0b19e1b3464534e9d630378cc4c1cc6bd192f1f8..817245e360356dcfbec8ee753d6e0b486acc8909 100644 (file)
--- 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 { \