Adar Dembo has posted comments on this change. Change subject: [util] added Subprocess::GetExitStatus() ......................................................................
Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/4648/2/src/kudu/util/pstack_watcher.cc File src/kudu/util/pstack_watcher.cc: PS2, Line 178: RETURN_NOT_OK For consistency with the rest of the function, should use RETURN_NOT_OK_PREPEND() here. http://gerrit.cloudera.org:8080/#/c/4648/2/src/kudu/util/subprocess-test.cc File src/kudu/util/subprocess-test.cc: Line 153: TEST_F(SubprocessTest, TestGetExitStatusExitSuccess) { Can you use initializer lists to set up argv in the new tests? Line 191: TEST_F(SubprocessTest, TestGetExitStatusSignaled) { Does this test work properly if /bin/cat finishes before p.Kill() has a chance to run? Or does it fail? http://gerrit.cloudera.org:8080/#/c/4648/2/src/kudu/util/subprocess.cc File src/kudu/util/subprocess.cc: Line 526: AFAICT, the three functions below were just moved, right? Or have they changed in a non-trivial way (i.e. not just WNOHANG->WaitMode)? -- To view, visit http://gerrit.cloudera.org:8080/4648 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic2b16e2a2a53a01982f816b9ee41cc61fd93d4bf Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
