Commit 2f6eb182 authored by Christopher Dykes's avatar Christopher Dykes Committed by Facebook Github Bot

Rename stdin, etc. in Subprocess to work with MSVC

Summary:
`stdin`, `stdout` and `stderr` are macros that expand to function calls with the MSVC CRT implementation. This is also the case for musl-libc. This means that Subprocess simply cannot be compiled on those platforms without changing the API.
To solve that, we change the API and deprecate the old API.

For more fun, `stdin`, `stdout` and `stderr` are also macros in glibc, they just expand to other identifiers rather than a function call.

Reviewed By: yfeldblum

Differential Revision: D4229544

fbshipit-source-id: 97f1a3b228b83cfdcaffee56d729063ea235e608
parent 65d64c0d
...@@ -34,12 +34,12 @@ ...@@ -34,12 +34,12 @@
* *
* A thread-safe [1] version of popen() (type="r", to read from the child): * A thread-safe [1] version of popen() (type="r", to read from the child):
* Subprocess proc(cmd, Subprocess::pipeStdout()); * Subprocess proc(cmd, Subprocess::pipeStdout());
* // read from proc.stdout() * // read from proc.stdoutFd()
* proc.wait(); * proc.wait();
* *
* A thread-safe [1] version of popen() (type="w", to write to the child): * A thread-safe [1] version of popen() (type="w", to write to the child):
* Subprocess proc(cmd, Subprocess::pipeStdin()); * Subprocess proc(cmd, Subprocess::pipeStdin());
* // write to proc.stdin() * // write to proc.stdinFd()
* proc.wait(); * proc.wait();
* *
* If you want to redirect both stdin and stdout to pipes, you can, but note * If you want to redirect both stdin and stdout to pipes, you can, but note
...@@ -298,19 +298,19 @@ class Subprocess { ...@@ -298,19 +298,19 @@ class Subprocess {
/** /**
* Shortcut to change the action for standard input. * Shortcut to change the action for standard input.
*/ */
Options& stdin(int action) { return fd(STDIN_FILENO, action); } Options& stdinFd(int action) { return fd(STDIN_FILENO, action); }
/** /**
* Shortcut to change the action for standard output. * Shortcut to change the action for standard output.
*/ */
Options& stdout(int action) { return fd(STDOUT_FILENO, action); } Options& stdoutFd(int action) { return fd(STDOUT_FILENO, action); }
/** /**
* Shortcut to change the action for standard error. * Shortcut to change the action for standard error.
* Note that stderr(1) will redirect the standard error to the same * Note that stderr(1) will redirect the standard error to the same
* file descriptor as standard output; the equivalent of bash's "2>&1" * file descriptor as standard output; the equivalent of bash's "2>&1"
*/ */
Options& stderr(int action) { return fd(STDERR_FILENO, action); } Options& stderrFd(int action) { return fd(STDERR_FILENO, action); }
Options& pipeStdin() { return fd(STDIN_FILENO, PIPE_IN); } Options& pipeStdin() { return fd(STDIN_FILENO, PIPE_IN); }
Options& pipeStdout() { return fd(STDOUT_FILENO, PIPE_OUT); } Options& pipeStdout() { return fd(STDOUT_FILENO, PIPE_OUT); }
...@@ -416,9 +416,9 @@ class Subprocess { ...@@ -416,9 +416,9 @@ class Subprocess {
dangerousPostForkPreExecCallback_{nullptr}; dangerousPostForkPreExecCallback_{nullptr};
}; };
static Options pipeStdin() { return Options().stdin(PIPE); } static Options pipeStdin() { return Options().stdinFd(PIPE); }
static Options pipeStdout() { return Options().stdout(PIPE); } static Options pipeStdout() { return Options().stdoutFd(PIPE); }
static Options pipeStderr() { return Options().stderr(PIPE); } static Options pipeStderr() { return Options().stderrFd(PIPE); }
// Non-copiable, but movable // Non-copiable, but movable
Subprocess(const Subprocess&) = delete; Subprocess(const Subprocess&) = delete;
...@@ -768,9 +768,9 @@ class Subprocess { ...@@ -768,9 +768,9 @@ class Subprocess {
int parentFd(int childFd) const { int parentFd(int childFd) const {
return pipes_[findByChildFd(childFd)].pipe.fd(); return pipes_[findByChildFd(childFd)].pipe.fd();
} }
int stdin() const { return parentFd(0); } int stdinFd() const { return parentFd(0); }
int stdout() const { return parentFd(1); } int stdoutFd() const { return parentFd(1); }
int stderr() const { return parentFd(2); } int stderrFd() const { return parentFd(2); }
/** /**
* The child's pipes are logically separate from the process metadata * The child's pipes are logically separate from the process metadata
......
...@@ -47,7 +47,7 @@ std::string callHelper(std::initializer_list<std::string> args, ...@@ -47,7 +47,7 @@ std::string callHelper(std::initializer_list<std::string> args,
Subprocess::Options options; Subprocess::Options options;
if (stdoutFd != -1) { if (stdoutFd != -1) {
options.stdout(stdoutFd); options.stdoutFd(stdoutFd);
} else { } else {
options.pipeStdout(); options.pipeStdout();
} }
......
...@@ -608,8 +608,8 @@ TEST(Singleton, DoubleRegistrationLogging) { ...@@ -608,8 +608,8 @@ TEST(Singleton, DoubleRegistrationLogging) {
auto p = Subprocess( auto p = Subprocess(
std::vector<std::string>{sub.string()}, std::vector<std::string>{sub.string()},
Subprocess::Options() Subprocess::Options()
.stdin(Subprocess::CLOSE) .stdinFd(Subprocess::CLOSE)
.stdout(Subprocess::CLOSE) .stdoutFd(Subprocess::CLOSE)
.pipeStderr() .pipeStderr()
.closeOtherFds()); .closeOtherFds());
auto err = p.communicate("").second; auto err = p.communicate("").second;
......
...@@ -199,7 +199,7 @@ TEST(SimpleSubprocessTest, FdLeakTest) { ...@@ -199,7 +199,7 @@ TEST(SimpleSubprocessTest, FdLeakTest) {
checkFdLeak([] { checkFdLeak([] {
try { try {
Subprocess proc(std::vector<std::string>({"/no/such/file"}), Subprocess proc(std::vector<std::string>({"/no/such/file"}),
Subprocess::pipeStdout().stderr(Subprocess::PIPE)); Subprocess::pipeStdout().stderrFd(Subprocess::PIPE));
ADD_FAILURE() << "expected an error when running /no/such/file"; ADD_FAILURE() << "expected an error when running /no/such/file";
} catch (const SubprocessSpawnError& ex) { } catch (const SubprocessSpawnError& ex) {
EXPECT_EQ(ENOENT, ex.errnoValue()); EXPECT_EQ(ENOENT, ex.errnoValue());
...@@ -236,7 +236,7 @@ TEST(ParentDeathSubprocessTest, ParentDeathSignal) { ...@@ -236,7 +236,7 @@ TEST(ParentDeathSubprocessTest, ParentDeathSignal) {
TEST(PopenSubprocessTest, PopenRead) { TEST(PopenSubprocessTest, PopenRead) {
Subprocess proc("ls /", Subprocess::pipeStdout()); Subprocess proc("ls /", Subprocess::pipeStdout());
int found = 0; int found = 0;
gen::byLine(File(proc.stdout())) | gen::byLine(File(proc.stdoutFd())) |
[&] (StringPiece line) { [&] (StringPiece line) {
if (line == "etc" || line == "bin" || line == "usr") { if (line == "etc" || line == "bin" || line == "usr") {
++found; ++found;
......
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