kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t...@apache.org
Subject [1/4] kudu git commit: KUDU-1674: Fix SubProcess:Call SEGV when trying to capture stderr alone
Date Wed, 05 Oct 2016 21:29:42 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 3830fc972 -> 64f9ab34f


KUDU-1674: Fix SubProcess:Call SEGV when trying to capture stderr alone

ReadFdsFully() captures either stdout or stderr or both depending on how
caller has registered. Once the helpers are done with capturing, it needs
take into account that there can be less than 2 fds registered from the
caller and hence can not do std::move(vector[1]) operation on it.

Also added a test code to exercise combination of:
SubProcess::Call({argv}, nullptr, &stderr);
SubProcess::Call({argv}, &stdout, nullptr);
SubProcess::Call({argv}, nullptr, nullptr);

Change-Id: I67bd462098526a9ba032669b621b8139b56ee5b8
Reviewed-on: http://gerrit.cloudera.org:8080/4594
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <aserbin@cloudera.com>


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

Branch: refs/heads/master
Commit: 0f99d403da61a5304d1e2b01516d949ecd311334
Parents: 3830fc9
Author: Dinesh Bhat <dinesh@cloudera.com>
Authored: Mon Oct 3 00:34:36 2016 -0700
Committer: Alexey Serbin <aserbin@cloudera.com>
Committed: Wed Oct 5 19:22:58 2016 +0000

----------------------------------------------------------------------
 src/kudu/util/subprocess-test.cc | 20 ++++++++++++++++++++
 src/kudu/util/subprocess.cc      | 12 ++++++++----
 2 files changed, 28 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/0f99d403/src/kudu/util/subprocess-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/subprocess-test.cc b/src/kudu/util/subprocess-test.cc
index 31b47fa..a1da506 100644
--- a/src/kudu/util/subprocess-test.cc
+++ b/src/kudu/util/subprocess-test.cc
@@ -24,6 +24,7 @@
 
 #include <gtest/gtest.h>
 
+#include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/test_util.h"
 
 using std::string;
@@ -127,4 +128,23 @@ TEST_F(SubprocessTest, TestReadFromStdoutAndStderr) {
   }, &stdout, &stderr));
 }
 
+// Test KUDU-1674: '/bin/bash -c "echo"' command below is expected to
+// capture a string on stderr. This test validates that passing
+// stderr alone doesn't result in SIGSEGV as reported in the bug and
+// also check for sanity of stderr in the output.
+TEST_F(SubprocessTest, TestReadSingleFD) {
+  string stderr;
+  const string str = "ApacheKudu";
+  const string cmd_str = strings::Substitute("/bin/echo -n $0 1>&2", str);
+  ASSERT_OK(Subprocess::Call({"/bin/sh", "-c", cmd_str}, nullptr, &stderr));
+  ASSERT_EQ(stderr, str);
+
+  // Also sanity check other combinations.
+  string stdout;
+  ASSERT_OK(Subprocess::Call({"/bin/ls", "/dev/null"}, &stdout, nullptr));
+  ASSERT_STR_CONTAINS(stdout, "/dev/null");
+
+  ASSERT_OK(Subprocess::Call({"/bin/ls", "/dev/zero"}, nullptr, nullptr));
+}
+
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/0f99d403/src/kudu/util/subprocess.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/subprocess.cc b/src/kudu/util/subprocess.cc
index fb296d4..db8315f 100644
--- a/src/kudu/util/subprocess.cc
+++ b/src/kudu/util/subprocess.cc
@@ -493,14 +493,18 @@ Status Subprocess::Call(const vector<string>& argv,
   if (stderr_out) {
     fds.push_back(p.from_child_stderr_fd());
   }
-  vector<string> out;
-  RETURN_NOT_OK(ReadFdsFully(argv[0], fds, &out));
+  vector<string> outv;
+  RETURN_NOT_OK(ReadFdsFully(argv[0], fds, &outv));
 
+  // Given that ReadFdsFully captures the strings in the order in which we
+  // had installed 'fds' above, it can be assured that we can receive
+  // as many strings as there were 'fds' in the vector and in that order.
+  CHECK_EQ(outv.size(), fds.size());
   if (stdout_out) {
-    *stdout_out = std::move(out[0]);
+    *stdout_out = std::move(outv.front());
   }
   if (stderr_out) {
-    *stderr_out = std::move(out[1]);
+    *stderr_out = std::move(outv.back());
   }
 
   int retcode;


Mime
View raw message