kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject [1/6] kudu git commit: subprocess: some cosmetic changes
Date Mon, 02 Oct 2017 19:25:18 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 5ca7deae7 -> 350c8a79c


subprocess: some cosmetic changes

Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d
Reviewed-on: http://gerrit.cloudera.org:8080/8157
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <danburkert@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/c0be2370
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/c0be2370
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/c0be2370

Branch: refs/heads/master
Commit: c0be2370d543f749a783d594b3e33919f9818877
Parents: 5ca7dea
Author: Adar Dembo <adar@cloudera.com>
Authored: Wed Sep 27 15:46:44 2017 -0700
Committer: Adar Dembo <adar@cloudera.com>
Committed: Mon Oct 2 19:24:54 2017 +0000

----------------------------------------------------------------------
 src/kudu/util/subprocess.cc | 42 +++++++++++++++++++++++++---------------
 src/kudu/util/subprocess.h  | 10 +++++++---
 2 files changed, 33 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/c0be2370/src/kudu/util/subprocess.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/subprocess.cc b/src/kudu/util/subprocess.cc
index 9518ca8..a1e01ba 100644
--- a/src/kudu/util/subprocess.cc
+++ b/src/kudu/util/subprocess.cc
@@ -151,10 +151,17 @@ void CloseNonStandardFDs(DIR* fd_dir) {
 }
 
 void RedirectToDevNull(int fd) {
-  // We must not close stderr or stdout, because then when a new file descriptor
-  // gets opened, it might get that fd number.  (We always allocate the lowest
-  // available file descriptor number.)  Instead, we reopen that fd as
-  // /dev/null.
+  // We must not close stderr or stdout, because then when a new file
+  // descriptor is opened, it might reuse the closed file descriptor's number
+  // (we always allocate the lowest available file descriptor number).
+  //
+  // Instead, we open /dev/null as a new file descriptor, then use dup2() to
+  // atomically close 'fd' and reuse its file descriptor number as an open file
+  // handle to /dev/null.
+  //
+  // It is expected that the file descriptor allocated when opening /dev/null
+  // will be closed when the child process closes all of its "non-standard"
+  // file descriptors later on.
   int dev_null = open("/dev/null", O_WRONLY);
   if (dev_null < 0) {
     PLOG(WARNING) << "failed to open /dev/null";
@@ -280,16 +287,6 @@ Subprocess::~Subprocess() {
   }
 }
 
-void Subprocess::DisableStderr() {
-  CHECK_EQ(state_, kNotStarted);
-  fd_state_[STDERR_FILENO] = DISABLED;
-}
-
-void Subprocess::DisableStdout() {
-  CHECK_EQ(state_, kNotStarted);
-  fd_state_[STDOUT_FILENO] = DISABLED;
-}
-
 #if defined(__APPLE__)
 static int pipe2(int pipefd[2], int flags) {
   DCHECK_EQ(O_CLOEXEC, flags);
@@ -378,6 +375,8 @@ Status Subprocess::Start() {
     // stdin
     if (fd_state_[STDIN_FILENO] == PIPED) {
       PCHECK(dup2(child_stdin[0], STDIN_FILENO) == STDIN_FILENO);
+    } else {
+      DCHECK_EQ(SHARED, fd_state_[STDIN_FILENO]);
     }
 
     // stdout
@@ -391,6 +390,7 @@ Status Subprocess::Start() {
         break;
       }
       default:
+        DCHECK_EQ(SHARED, fd_state_[STDOUT_FILENO]);
         break;
     }
 
@@ -405,6 +405,7 @@ Status Subprocess::Start() {
         break;
       }
       default:
+        DCHECK_EQ(SHARED, fd_state_[STDERR_FILENO]);
         break;
     }
 
@@ -763,8 +764,17 @@ void Subprocess::SetCurrentDir(string cwd) {
 
 void Subprocess::SetFdShared(int stdfd, bool share) {
   CHECK_EQ(state_, kNotStarted);
-  CHECK_NE(fd_state_[stdfd], DISABLED);
-  fd_state_[stdfd] = share? SHARED : PIPED;
+  fd_state_[stdfd] = share ? SHARED : PIPED;
+}
+
+void Subprocess::DisableStderr() {
+  CHECK_EQ(state_, kNotStarted);
+  fd_state_[STDERR_FILENO] = DISABLED;
+}
+
+void Subprocess::DisableStdout() {
+  CHECK_EQ(state_, kNotStarted);
+  fd_state_[STDOUT_FILENO] = DISABLED;
 }
 
 int Subprocess::CheckAndOffer(int stdfd) const {

http://git-wip-us.apache.org/repos/asf/kudu/blob/c0be2370/src/kudu/util/subprocess.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/subprocess.h b/src/kudu/util/subprocess.h
index d69d5e2..4d33c8f 100644
--- a/src/kudu/util/subprocess.h
+++ b/src/kudu/util/subprocess.h
@@ -58,12 +58,16 @@ class Subprocess {
   explicit Subprocess(std::vector<std::string> argv, int sig_on_destruct = SIGKILL);
   ~Subprocess();
 
-  // Disable subprocess stream output.  Must be called before subprocess starts.
+  // Disables subprocess stream output. Is mutually exclusive with stream sharing.
+  //
+  // Must be called before subprocess starts.
   void DisableStderr();
   void DisableStdout();
 
-  // Share a stream with parent. Must be called before subprocess starts.
-  // Cannot set sharing at all if stream is disabled
+  // Configures the subprocess to share the parent's stream. Is mutually
+  // exclusive with stream disabling.
+  //
+  // Must be called before subprocess starts.
   void ShareParentStdin(bool  share = true) { SetFdShared(STDIN_FILENO,  share); }
   void ShareParentStdout(bool share = true) { SetFdShared(STDOUT_FILENO, share); }
   void ShareParentStderr(bool share = true) { SetFdShared(STDERR_FILENO, share); }


Mime
View raw message