There were two distinct implementations for executing a process and reading its output

Source commit: eb797b2e9e38fdbb005956549aa928e38e015062

source/resource/TextsWin.h

@@ -56,7 +56,6 @@
 #define CONSOLE_SEND_PIPE       1172
 #define WORKSPACE_NOT_FOLDER    1173
 #define FOLDER_NOT_WORKSPACE    1174
-#define SHELL_PATTERN_ERROR     1175
 #define PATH_ENV_TOO_LONG       1176
 #define STACK_TRACE             1177
 #define PUTTY_NO_SITES2         1180

source/resource/TextsWin1.rc

@@ -64,7 +64,6 @@ BEGIN
         CONSOLE_SEND_PIPE, "External console output is redirected to a pipe. Make sure the pipe is being read from."
         WORKSPACE_NOT_FOLDER, "'%s' is workspace, not site folder."
         FOLDER_NOT_WORKSPACE, "'%s' is site folder, not workspace."
-        SHELL_PATTERN_ERROR, "Error executing command \"%s\" (%s)."
         PATH_ENV_TOO_LONG, "Cannot add new path to %PATH%, %PATH% is already too long."
         STACK_TRACE, "Stack trace:"
         PUTTY_NO_SITES2, "No sites found in %s sites registry key (%s)."

source/windows/Tools.cpp

@@ -359,91 +359,101 @@ UnicodeString __fastcall StoreFormSize(TForm * Form)
   return FORMAT(L"%d,%d,%s", (Form->Width, Form->Height, SavePixelsPerInch(Form)));
 }
 //---------------------------------------------------------------------------
-static void __fastcall ExecuteProcessAndReadOutput(const
-  UnicodeString & Command, const UnicodeString & HelpKeyword, UnicodeString & Output)
+void ExecuteProcessAndReadOutput(const UnicodeString & Command, UnicodeString & Output, DWORD & ExitCode, bool ReadStdErr)
 {
-  if (!CopyCommandToClipboard(Command))
-  {
-    SECURITY_ATTRIBUTES SecurityAttributes;
-    ZeroMemory(&SecurityAttributes, sizeof(SecurityAttributes));
-    SecurityAttributes.nLength = sizeof(SECURITY_ATTRIBUTES);
-    SecurityAttributes.bInheritHandle = TRUE;
-    SecurityAttributes.lpSecurityDescriptor = NULL;
+  SECURITY_ATTRIBUTES SecurityAttributes;
+  ZeroMemory(&SecurityAttributes, sizeof(SecurityAttributes));
+  SecurityAttributes.nLength = sizeof(SECURITY_ATTRIBUTES);
+  SecurityAttributes.bInheritHandle = TRUE;
+  SecurityAttributes.lpSecurityDescriptor = NULL;
 
-    HANDLE PipeRead = INVALID_HANDLE_VALUE;
-    HANDLE PipeWrite = INVALID_HANDLE_VALUE;
+  HANDLE PipeRead = INVALID_HANDLE_VALUE;
+  HANDLE PipeWrite = INVALID_HANDLE_VALUE;
 
-    if (!CreatePipe(&PipeRead, &PipeWrite, &SecurityAttributes, 0) ||
-        !SetHandleInformation(PipeRead, HANDLE_FLAG_INHERIT, 0))
-    {
-      throw EOSExtException(FMTLOAD(EXECUTE_APP_ERROR, (Command)));
-    }
+  if (!CreatePipe(&PipeRead, &PipeWrite, &SecurityAttributes, 0) ||
+      !SetHandleInformation(PipeRead, HANDLE_FLAG_INHERIT, 0))
+  {
+    throw EOSExtException(FMTLOAD(EXECUTE_APP_ERROR, (Command)));
+  }
 
-    PROCESS_INFORMATION ProcessInformation;
-    ZeroMemory(&ProcessInformation, sizeof(ProcessInformation));
+  PROCESS_INFORMATION ProcessInformation;
+  ZeroMemory(&ProcessInformation, sizeof(ProcessInformation));
 
+  try
+  {
     try
     {
-      try
+      STARTUPINFO StartupInfo;
+      ZeroMemory(&StartupInfo, sizeof(StartupInfo));
+      StartupInfo.cb = sizeof(STARTUPINFO);
+      if (ReadStdErr)
       {
-        STARTUPINFO StartupInfo;
-        ZeroMemory(&StartupInfo, sizeof(StartupInfo));
-        StartupInfo.cb = sizeof(STARTUPINFO);
         StartupInfo.hStdError = PipeWrite;
-        StartupInfo.hStdOutput = PipeWrite;
-        StartupInfo.wShowWindow = SW_HIDE;
-        StartupInfo.dwFlags = STARTF_USESTDHANDLES | STARTF_USESHOWWINDOW;
-
-        if (!CreateProcess(NULL, Command.c_str(), NULL, NULL, TRUE, 0, NULL, NULL, &StartupInfo, &ProcessInformation))
-        {
-          throw EOSExtException(FMTLOAD(EXECUTE_APP_ERROR, (Command)));
-        }
-      }
-      __finally
-      {
-        // If we do not close the handle here, the ReadFile below would get stuck once the app finishes writting,
-        // as it still sees that someone "can" write to the pipe.
-        CloseHandle(PipeWrite);
       }
+      StartupInfo.hStdOutput = PipeWrite;
+      StartupInfo.wShowWindow = SW_HIDE;
+      StartupInfo.dwFlags = STARTF_USESTDHANDLES | STARTF_USESHOWWINDOW;
 
-      DWORD BytesAvail;
-      while (PeekNamedPipe(PipeRead, NULL, 0, NULL, &BytesAvail, NULL))
+      if (!CreateProcess(NULL, Command.c_str(), NULL, NULL, TRUE, 0, NULL, NULL, &StartupInfo, &ProcessInformation))
       {
-        if (BytesAvail > 0)
-        {
-          char Buffer[4096];
-          DWORD BytesToRead = std::min(BytesAvail, static_cast<unsigned long>(sizeof(Buffer)));
-          DWORD BytesRead;
-          if (ReadFile(PipeRead, Buffer, BytesToRead, &BytesRead, NULL))
-          {
-            Output += UnicodeString(UTF8String(Buffer, BytesRead));
-          }
-        }
-        // Same as in ExecuteShellCheckedAndWait
-        Sleep(50);
-        Application->ProcessMessages();
+        throw EOSExtException(FMTLOAD(EXECUTE_APP_ERROR, (Command)));
       }
+    }
+    __finally
+    {
+      // If we do not close the handle here, the ReadFile below would get stuck once the app finishes writting,
+      // as it still sees that someone "can" write to the pipe.
+      CloseHandle(PipeWrite);
+    }
 
-      DWORD ExitCode;
-      if (DebugAlwaysTrue(GetExitCodeProcess(ProcessInformation.hProcess, &ExitCode)) &&
-          (ExitCode != 0))
+    DWORD BytesAvail;
+    while (PeekNamedPipe(PipeRead, NULL, 0, NULL, &BytesAvail, NULL))
+    {
+      if (BytesAvail > 0)
       {
-        UnicodeString Buf = Output;
-        UnicodeString Buf2;
-        if (ExtractMainInstructions(Buf, Buf2))
-        {
-          throw ExtException(Output, UnicodeString(), HelpKeyword);
-        }
-        else
+        char Buffer[4096];
+        DWORD BytesToRead = std::min(BytesAvail, static_cast<unsigned long>(sizeof(Buffer)));
+        DWORD BytesRead;
+        if (ReadFile(PipeRead, Buffer, BytesToRead, &BytesRead, NULL))
         {
-          throw ExtException(MainInstructions(FMTLOAD(COMMAND_FAILED_CODEONLY, (static_cast<int>(ExitCode)))), Output, HelpKeyword);
+          Output += UnicodeString(UTF8String(Buffer, BytesRead));
         }
       }
+      // Same as in ExecuteShellCheckedAndWait
+      Sleep(50);
+      Application->ProcessMessages();
     }
-    __finally
+
+    DebugCheck(GetExitCodeProcess(ProcessInformation.hProcess, &ExitCode));
+  }
+  __finally
+  {
+    CloseHandle(ProcessInformation.hProcess);
+    CloseHandle(ProcessInformation.hThread);
+    CloseHandle(PipeRead);
+  }
+}
+//---------------------------------------------------------------------------
+static void __fastcall DoExecuteProcessAndReadOutput(
+  const UnicodeString & Command, const UnicodeString & HelpKeyword, UnicodeString & Output)
+{
+  if (!CopyCommandToClipboard(Command))
+  {
+    DWORD ExitCode;
+    ExecuteProcessAndReadOutput(Command, Output, ExitCode, true);
+    if (ExitCode != 0)
     {
-      CloseHandle(ProcessInformation.hProcess);
-      CloseHandle(ProcessInformation.hThread);
+      UnicodeString Buf = Output;
+      UnicodeString Buf2;
+      if (ExtractMainInstructions(Buf, Buf2))
+      {
+        throw ExtException(Output, EmptyStr, HelpKeyword);
+      }
+      else
+      {
+        UnicodeString Message = MainInstructions(FMTLOAD(COMMAND_FAILED_CODEONLY, (static_cast<int>(ExitCode))));
+        throw ExtException(Message, Output, HelpKeyword);
+      }
     }
   }
 }
@@ -457,7 +467,7 @@ void __fastcall ExecuteProcessChecked(
   }
   else
   {
-    ExecuteProcessAndReadOutput(Command, HelpKeyword, *Output);
+    DoExecuteProcessAndReadOutput(Command, HelpKeyword, *Output);
   }
 }
 //---------------------------------------------------------------------------
@@ -470,7 +480,7 @@ void __fastcall ExecuteProcessCheckedAndWait(
   }
   else
   {
-    ExecuteProcessAndReadOutput(Command, HelpKeyword, *Output);
+    DoExecuteProcessAndReadOutput(Command, HelpKeyword, *Output);
   }
 }
 //---------------------------------------------------------------------------

source/windows/Tools.h

@@ -10,6 +10,7 @@
 #include <Vcl.Graphics.hpp>
 //---------------------------------------------------------------------------
 void __fastcall CenterFormOn(TForm * Form, TControl * CenterOn);
+void ExecuteProcessAndReadOutput(const UnicodeString & Command, UnicodeString & Output, DWORD & ExitCode, bool ReadStdErr);
 void __fastcall ExecuteProcessChecked(
   const UnicodeString & Command, const UnicodeString & HelpKeyword, UnicodeString * Output);
 void __fastcall ExecuteProcessCheckedAndWait(

source/windows/WinInterface.cpp

@@ -1071,110 +1071,10 @@ void __fastcall TWinInteractiveCustomCommand::Prompt(
 void __fastcall TWinInteractiveCustomCommand::Execute(
   const UnicodeString & Command, UnicodeString & Value)
 {
-  HANDLE StdOutOutput;
-  HANDLE StdOutInput;
-  HANDLE StdInOutput;
-  HANDLE StdInInput;
-  SECURITY_ATTRIBUTES SecurityAttributes;
-  SecurityAttributes.nLength = sizeof(SecurityAttributes);
-  SecurityAttributes.lpSecurityDescriptor = NULL;
-  SecurityAttributes.bInheritHandle = TRUE;
-  try
-  {
-    if (!CreatePipe(&StdOutOutput, &StdOutInput, &SecurityAttributes, 0))
-    {
-      throw Exception(FMTLOAD(SHELL_PATTERN_ERROR, (Command, L"out")));
-    }
-    else if (!CreatePipe(&StdInOutput, &StdInInput, &SecurityAttributes, 0))
-    {
-      throw Exception(FMTLOAD(SHELL_PATTERN_ERROR, (Command, L"in")));
-    }
-    else
-    {
-      STARTUPINFO StartupInfo;
-      PROCESS_INFORMATION ProcessInformation;
-
-      FillMemory(&StartupInfo, sizeof(StartupInfo), 0);
-      StartupInfo.cb = sizeof(StartupInfo);
-      StartupInfo.wShowWindow = SW_HIDE;
-      StartupInfo.hStdInput = StdInOutput;
-      StartupInfo.hStdOutput = StdOutInput;
-      StartupInfo.dwFlags = STARTF_USESTDHANDLES | STARTF_USESHOWWINDOW;
-
-      if (!CreateProcess(NULL, Command.c_str(), &SecurityAttributes, &SecurityAttributes,
-            TRUE, NORMAL_PRIORITY_CLASS, NULL, NULL, &StartupInfo, &ProcessInformation))
-      {
-        throw Exception(FMTLOAD(SHELL_PATTERN_ERROR, (Command, L"process")));
-      }
-      else
-      {
-        try
-        {
-          // wait until the console program terminated
-          bool Running = true;
-          while (Running)
-          {
-            switch (WaitForSingleObject(ProcessInformation.hProcess, 200))
-            {
-              case WAIT_TIMEOUT:
-                Application->ProcessMessages();
-                break;
-
-              case WAIT_OBJECT_0:
-                Running = false;
-                break;
-
-              default:
-                throw Exception(FMTLOAD(SHELL_PATTERN_ERROR, (Command, L"wait")));
-            }
-          }
-
-          char Buffer[1024];
-          unsigned long Read;
-          while (PeekNamedPipe(StdOutOutput, NULL, 0, NULL, &Read, NULL) &&
-                 (Read > 0))
-
-          {
-            if (!ReadFile(StdOutOutput, &Buffer, Read, &Read, NULL))
-            {
-              throw Exception(FMTLOAD(SHELL_PATTERN_ERROR, (Command, L"read")));
-            }
-            else if (Read > 0)
-            {
-              Value += AnsiToString(Buffer, Read);
-            }
-          }
-
-          // trim trailing cr/lf
-          Value = TrimRight(Value);
-        }
-        __finally
-        {
-          CloseHandle(ProcessInformation.hProcess);
-          CloseHandle(ProcessInformation.hThread);
-        }
-      }
-    }
-  }
-  __finally
-  {
-    if (StdOutOutput != INVALID_HANDLE_VALUE)
-    {
-      CloseHandle(StdOutOutput);
-    }
-    if (StdOutInput != INVALID_HANDLE_VALUE)
-    {
-      CloseHandle(StdOutInput);
-    }
-    if (StdInOutput != INVALID_HANDLE_VALUE)
-    {
-      CloseHandle(StdInOutput);
-    }
-    if (StdInInput != INVALID_HANDLE_VALUE)
-    {
-      CloseHandle(StdInInput);
-    }
-  }
+  DWORD DummyExitCode;
+  ExecuteProcessAndReadOutput(Command, Value, DummyExitCode, false);
+  // trim trailing cr/lf
+  Value = TrimRight(Value);
 }
 //---------------------------------------------------------------------------
 void __fastcall MenuPopup(TPopupMenu * Menu, TButton * Button)