Commit fca9fccb authored by Tudor Bosman's avatar Tudor Bosman

Simplify pipe2 / O_CLOEXEC handling in Subprocess.cpp

Summary: per @rockyliu's suggestions

Test Plan: subprocess_test

Reviewed By: rockyliu4@fb.com

Subscribers: rockyliu, jhj, lesha, kma

FB internal diff: D1423157
parent 2c378ec8
...@@ -256,29 +256,25 @@ void Subprocess::spawn( ...@@ -256,29 +256,25 @@ void Subprocess::spawn(
// in case it fails before calling exec() // in case it fails before calling exec()
int errFds[2]; int errFds[2];
#if FOLLY_HAVE_PIPE2 #if FOLLY_HAVE_PIPE2
int r = ::pipe2(errFds, O_CLOEXEC); checkUnixError(::pipe2(errFds, O_CLOEXEC), "pipe2");
#else #else
int r = ::pipe(errFds); checkUnixError(::pipe(errFds), "pipe");
#endif #endif
checkUnixError(r, "pipe");
SCOPE_EXIT { SCOPE_EXIT {
CHECK_ERR(::close(errFds[0])); CHECK_ERR(::close(errFds[0]));
if (errFds[1] >= 0) { if (errFds[1] >= 0) {
CHECK_ERR(::close(errFds[1])); CHECK_ERR(::close(errFds[1]));
} }
}; };
// Ask the child to close the read end of the error pipe.
options.fdActions_[errFds[0]] = CLOSE;
#if !FOLLY_HAVE_PIPE2 #if !FOLLY_HAVE_PIPE2
r = fcntl(errFds[0], F_SETFD, FD_CLOEXEC); // Ask the child to close the read end of the error pipe.
checkUnixError(r, "set FD_CLOEXEC"); checkUnixError(fcntl(errFds[0], F_SETFD, FD_CLOEXEC), "set FD_CLOEXEC");
// Set the close-on-exec flag on the write side of the pipe. // Set the close-on-exec flag on the write side of the pipe.
// This way the pipe will be closed automatically in the child if execve() // This way the pipe will be closed automatically in the child if execve()
// succeeds. If the exec fails the child can write error information to the // succeeds. If the exec fails the child can write error information to the
// pipe. // pipe.
r = fcntl(errFds[1], F_SETFD, FD_CLOEXEC); checkUnixError(fcntl(errFds[1], F_SETFD, FD_CLOEXEC), "set FD_CLOEXEC");
checkUnixError(r, "set FD_CLOEXEC");
#endif #endif
// Perform the actual work of setting up pipes then forking and // Perform the actual work of setting up pipes then forking and
...@@ -454,12 +450,9 @@ int Subprocess::prepareChild(const Options& options, ...@@ -454,12 +450,9 @@ int Subprocess::prepareChild(const Options& options,
} }
} }
// Close parent's ends of all pipes // We don't have to explicitly close the parent's end of all pipes,
for (auto& p : pipes_) { // as they all have the FD_CLOEXEC flag set and will be closed at
if (::close(p.parentFd) == -1) { // exec time.
return errno;
}
}
// Close all fds that we're supposed to close. // Close all fds that we're supposed to close.
for (auto& p : options.fdActions_) { for (auto& p : options.fdActions_) {
......
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