Browse Source

cmSystemTools: Teach RunSingleCommand to merge child pipes when possible

Audit the code to make sure there are no callers that use OUTPUT_MERGE
with separate capture strings.  Then change RunSingleCommand to
implement output merging by giving the child process a single pipe for
both its stdout and stderr descriptors.  This will more cleanly merge
the content on atomic write boundaries in the child instead of on
arbitrary buffering boundaries in the parent.
Brad King 10 years ago
parent
commit
1040e690c6
1 changed files with 21 additions and 23 deletions
  1. 21 23
      Source/cmSystemTools.cxx

+ 21 - 23
Source/cmSystemTools.cxx

@@ -17,6 +17,7 @@
 #include <time.h>
 #include <string.h>
 #include <stdlib.h>
+#include <assert.h>
 #ifdef __QNX__
 # include <malloc.h> /* for malloc/free on QNX */
 #endif
@@ -673,7 +674,16 @@ bool cmSystemTools::RunSingleCommand(std::vector<std::string>const& command,
     {
     cmsysProcess_SetPipeShared(cp, cmsysProcess_Pipe_STDOUT, 1);
     cmsysProcess_SetPipeShared(cp, cmsysProcess_Pipe_STDERR, 1);
+    captureStdOut = 0;
+    captureStdErr = 0;
     }
+  else if (outputflag == OUTPUT_MERGE ||
+           (captureStdErr && captureStdErr == captureStdOut))
+    {
+    cmsysProcess_SetOption(cp, cmsysProcess_Option_MergeOutput, 1);
+    captureStdErr = 0;
+    }
+  assert(!captureStdErr || captureStdErr != captureStdOut);
 
   cmsysProcess_SetTimeout(cp, timeout);
   cmsysProcess_Execute(cp);
@@ -699,38 +709,26 @@ bool cmSystemTools::RunSingleCommand(std::vector<std::string>const& command,
           }
         }
 
-      if(pipe == cmsysProcess_Pipe_STDOUT ||
-         (pipe == cmsysProcess_Pipe_STDERR &&
-          captureStdOut == captureStdErr))
+      if (pipe == cmsysProcess_Pipe_STDOUT)
         {
-        if (captureStdOut)
+        if (outputflag != OUTPUT_NONE)
           {
-          tempStdOut.insert(tempStdOut.end(), data, data+length);
+          cmSystemTools::Stdout(data, length);
           }
-        }
-      else if(pipe == cmsysProcess_Pipe_STDERR)
-        {
-        if (captureStdErr)
+        if (captureStdOut)
           {
-          tempStdErr.insert(tempStdErr.end(), data, data+length);
+          tempStdOut.insert(tempStdOut.end(), data, data+length);
           }
         }
-      if(outputflag != OUTPUT_NONE)
+      else if (pipe == cmsysProcess_Pipe_STDERR)
         {
-        if(outputflag == OUTPUT_MERGE)
+        if (outputflag != OUTPUT_NONE)
           {
-          cmSystemTools::Stdout(data, length);
+          cmSystemTools::Stderr(data, length);
           }
-        else
+        if (captureStdErr)
           {
-          if(pipe == cmsysProcess_Pipe_STDERR)
-            {
-            cmSystemTools::Stderr(data, length);
-            }
-          else if(pipe == cmsysProcess_Pipe_STDOUT)
-            {
-            cmSystemTools::Stdout(data, length);
-            }
+          tempStdErr.insert(tempStdErr.end(), data, data+length);
           }
         }
       }
@@ -741,7 +739,7 @@ bool cmSystemTools::RunSingleCommand(std::vector<std::string>const& command,
     {
     captureStdOut->assign(tempStdOut.begin(), tempStdOut.end());
     }
-  if (captureStdErr && captureStdErr != captureStdOut)
+  if (captureStdErr)
     {
     captureStdErr->assign(tempStdErr.begin(), tempStdErr.end());
     }