Commit c59d3032 authored by Adam Simpkins's avatar Adam Simpkins Committed by Facebook Github Bot

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
parent f2423568
......@@ -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");
}
......
......@@ -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
......
......@@ -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 { \
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment