From: Christopher Dykes Date: Wed, 7 Dec 2016 19:42:17 +0000 (-0800) Subject: Rename stdin, etc. in Subprocess to work with MSVC X-Git-Tag: v2016.12.12.00~13 X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=2f6eb18232a6171794142ad8e7fa234b14726d7f;p=folly.git 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 --- diff --git a/folly/Subprocess.h b/folly/Subprocess.h index a3b9f877..35b9a532 100644 --- a/folly/Subprocess.h +++ b/folly/Subprocess.h @@ -34,12 +34,12 @@ * * A thread-safe [1] version of popen() (type="r", to read from the child): * Subprocess proc(cmd, Subprocess::pipeStdout()); - * // read from proc.stdout() + * // read from proc.stdoutFd() * proc.wait(); * * A thread-safe [1] version of popen() (type="w", to write to the child): * Subprocess proc(cmd, Subprocess::pipeStdin()); - * // write to proc.stdin() + * // write to proc.stdinFd() * proc.wait(); * * If you want to redirect both stdin and stdout to pipes, you can, but note @@ -298,19 +298,19 @@ class Subprocess { /** * 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. */ - 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. * Note that stderr(1) will redirect the standard error to the same * 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& pipeStdout() { return fd(STDOUT_FILENO, PIPE_OUT); } @@ -416,9 +416,9 @@ class Subprocess { dangerousPostForkPreExecCallback_{nullptr}; }; - static Options pipeStdin() { return Options().stdin(PIPE); } - static Options pipeStdout() { return Options().stdout(PIPE); } - static Options pipeStderr() { return Options().stderr(PIPE); } + static Options pipeStdin() { return Options().stdinFd(PIPE); } + static Options pipeStdout() { return Options().stdoutFd(PIPE); } + static Options pipeStderr() { return Options().stderrFd(PIPE); } // Non-copiable, but movable Subprocess(const Subprocess&) = delete; @@ -768,9 +768,9 @@ class Subprocess { int parentFd(int childFd) const { return pipes_[findByChildFd(childFd)].pipe.fd(); } - int stdin() const { return parentFd(0); } - int stdout() const { return parentFd(1); } - int stderr() const { return parentFd(2); } + int stdinFd() const { return parentFd(0); } + int stdoutFd() const { return parentFd(1); } + int stderrFd() const { return parentFd(2); } /** * The child's pipes are logically separate from the process metadata diff --git a/folly/experimental/test/NestedCommandLineAppTest.cpp b/folly/experimental/test/NestedCommandLineAppTest.cpp index 597b15f7..4acddd2e 100644 --- a/folly/experimental/test/NestedCommandLineAppTest.cpp +++ b/folly/experimental/test/NestedCommandLineAppTest.cpp @@ -47,7 +47,7 @@ std::string callHelper(std::initializer_list args, Subprocess::Options options; if (stdoutFd != -1) { - options.stdout(stdoutFd); + options.stdoutFd(stdoutFd); } else { options.pipeStdout(); } diff --git a/folly/test/SingletonTest.cpp b/folly/test/SingletonTest.cpp index 40e12b98..14c4efa6 100644 --- a/folly/test/SingletonTest.cpp +++ b/folly/test/SingletonTest.cpp @@ -608,8 +608,8 @@ TEST(Singleton, DoubleRegistrationLogging) { auto p = Subprocess( std::vector{sub.string()}, Subprocess::Options() - .stdin(Subprocess::CLOSE) - .stdout(Subprocess::CLOSE) + .stdinFd(Subprocess::CLOSE) + .stdoutFd(Subprocess::CLOSE) .pipeStderr() .closeOtherFds()); auto err = p.communicate("").second; diff --git a/folly/test/SubprocessTest.cpp b/folly/test/SubprocessTest.cpp index 3d29c972..93fd08ce 100644 --- a/folly/test/SubprocessTest.cpp +++ b/folly/test/SubprocessTest.cpp @@ -199,7 +199,7 @@ TEST(SimpleSubprocessTest, FdLeakTest) { checkFdLeak([] { try { Subprocess proc(std::vector({"/no/such/file"}), - Subprocess::pipeStdout().stderr(Subprocess::PIPE)); + Subprocess::pipeStdout().stderrFd(Subprocess::PIPE)); ADD_FAILURE() << "expected an error when running /no/such/file"; } catch (const SubprocessSpawnError& ex) { EXPECT_EQ(ENOENT, ex.errnoValue()); @@ -236,7 +236,7 @@ TEST(ParentDeathSubprocessTest, ParentDeathSignal) { TEST(PopenSubprocessTest, PopenRead) { Subprocess proc("ls /", Subprocess::pipeStdout()); int found = 0; - gen::byLine(File(proc.stdout())) | + gen::byLine(File(proc.stdoutFd())) | [&] (StringPiece line) { if (line == "etc" || line == "bin" || line == "usr") { ++found;