llvm dev - Jul 2009 - [LLVMdev] [PATCH 2/2] Make Program::ExecuteNoWait return a process ID.

---
 include/llvm/System/Program.h |   14 ++++++++++----
 lib/System/Unix/Program.inc   |   17 +++++++++--------
 lib/System/Win32/Program.inc  |   16 +++++++++-------
 3 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/include/llvm/System/Program.h b/include/llvm/System/Program.h
index 14f9e9e..05c73ac 100644
--- a/include/llvm/System/Program.h
+++ b/include/llvm/System/Program.h
@@ -15,6 +15,7 @@
 #define LLVM_SYSTEM_PROGRAM_H
 
 #include "llvm/System/Path.h"
+#include "llvm/Config/config.h"
 
 namespace llvm {
 namespace sys {
@@ -88,6 +89,12 @@ namespace sys {
       static bool ChangeStdoutToBinary();
     /// @}
 
+#ifdef LLVM_ON_WINDOWS
+    typedef unsigned ProcessID;
+#else
+    typedef int ProcessID;
+#endif
+
      /// This function executes the program using the \p arguments provided and
      /// waits for the program to exit. This function will block the current
      /// program until the invoked program exits. The invoked program will
@@ -95,12 +102,11 @@ namespace sys {
      /// environment and other configuration settings of the invoking program.
      /// If Path::executable() does not return true when this function is
      /// called then a std::string is thrown.
-     /// @returns an integer result code indicating the status of the program.
-     /// A zero or positive value indicates the result code of the program. A
-     /// negative value is the signal number on which it terminated.
+     /// @returns the child process's process ID on success, or 0 if the
+     /// invocation failed.
      /// @see FindProgrambyName
      /// @brief Executes the program with the given set of \p args.
-     static void ExecuteNoWait(
+     static ProcessID ExecuteNoWait(
         const Path& path,  ///< sys::Path object providing the path of
the
         ///< program to be executed. It is presumed this is the result of
         ///< the FindProgramByName method.
diff --git a/lib/System/Unix/Program.inc b/lib/System/Unix/Program.inc
index 342b45c..15f6e3f 100644
--- a/lib/System/Unix/Program.inc
+++ b/lib/System/Unix/Program.inc
@@ -274,7 +274,7 @@ Program::ExecuteAndWait(const Path& path,
 
 }
 
-void
+Program::ProcessID
 Program::ExecuteNoWait(const Path& path,
                        const char** args,
                        const char** envp,
@@ -285,36 +285,36 @@ Program::ExecuteNoWait(const Path& path,
   if (!path.canExecute()) {
     if (ErrMsg)
       *ErrMsg = path.toString() + " is not executable";
-    return;
+    return 0;
   }
 
   // Create a child process.
-  int child = fork();
+  ProcessID child = fork();
   switch (child) {
     // An error occured:  Return to the caller.
     case -1:
       MakeErrMsg(ErrMsg, "Couldn't fork");
-      return;
+      return 0;
 
     // Child process: Execute the program.
     case 0: {
       // Redirect file descriptors...
       if (redirects) {
         // Redirect stdin
-        if (RedirectIO(redirects[0], 0, ErrMsg)) { return; }
+        if (RedirectIO(redirects[0], 0, ErrMsg)) { return 0; }
         // Redirect stdout
-        if (RedirectIO(redirects[1], 1, ErrMsg)) { return; }
+        if (RedirectIO(redirects[1], 1, ErrMsg)) { return 0; }
         if (redirects[1] && redirects[2] &&
             *(redirects[1]) == *(redirects[2])) {
           // If stdout and stderr should go to the same place, redirect stderr
           // to the FD already open for stdout.
           if (-1 == dup2(1,2)) {
             MakeErrMsg(ErrMsg, "Can't redirect stderr to
stdout");
-            return;
+            return 0;
           }
         } else {
           // Just redirect stderr
-          if (RedirectIO(redirects[2], 2, ErrMsg)) { return; }
+          if (RedirectIO(redirects[2], 2, ErrMsg)) { return 0; }
         }
       }
 
@@ -344,6 +344,7 @@ Program::ExecuteNoWait(const Path& path,
   fsync(1);
   fsync(2);
 
+  return child;
 }
 
 bool Program::ChangeStdinToBinary(){
diff --git a/lib/System/Win32/Program.inc b/lib/System/Win32/Program.inc
index 71f686c..4577737 100644
--- a/lib/System/Win32/Program.inc
+++ b/lib/System/Win32/Program.inc
@@ -303,7 +303,7 @@ Program::ExecuteAndWait(const Path& path,
   return status;
 }
 
-void
+Program::ProcessID
 Program::ExecuteNoWait(const Path& path,
                        const char** args,
                        const char** envp,
@@ -313,7 +313,7 @@ Program::ExecuteNoWait(const Path& path,
   if (!path.canExecute()) {
     if (ErrMsg)
       *ErrMsg = "program not executable";
-    return;
+    return 0;
   }
 
   // Windows wants a command line, not an array of args, to pass to the new
@@ -388,13 +388,13 @@ Program::ExecuteNoWait(const Path& path,
     si.hStdInput = RedirectIO(redirects[0], 0, ErrMsg);
     if (si.hStdInput == INVALID_HANDLE_VALUE) {
       MakeErrMsg(ErrMsg, "can't redirect stdin");
-      return;
+      return 0;
     }
     si.hStdOutput = RedirectIO(redirects[1], 1, ErrMsg);
     if (si.hStdOutput == INVALID_HANDLE_VALUE) {
       CloseHandle(si.hStdInput);
       MakeErrMsg(ErrMsg, "can't redirect stdout");
-      return;
+      return 0;
     }
     if (redirects[1] && redirects[2] && *(redirects[1]) ==
*(redirects[2])) {
       // If stdout and stderr should go to the same place, redirect stderr
@@ -409,7 +409,7 @@ Program::ExecuteNoWait(const Path& path,
         CloseHandle(si.hStdInput);
         CloseHandle(si.hStdOutput);
         MakeErrMsg(ErrMsg, "can't redirect stderr");
-        return;
+        return 0;
       }
     }
   }
@@ -435,7 +435,7 @@ Program::ExecuteNoWait(const Path& path,
     SetLastError(err);
     MakeErrMsg(ErrMsg, std::string("Couldn't execute program
'") +
                path.toString() + "'");
-    return;
+    return 0;
   }
 
   // Make sure these get closed no matter what.
@@ -463,9 +463,11 @@ Program::ExecuteNoWait(const Path& path,
       MakeErrMsg(ErrMsg, std::string("Unable to set memory limit"));
       TerminateProcess(pi.hProcess, 1);
       WaitForSingleObject(pi.hProcess, INFINITE);
-      return;
+      return 0;
     }
   }
+
+  return pi.dwProcessId;
 }
 
 bool Program::ChangeStdinToBinary(){
-- 
1.6.3.3

Hi Daniel,

Daniel Dunbar <daniel <at> zuster.org> writes:
> 
> Ultimately I think a better API would be to provide a generic class
> which represents an executed operating system process, and includes
> operations to wait for its completion, redirect its IO, communicate
> with it, etc. This would be a big improvement over the current
> monolithic function.
I agree, but my application needs access to the process ID. So if
I'll start implementing such class, it'll have a getPID()
method or similar :)