Stderr: overhaul error piping from child (#25)

We were not reading stderr from subprocesses (which could cause a hang).

Signed-off-by: John Parent <john.parent@kitware.com>

Author: johnwparent

Makefile

@@ -147,15 +147,26 @@ test_relocate_dll: build_and_check_test_sample
 	.\tester.exe
 	cd ../..
 
-test_pipe_overflow: build_and_check_test_sample
-	echo "--------------------"
-	echo " Pipe overflow test"
-	echo "--------------------"
+test_pipe_out_overflow: build_and_check_test_sample
+	@echo \n
+	@echo ---------------------------
+	@echo  Pipe stdout overflow test
+	@echo ---------------------------
 	set SPACK_CC_TMP=%SPACK_CC%
 	set SPACK_CC=$(MAKEDIR)\test\lots-of-output.bat
 	cl /c /EHsc "test\src file\calc.cxx"
 	set SPACK_CC=%SPACK_CC_TMP%
 
+test_pipe_error_overflow: build_and_check_test_sample
+	@echo \n
+	@echo ---------------------------
+	@echo  Pipe stderr overflow test
+	@echo ---------------------------
+	set SPACK_CC_TMP=%SPACK_CC%
+	set SPACK_CC=$(MAKEDIR)\test\lots-of-error.bat
+	cl /c /EHsc "test\src file\calc.cxx"
+	set SPACK_CC=%SPACK_CC_TMP%
+
 build_zerowrite_test: test\writezero.obj
 	link $(LFLAGS) $** $(API_LIBS) /out:writezero.exe
 
@@ -247,7 +258,7 @@ test_def_file_name_override:
 test_and_cleanup: test clean-test
 
 
-test: test_wrapper test_relocate_exe test_relocate_dll test_def_file_name_override test_exe_with_exports test_long_paths test_pipe_overflow
+test: test_wrapper test_relocate_exe test_relocate_dll test_def_file_name_override test_exe_with_exports test_long_paths test_pipe_out_overflow test_pipe_error_overflow
 
 
 clean : clean-test clean-cl

src/execute.cxx

@@ -31,6 +31,8 @@ enum : std::uint16_t { InvalidExitCode = 999 };
 ExecuteCommand::ExecuteCommand(std::string command)
     : ChildStdOut_Rd(nullptr),
       ChildStdOut_Wd(nullptr),
+      ChildStdErr_Rd(nullptr),
+      ChildStdErr_Wd(nullptr),
       base_command(std::move(command)) {
     this->CreateChildPipes();
     this->SetupExecute();
@@ -39,6 +41,8 @@ ExecuteCommand::ExecuteCommand(std::string command)
 ExecuteCommand::ExecuteCommand(std::string arg, const StrList& args)
     : ChildStdOut_Rd(nullptr),
       ChildStdOut_Wd(nullptr),
+      ChildStdErr_Rd(nullptr),
+      ChildStdErr_Wd(nullptr),
       base_command(std::move(arg)) {
     for (const auto& argp : args) {
         this->command_args.push_back(argp);
@@ -51,6 +55,8 @@ ExecuteCommand& ExecuteCommand::operator=(
     ExecuteCommand&& execute_command) noexcept {
     this->ChildStdOut_Rd = std::move(execute_command.ChildStdOut_Rd);
     this->ChildStdOut_Wd = std::move(execute_command.ChildStdOut_Wd);
+    this->ChildStdErr_Rd = std::move(execute_command.ChildStdErr_Rd);
+    this->ChildStdErr_Wd = std::move(execute_command.ChildStdErr_Wd);
     this->procInfo = std::move(execute_command.procInfo);
     this->startInfo = std::move(execute_command.startInfo);
     this->saAttr = std::move(execute_command.saAttr);
@@ -59,6 +65,7 @@ ExecuteCommand& ExecuteCommand::operator=(
     this->base_command = std::move(execute_command.base_command);
     this->command_args = std::move(execute_command.command_args);
     this->child_out_future = std::move(execute_command.child_out_future);
+    this->child_err_future = std::move(execute_command.child_err_future);
     this->exit_code_future = std::move(exit_code_future);
     return *this;
 }
@@ -76,7 +83,7 @@ void ExecuteCommand::SetupExecute() {
     // This structure specifies the STDIN and STDOUT handles for redirection.
     ZeroMemory(&si_start_info, sizeof(STARTUPINFOW));
     si_start_info.cb = sizeof(STARTUPINFOW);
-    si_start_info.hStdError = this->ChildStdOut_Wd;
+    si_start_info.hStdError = this->ChildStdErr_Wd;
     si_start_info.hStdOutput = this->ChildStdOut_Wd;
     si_start_info.dwFlags |= STARTF_USESTDHANDLES;
     this->procInfo = pi_proc_info;
@@ -162,6 +169,7 @@ bool ExecuteCommand::ExecuteToolChainChild() {
     // determine when child proc is done
     free(nc_command_line);
     CloseHandle(this->ChildStdOut_Wd);
+    CloseHandle(this->ChildStdErr_Wd);
     return true;
 }
 

test/lots-of-error.bat

@@ -0,0 +1,3 @@
+@echo OFF
+
+for /l %%x in (1, 1, 1250) do echo Test boilerplate output to fill stderr line number %%x 1>&2