cmUVProcessChain: Return output and error streams as file descriptors

9 files changed, 68 insertions, 89 deletions

diff --git a/Source/cmBinUtilsLinuxELFObjdumpGetRuntimeDependenciesTool.cxx b/Source/cmBinUtilsLinuxELFObjdumpGetRuntimeDependenciesTool.cxx
index c5c41e0..8043f82 100644
--- a/Source/cmBinUtilsLinuxELFObjdumpGetRuntimeDependenciesTool.cxx
+++ b/Source/cmBinUtilsLinuxELFObjdumpGetRuntimeDependenciesTool.cxx
@@ -10,6 +10,7 @@
 #include "cmRuntimeDependencyArchive.h"
 #include "cmSystemTools.h"
 #include "cmUVProcessChain.h"
+#include "cmUVStream.h"
 
 cmBinUtilsLinuxELFObjdumpGetRuntimeDependenciesTool::
   cmBinUtilsLinuxELFObjdumpGetRuntimeDependenciesTool(
@@ -46,7 +47,8 @@ bool cmBinUtilsLinuxELFObjdumpGetRuntimeDependenciesTool::GetFileInfo(
   static const cmsys::RegularExpression neededRegex("^ *NEEDED *([^\n]*)$");
   static const cmsys::RegularExpression rpathRegex("^ *RPATH *([^\n]*)$");
   static const cmsys::RegularExpression runpathRegex("^ *RUNPATH *([^\n]*)$");
-  while (std::getline(*process.OutputStream(), line)) {
+  cmUVPipeIStream output(process.GetLoop(), process.OutputStream());
+  while (std::getline(output, line)) {
     cmsys::RegularExpressionMatch match;
     if (neededRegex.find(line.c_str(), match)) {
       needed.push_back(match.match(1));
diff --git a/Source/cmBinUtilsMacOSMachOOToolGetRuntimeDependenciesTool.cxx b/Source/cmBinUtilsMacOSMachOOToolGetRuntimeDependenciesTool.cxx
index cf4a8fe..4c35841 100644
--- a/Source/cmBinUtilsMacOSMachOOToolGetRuntimeDependenciesTool.cxx
+++ b/Source/cmBinUtilsMacOSMachOOToolGetRuntimeDependenciesTool.cxx
@@ -9,6 +9,7 @@
 
 #include "cmRuntimeDependencyArchive.h"
 #include "cmUVProcessChain.h"
+#include "cmUVStream.h"
 
 cmBinUtilsMacOSMachOOToolGetRuntimeDependenciesTool::
   cmBinUtilsMacOSMachOOToolGetRuntimeDependenciesTool(
@@ -49,11 +50,12 @@ bool cmBinUtilsMacOSMachOOToolGetRuntimeDependenciesTool::GetFileInfo(
     "^ *path (.*) \\(offset [0-9]+\\)$");
   static const cmsys::RegularExpression nameRegex(
     "^ *name (.*) \\(offset [0-9]+\\)$");
-  while (std::getline(*process.OutputStream(), line)) {
+  cmUVPipeIStream output(process.GetLoop(), process.OutputStream());
+  while (std::getline(output, line)) {
     cmsys::RegularExpressionMatch cmdMatch;
     if (rpathRegex.find(line.c_str(), cmdMatch)) {
-      if (!std::getline(*process.OutputStream(), line) ||
-          !std::getline(*process.OutputStream(), line)) {
+      // NOLINTNEXTLINE(misc-redundant-expression)
+      if (!std::getline(output, line) || !std::getline(output, line)) {
         this->SetError("Invalid output from otool");
         return false;
       }
@@ -66,8 +68,8 @@ bool cmBinUtilsMacOSMachOOToolGetRuntimeDependenciesTool::GetFileInfo(
         return false;
       }
     } else if (loadDylibRegex.find(line.c_str(), cmdMatch)) {
-      if (!std::getline(*process.OutputStream(), line) ||
-          !std::getline(*process.OutputStream(), line)) {
+      // NOLINTNEXTLINE(misc-redundant-expression)
+      if (!std::getline(output, line) || !std::getline(output, line)) {
         this->SetError("Invalid output from otool");
         return false;
       }
diff --git a/Source/cmBinUtilsWindowsPEDumpbinGetRuntimeDependenciesTool.cxx b/Source/cmBinUtilsWindowsPEDumpbinGetRuntimeDependenciesTool.cxx
index 3b02c2e..cd21140 100644
--- a/Source/cmBinUtilsWindowsPEDumpbinGetRuntimeDependenciesTool.cxx
+++ b/Source/cmBinUtilsWindowsPEDumpbinGetRuntimeDependenciesTool.cxx
@@ -9,6 +9,7 @@
 
 #include "cmRuntimeDependencyArchive.h"
 #include "cmUVProcessChain.h"
+#include "cmUVStream.h"
 
 cmBinUtilsWindowsPEDumpbinGetRuntimeDependenciesTool::
   cmBinUtilsWindowsPEDumpbinGetRuntimeDependenciesTool(
@@ -43,7 +44,8 @@ bool cmBinUtilsWindowsPEDumpbinGetRuntimeDependenciesTool::GetFileInfo(
   std::string line;
   static const cmsys::RegularExpression regex(
     "^    ([^\n]*\\.[Dd][Ll][Ll])\r$");
-  while (std::getline(*process.OutputStream(), line)) {
+  cmUVPipeIStream output(process.GetLoop(), process.OutputStream());
+  while (std::getline(output, line)) {
     cmsys::RegularExpressionMatch match;
     if (regex.find(line.c_str(), match)) {
       needed.push_back(match.match(1));
diff --git a/Source/cmBinUtilsWindowsPEObjdumpGetRuntimeDependenciesTool.cxx b/Source/cmBinUtilsWindowsPEObjdumpGetRuntimeDependenciesTool.cxx
index 2139b4b..d95da95 100644
--- a/Source/cmBinUtilsWindowsPEObjdumpGetRuntimeDependenciesTool.cxx
+++ b/Source/cmBinUtilsWindowsPEObjdumpGetRuntimeDependenciesTool.cxx
@@ -10,6 +10,7 @@
 #include "cmRuntimeDependencyArchive.h"
 #include "cmSystemTools.h"
 #include "cmUVProcessChain.h"
+#include "cmUVStream.h"
 
 cmBinUtilsWindowsPEObjdumpGetRuntimeDependenciesTool::
   cmBinUtilsWindowsPEObjdumpGetRuntimeDependenciesTool(
@@ -44,7 +45,8 @@ bool cmBinUtilsWindowsPEObjdumpGetRuntimeDependenciesTool::GetFileInfo(
   std::string line;
   static const cmsys::RegularExpression regex(
     "^\t*DLL Name: ([^\n]*\\.[Dd][Ll][Ll])$");
-  while (cmSystemTools::GetLineFromStream(*process.OutputStream(), line)) {
+  cmUVPipeIStream output(process.GetLoop(), process.OutputStream());
+  while (cmSystemTools::GetLineFromStream(output, line)) {
     cmsys::RegularExpressionMatch match;
     if (regex.find(line.c_str(), match)) {
       needed.push_back(match.match(1));
diff --git a/Source/cmLDConfigLDConfigTool.cxx b/Source/cmLDConfigLDConfigTool.cxx
index 7ce1a59..154aa27 100644
--- a/Source/cmLDConfigLDConfigTool.cxx
+++ b/Source/cmLDConfigLDConfigTool.cxx
@@ -14,6 +14,7 @@
 #include "cmRuntimeDependencyArchive.h"
 #include "cmSystemTools.h"
 #include "cmUVProcessChain.h"
+#include "cmUVStream.h"
 
 cmLDConfigLDConfigTool::cmLDConfigLDConfigTool(
   cmRuntimeDependencyArchive* archive)
@@ -50,7 +51,8 @@ bool cmLDConfigLDConfigTool::GetLDConfigPaths(std::vector<std::string>& paths)
 
   std::string line;
   static const cmsys::RegularExpression regex("^([^\t:]*):");
-  while (std::getline(*process.OutputStream(), line)) {
+  cmUVPipeIStream output(process.GetLoop(), process.OutputStream());
+  while (std::getline(output, line)) {
     cmsys::RegularExpressionMatch match;
     if (regex.find(line.c_str(), match)) {
       paths.push_back(match.match(1));
diff --git a/Source/cmUVProcessChain.cxx b/Source/cmUVProcessChain.cxx
index cf4475b..3de7ae5 100644
--- a/Source/cmUVProcessChain.cxx
+++ b/Source/cmUVProcessChain.cxx
@@ -17,36 +17,15 @@
 
 #include "cmGetPipes.h"
 #include "cmUVHandlePtr.h"
-#include "cmUVStreambuf.h"
 
 struct cmUVProcessChain::InternalData
 {
-  struct BasicStreamData
+  struct StreamData
   {
-    cmUVStreambuf Streambuf;
-    cm::uv_pipe_ptr BuiltinStream;
+    int BuiltinStream = -1;
     uv_stdio_container_t Stdio;
   };
 
-  template <typename IOStream>
-  struct StreamData : public BasicStreamData
-  {
-    StreamData()
-      : BuiltinIOStream(&this->Streambuf)
-    {
-    }
-
-    IOStream BuiltinIOStream;
-
-    IOStream* GetBuiltinStream()
-    {
-      if (this->BuiltinStream.get()) {
-        return &this->BuiltinIOStream;
-      }
-      return nullptr;
-    }
-  };
-
   struct ProcessData
   {
     cmUVProcessChain::InternalData* Data;
@@ -64,9 +43,9 @@ struct cmUVProcessChain::InternalData
 
   cm::uv_loop_ptr Loop;
 
-  StreamData<std::ostream> InputStreamData;
-  StreamData<std::istream> OutputStreamData;
-  StreamData<std::istream> ErrorStreamData;
+  StreamData InputStreamData;
+  StreamData OutputStreamData;
+  StreamData ErrorStreamData;
   cm::uv_pipe_ptr TempOutputPipe;
   cm::uv_pipe_ptr TempErrorPipe;
 
@@ -215,12 +194,7 @@ bool cmUVProcessChain::InternalData::Prepare(
         return false;
       }
 
-      if (errorData.BuiltinStream.init(*this->Loop, 0) < 0) {
-        return false;
-      }
-      if (uv_pipe_open(errorData.BuiltinStream, pipeFd[0]) < 0) {
-        return false;
-      }
+      errorData.BuiltinStream = pipeFd[0];
       errorData.Stdio.flags = UV_INHERIT_FD;
       errorData.Stdio.data.fd = pipeFd[1];
 
@@ -231,7 +205,6 @@ bool cmUVProcessChain::InternalData::Prepare(
         return false;
       }
 
-      errorData.Streambuf.open(errorData.BuiltinStream);
       break;
     }
 
@@ -251,6 +224,7 @@ bool cmUVProcessChain::InternalData::Prepare(
 
     case cmUVProcessChainBuilder::Builtin:
       if (this->Builder->MergedBuiltinStreams) {
+        outputData.BuiltinStream = errorData.BuiltinStream;
         outputData.Stdio.flags = UV_INHERIT_FD;
         outputData.Stdio.data.fd = errorData.Stdio.data.fd;
       } else {
@@ -259,12 +233,7 @@ bool cmUVProcessChain::InternalData::Prepare(
           return false;
         }
 
-        if (outputData.BuiltinStream.init(*this->Loop, 0) < 0) {
-          return false;
-        }
-        if (uv_pipe_open(outputData.BuiltinStream, pipeFd[0]) < 0) {
-          return false;
-        }
+        outputData.BuiltinStream = pipeFd[0];
         outputData.Stdio.flags = UV_INHERIT_FD;
         outputData.Stdio.data.fd = pipeFd[1];
 
@@ -274,8 +243,6 @@ bool cmUVProcessChain::InternalData::Prepare(
         if (uv_pipe_open(this->TempOutputPipe, outputData.Stdio.data.fd) < 0) {
           return false;
         }
-
-        outputData.Streambuf.open(outputData.BuiltinStream);
       }
       break;
 
@@ -411,17 +378,14 @@ uv_loop_t& cmUVProcessChain::GetLoop()
   return *this->Data->Loop;
 }
 
-std::istream* cmUVProcessChain::OutputStream()
+int cmUVProcessChain::OutputStream()
 {
-  if (this->Data->Builder->MergedBuiltinStreams) {
-    return this->Data->ErrorStreamData.GetBuiltinStream();
-  }
-  return this->Data->OutputStreamData.GetBuiltinStream();
+  return this->Data->OutputStreamData.BuiltinStream;
 }
 
-std::istream* cmUVProcessChain::ErrorStream()
+int cmUVProcessChain::ErrorStream()
 {
-  return this->Data->ErrorStreamData.GetBuiltinStream();
+  return this->Data->ErrorStreamData.BuiltinStream;
 }
 
 bool cmUVProcessChain::Valid() const
diff --git a/Source/cmUVProcessChain.h b/Source/cmUVProcessChain.h
index 547a944..d7a4a0e 100644
--- a/Source/cmUVProcessChain.h
+++ b/Source/cmUVProcessChain.h
@@ -5,7 +5,6 @@
 #include <array>
 #include <cstddef> // IWYU pragma: keep
 #include <cstdint>
-#include <iosfwd>
 #include <memory>
 #include <string>
 #include <utility>
@@ -99,8 +98,8 @@ public:
   uv_loop_t& GetLoop();
 
   // FIXME: Add stdin support
-  std::istream* OutputStream();
-  std::istream* ErrorStream();
+  int OutputStream();
+  int ErrorStream();
 
   bool Valid() const;
   bool Wait(int64_t milliseconds = -1);
diff --git a/Source/cmcmd.cxx b/Source/cmcmd.cxx
index 1bbd0ac..eb1f93d 100644
--- a/Source/cmcmd.cxx
+++ b/Source/cmcmd.cxx
@@ -28,6 +28,7 @@
 #include "cmSystemTools.h"
 #include "cmTransformDepfile.h"
 #include "cmUVProcessChain.h"
+#include "cmUVStream.h"
 #include "cmUtils.hxx"
 #include "cmValue.h"
 #include "cmVersion.h"
@@ -2017,10 +2018,8 @@ int cmcmd::RunPreprocessor(const std::vector<std::string>& command,
     return 1;
   }
   if (process.GetStatus(0).ExitStatus != 0) {
-    auto* errorStream = process.ErrorStream();
-    if (errorStream) {
-      std::cerr << errorStream->rdbuf();
-    }
+    cmUVPipeIStream errorStream(process.GetLoop(), process.ErrorStream());
+    std::cerr << errorStream.rdbuf();
 
     return 1;
   }
@@ -2144,10 +2143,8 @@ int cmcmd::RunLLVMRC(std::vector<std::string> const& args)
     return result;
   }
   if (process.GetStatus(0).ExitStatus != 0) {
-    auto* errorStream = process.ErrorStream();
-    if (errorStream) {
-      std::cerr << errorStream->rdbuf();
-    }
+    cmUVPipeIStream errorStream(process.GetLoop(), process.ErrorStream());
+    std::cerr << errorStream.rdbuf();
     return 1;
   }
 
diff --git a/Tests/CMakeLib/testUVProcessChain.cxx b/Tests/CMakeLib/testUVProcessChain.cxx
index f3024dc..7630aa0 100644
--- a/Tests/CMakeLib/testUVProcessChain.cxx
+++ b/Tests/CMakeLib/testUVProcessChain.cxx
@@ -17,6 +17,7 @@
 #include "cmStringAlgorithms.h"
 #include "cmUVHandlePtr.h"
 #include "cmUVProcessChain.h"
+#include "cmUVStream.h"
 #include "cmUVStreambuf.h"
 
 struct ExpectedStatus
@@ -301,16 +302,19 @@ bool testUVProcessChainBuiltin(const char* helperCommand)
     return false;
   }
 
-  if (!chain->OutputStream()) {
-    std::cout << "OutputStream() was null, expecting not null" << std::endl;
+  if (chain->OutputStream() < 0) {
+    std::cout << "OutputStream() was invalid, expecting valid" << std::endl;
     return false;
   }
-  if (!chain->ErrorStream()) {
-    std::cout << "ErrorStream() was null, expecting not null" << std::endl;
+  if (chain->ErrorStream() < 0) {
+    std::cout << "ErrorStream() was invalid, expecting valid" << std::endl;
     return false;
   }
 
-  if (!checkOutput(*chain->OutputStream(), *chain->ErrorStream())) {
+  cmUVPipeIStream output(chain->GetLoop(), chain->OutputStream());
+  cmUVPipeIStream error(chain->GetLoop(), chain->ErrorStream());
+
+  if (!checkOutput(output, error)) {
     return false;
   }
 
@@ -330,12 +334,12 @@ bool testUVProcessChainBuiltinMerged(const char* helperCommand)
     return false;
   }
 
-  if (!chain->OutputStream()) {
-    std::cout << "OutputStream() was null, expecting not null" << std::endl;
+  if (chain->OutputStream() < 0) {
+    std::cout << "OutputStream() was invalid, expecting valid" << std::endl;
     return false;
   }
-  if (!chain->ErrorStream()) {
-    std::cout << "ErrorStream() was null, expecting not null" << std::endl;
+  if (chain->ErrorStream() < 0) {
+    std::cout << "ErrorStream() was invalid, expecting valid" << std::endl;
     return false;
   }
   if (chain->OutputStream() != chain->ErrorStream()) {
@@ -344,7 +348,9 @@ bool testUVProcessChainBuiltinMerged(const char* helperCommand)
     return false;
   }
 
-  std::string merged = getInput(*chain->OutputStream());
+  cmUVPipeIStream mergedStream(chain->GetLoop(), chain->OutputStream());
+
+  std::string merged = getInput(mergedStream);
   auto qemuErrorPos = merged.find("qemu:");
   if (qemuErrorPos != std::string::npos) {
     merged.resize(qemuErrorPos);
@@ -398,12 +404,12 @@ bool testUVProcessChainExternal(const char* helperCommand)
     return false;
   }
 
-  if (chain->OutputStream()) {
-    std::cout << "OutputStream() was not null, expecting null" << std::endl;
+  if (chain->OutputStream() >= 0) {
+    std::cout << "OutputStream() was valid, expecting invalid" << std::endl;
     return false;
   }
-  if (chain->ErrorStream()) {
-    std::cout << "ErrorStream() was not null, expecting null" << std::endl;
+  if (chain->ErrorStream() >= 0) {
+    std::cout << "ErrorStream() was valid, expecting invalid" << std::endl;
     return false;
   }
 
@@ -446,12 +452,12 @@ bool testUVProcessChainNone(const char* helperCommand)
     return false;
   }
 
-  if (chain->OutputStream()) {
-    std::cout << "OutputStream() was not null, expecting null" << std::endl;
+  if (chain->OutputStream() >= 0) {
+    std::cout << "OutputStream() was valid, expecting invalid" << std::endl;
     return false;
   }
-  if (chain->ErrorStream()) {
-    std::cout << "ErrorStream() was not null, expecting null" << std::endl;
+  if (chain->ErrorStream() >= 0) {
+    std::cout << "ErrorStream() was valid, expecting invalid" << std::endl;
     return false;
   }
 
@@ -473,7 +479,8 @@ bool testUVProcessChainCwdUnchanged(const char* helperCommand)
     return false;
   }
 
-  auto cwd = getInput(*chain.OutputStream());
+  cmUVPipeIStream output(chain.GetLoop(), chain.OutputStream());
+  auto cwd = getInput(output);
   if (!cmHasLiteralSuffix(cwd, "/Tests/CMakeLib")) {
     std::cout << "Working directory was \"" << cwd
               << "\", expected to end in \"/Tests/CMakeLib\"" << std::endl;
@@ -499,7 +506,8 @@ bool testUVProcessChainCwdChanged(const char* helperCommand)
     return false;
   }
 
-  auto cwd = getInput(*chain.OutputStream());
+  cmUVPipeIStream output(chain.GetLoop(), chain.OutputStream());
+  auto cwd = getInput(output);
   if (!cmHasLiteralSuffix(cwd, "/Tests")) {
     std::cout << "Working directory was \"" << cwd
               << "\", expected to end in \"/Tests\"" << std::endl;
@@ -633,7 +641,8 @@ bool testUVProcessChainInputFile(const char* helperCommand)
     return false;
   }
 
-  std::string output = getInput(*chain.OutputStream());
+  cmUVPipeIStream stream(chain.GetLoop(), chain.OutputStream());
+  std::string output = getInput(stream);
   if (output != "HELO WRD!") {
     std::cout << "Output was \"" << output << "\", expected \"HELO WRD!\""
               << std::endl;