commit d42edb4699b183c7d1ef54b0386b868dd446d8c3
author James Kuszmaul <james.kuszmaul@bluerivertech.com> Fri Jan 07 18:00:16 2022 -0800
committer James Kuszmaul <jabukuszmaul+collab@gmail.com> Fri Jan 07 19:32:21 2022 -0800
tree 2f1cd1637abdc2f48ffd092cdfa9ab72d594da76
parent 731a05deae393a017ef525aa54c22e68b47b4cf2

Add support for capturing stdout/err in Application

This also makes it so that the Application object can poll instead of
just relying on SIGCHLD to watch for application stops, to make it a bit
cleaner for simple use-cases.

Change-Id: I8af71e1dd89e0cfa1b189ba1e5264df0df9b9560
Signed-off-by: James Kuszmaul <james.kuszmaul@bluerivertech.com>

aos/util/scoped_pipe.cc

diff --git a/aos/util/scoped_pipe.cc b/aos/util/scoped_pipe.cc
index 36bc1b3..d677b07 100644
--- a/aos/util/scoped_pipe.cc
+++ b/aos/util/scoped_pipe.cc
@@ -26,13 +26,21 @@
   return *this;
 }
 
-std::tuple<ScopedPipe::ScopedReadPipe, ScopedPipe::ScopedWritePipe>
-ScopedPipe::MakePipe() {
+ScopedPipe::PipePair ScopedPipe::MakePipe() {
   int fds[2];
   PCHECK(pipe(fds) != -1);
   PCHECK(fcntl(fds[0], F_SETFL, fcntl(fds[0], F_GETFL) | O_NONBLOCK) != -1);
   PCHECK(fcntl(fds[1], F_SETFL, fcntl(fds[1], F_GETFL) | O_NONBLOCK) != -1);
-  return {ScopedReadPipe(fds[0]), ScopedWritePipe(fds[1])};
+  return {std::unique_ptr<ScopedReadPipe>(new ScopedReadPipe(fds[0])),
+          std::unique_ptr<ScopedWritePipe>(new ScopedWritePipe(fds[1]))};
+}
+
+void ScopedPipe::SetCloexec() {
+  // FD_CLOEXEC is the only known file descriptor flag, but call GETFD just in
+  // case.
+  int flags = fcntl(fd(), F_GETFD);
+  PCHECK(flags != -1);
+  PCHECK(fcntl(fd(), F_SETFD, flags | FD_CLOEXEC) != -1);
 }
 
 size_t ScopedPipe::ScopedReadPipe::Read(std::string *buffer) {

aos/util/scoped_pipe.h

diff --git a/aos/util/scoped_pipe.h b/aos/util/scoped_pipe.h
index 6716fb5..fb91e02 100644
--- a/aos/util/scoped_pipe.h
+++ b/aos/util/scoped_pipe.h
@@ -3,8 +3,8 @@
 
 #include <stdint.h>
 
+#include <memory>
 #include <optional>
-#include <tuple>
 
 #include "absl/types/span.h"
 
@@ -16,11 +16,18 @@
   class ScopedReadPipe;
   class ScopedWritePipe;
 
-  static std::tuple<ScopedReadPipe, ScopedWritePipe> MakePipe();
+  struct PipePair {
+    std::unique_ptr<ScopedReadPipe> read;
+    std::unique_ptr<ScopedWritePipe> write;
+  };
+
+  static PipePair MakePipe();
 
   virtual ~ScopedPipe();
 
   int fd() const { return fd_; }
+  // Sets FD_CLOEXEC on the file descriptor.
+  void SetCloexec();
 
  private:
   ScopedPipe(int fd = -1);

aos/util/scoped_pipe_test.cc

diff --git a/aos/util/scoped_pipe_test.cc b/aos/util/scoped_pipe_test.cc
index 183688e..c71e272 100644
--- a/aos/util/scoped_pipe_test.cc
+++ b/aos/util/scoped_pipe_test.cc
@@ -1,5 +1,7 @@
 #include "aos/util/scoped_pipe.h"
 
+#include <fcntl.h>
+
 #include <array>
 #include <string>
 
@@ -11,39 +13,45 @@
 
 // Tests using uint32_t read/write methods on the ScopedPipe objects.
 TEST(ScopedPipeTest, IntegerPipe) {
-  std::tuple<ScopedPipe::ScopedReadPipe, ScopedPipe::ScopedWritePipe>
-      pipe = ScopedPipe::MakePipe();
-  ASSERT_FALSE(std::get<0>(pipe).Read().has_value())
+  ScopedPipe::PipePair pipe = ScopedPipe::MakePipe();
+  ASSERT_FALSE(pipe.read->Read().has_value())
       << "Shouldn't get anything on empty read.";
-  std::get<1>(pipe).Write(971);
-  ASSERT_EQ(971, std::get<0>(pipe).Read().value());
+  pipe.write->Write(971);
+  ASSERT_EQ(971, pipe.read->Read().value());
 }
 
 // Tests using string read/write methods on the ScopedPipe objects.
 TEST(ScopedPipeTest, StringPipe) {
-  std::tuple<ScopedPipe::ScopedReadPipe, ScopedPipe::ScopedWritePipe>
-      pipe = ScopedPipe::MakePipe();
+  ScopedPipe::PipePair pipe = ScopedPipe::MakePipe();
   std::string buffer;
-  ASSERT_EQ(0u, std::get<0>(pipe).Read(&buffer))
+  ASSERT_EQ(0u, pipe.read->Read(&buffer))
       << "Shouldn't get anything on empty read.";
   ASSERT_TRUE(buffer.empty());
 
   const char *const kAbc = "abcdef";
-  std::get<1>(pipe).Write(
+  pipe.write->Write(
       absl::Span<const uint8_t>(reinterpret_cast<const uint8_t *>(kAbc), 6));
-  ASSERT_EQ(6u, std::get<0>(pipe).Read(&buffer));
+  ASSERT_EQ(6u, pipe.read->Read(&buffer));
   ASSERT_EQ("abcdef", buffer);
 
   std::array<uint8_t, 10000> large_buffer;
   large_buffer.fill(99);
-  std::get<1>(pipe).Write(
+  pipe.write->Write(
       absl::Span<const uint8_t>(large_buffer.data(), large_buffer.size()));
-  ASSERT_EQ(large_buffer.size(), std::get<0>(pipe).Read(&buffer));
+  ASSERT_EQ(large_buffer.size(), pipe.read->Read(&buffer));
   for (size_t ii = 0; ii < large_buffer.size(); ++ii) {
     ASSERT_EQ(large_buffer[ii], buffer[ii + 6]);
   }
 }
 
+// Tests that calling SetCloexec succeeds and does indeed set FD_CLOEXEC.
+TEST(ScopedPipeTest, SetCloexec) {
+  ScopedPipe::PipePair pipe = ScopedPipe::MakePipe();
+  ASSERT_EQ(0, fcntl(pipe.read->fd(), F_GETFD) & FD_CLOEXEC);
+  pipe.read->SetCloexec();
+  ASSERT_NE(0, fcntl(pipe.read->fd(), F_GETFD) & FD_CLOEXEC);
+}
+
 }  // namespace testing
 }  // namespace util
 }  // namespace aos