--- include/llvm/System/Program.h | 8 ++++---- lib/System/Unix/Program.inc | 30 +++++++++++++++--------------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/include/llvm/System/Program.h b/include/llvm/System/Program.h index 49de7cf..14f9e9e 100644 --- a/include/llvm/System/Program.h +++ b/include/llvm/System/Program.h @@ -97,12 +97,12 @@ namespace sys { /// 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. + /// negative value is the signal number on which it terminated. /// @see FindProgrambyName /// @brief Executes the program with the given set of \p args. static void ExecuteNoWait( - const Path& path, ///< sys::Path object providing the path of the - ///< program to be executed. It is presumed this is the result of + 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. const char** args, ///< A vector of strings that are passed to the ///< program. The first element should be the name of the program. @@ -120,7 +120,7 @@ namespace sys { ///< higher limit, the child is killed and this call returns. If zero - ///< no memory limit. std::string* ErrMsg = 0 ///< If non-zero, provides a pointer to a string - ///< instance in which error messages will be returned. If the string + ///< instance in which error messages will be returned. If the string ///< is non-empty upon return an error occurred while invoking the ///< program. ); diff --git a/lib/System/Unix/Program.inc b/lib/System/Unix/Program.inc index 7e196b0..342b45c 100644 --- a/lib/System/Unix/Program.inc +++ b/lib/System/Unix/Program.inc @@ -1,10 +1,10 @@ //===- llvm/System/Unix/Program.cpp -----------------------------*- C++ -*-===// -// +// // The LLVM Compiler Infrastructure // // This file is distributed under the University of Illinois Open Source // License. See LICENSE.TXT for details. -// +// //===----------------------------------------------------------------------===// // // This file implements the Unix specific portion of the Program class. @@ -51,10 +51,10 @@ Program::FindProgramByName(const std::string& progName) { return temp; // At this point, the file name is valid and its not executable - + // Get the path. If its empty, we can't do anything to find it. const char *PathStr = getenv("PATH"); - if (PathStr == 0) + if (PathStr == 0) return Path(); // Now we have a colon separated list of directories to search; try them. @@ -142,14 +142,14 @@ static void SetMemoryLimits (unsigned size) #endif } -int -Program::ExecuteAndWait(const Path& path, +int +Program::ExecuteAndWait(const Path& path, const char** args, const char** envp, const Path** redirects, unsigned secondsToWait, unsigned memoryLimit, - std::string* ErrMsg) + std::string* ErrMsg) { if (!path.canExecute()) { if (ErrMsg) @@ -174,7 +174,7 @@ Program::ExecuteAndWait(const Path& path, if (RedirectIO(redirects[0], 0, ErrMsg)) { return -1; } // Redirect stdout if (RedirectIO(redirects[1], 1, ErrMsg)) { return -1; } - if (redirects[1] && redirects[2] && + 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. @@ -192,7 +192,7 @@ Program::ExecuteAndWait(const Path& path, if (memoryLimit!=0) { SetMemoryLimits(memoryLimit); } - + // Execute! if (envp != 0) execve (path.c_str(), (char**)args, (char**)envp); @@ -233,7 +233,7 @@ Program::ExecuteAndWait(const Path& path, if (secondsToWait && errno == EINTR) { // Kill the child. kill(child, SIGKILL); - + // Turn off the alarm and restore the signal handler alarm(0); sigaction(SIGALRM, &Old, 0); @@ -271,16 +271,16 @@ Program::ExecuteAndWait(const Path& path, #else return -99; #endif - + } void -Program::ExecuteNoWait(const Path& path, +Program::ExecuteNoWait(const Path& path, const char** args, const char** envp, const Path** redirects, unsigned memoryLimit, - std::string* ErrMsg) + std::string* ErrMsg) { if (!path.canExecute()) { if (ErrMsg) @@ -304,7 +304,7 @@ Program::ExecuteNoWait(const Path& path, if (RedirectIO(redirects[0], 0, ErrMsg)) { return; } // Redirect stdout if (RedirectIO(redirects[1], 1, ErrMsg)) { return; } - if (redirects[1] && redirects[2] && + 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. @@ -322,7 +322,7 @@ Program::ExecuteNoWait(const Path& path, if (memoryLimit!=0) { SetMemoryLimits(memoryLimit); } - + // Execute! if (envp != 0) execve (path.c_str(), (char**)args, (char**)envp); -- 1.6.3.3
Mikhail Glushenkov
2009-Jul-17 01:04 UTC
[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
Yes, please! - Daniel On Thu, Jul 16, 2009 at 6:04 PM, Mikhail Glushenkov<foldr at codedgers.com> wrote:> --- > include/llvm/System/Program.h | 8 ++++---- > lib/System/Unix/Program.inc | 30 +++++++++++++++--------------- > 2 files changed, 19 insertions(+), 19 deletions(-) > > diff --git a/include/llvm/System/Program.h b/include/llvm/System/Program.h > index 49de7cf..14f9e9e 100644 > --- a/include/llvm/System/Program.h > +++ b/include/llvm/System/Program.h > @@ -97,12 +97,12 @@ namespace sys { > /// 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. > + /// negative value is the signal number on which it terminated. > /// @see FindProgrambyName > /// @brief Executes the program with the given set of \p args. > static void ExecuteNoWait( > - const Path& path, ///< sys::Path object providing the path of the > - ///< program to be executed. It is presumed this is the result of > + 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. > const char** args, ///< A vector of strings that are passed to the > ///< program. The first element should be the name of the program. > @@ -120,7 +120,7 @@ namespace sys { > ///< higher limit, the child is killed and this call returns. If zero - > ///< no memory limit. > std::string* ErrMsg = 0 ///< If non-zero, provides a pointer to a string > - ///< instance in which error messages will be returned. If the string > + ///< instance in which error messages will be returned. If the string > ///< is non-empty upon return an error occurred while invoking the > ///< program. > ); > diff --git a/lib/System/Unix/Program.inc b/lib/System/Unix/Program.inc > index 7e196b0..342b45c 100644 > --- a/lib/System/Unix/Program.inc > +++ b/lib/System/Unix/Program.inc > @@ -1,10 +1,10 @@ > //===- llvm/System/Unix/Program.cpp -----------------------------*- C++ -*-===// > -// > +// > // The LLVM Compiler Infrastructure > // > // This file is distributed under the University of Illinois Open Source > // License. See LICENSE.TXT for details. > -// > +// > //===----------------------------------------------------------------------===// > // > // This file implements the Unix specific portion of the Program class. > @@ -51,10 +51,10 @@ Program::FindProgramByName(const std::string& progName) { > return temp; > > // At this point, the file name is valid and its not executable > - > + > // Get the path. If its empty, we can't do anything to find it. > const char *PathStr = getenv("PATH"); > - if (PathStr == 0) > + if (PathStr == 0) > return Path(); > > // Now we have a colon separated list of directories to search; try them. > @@ -142,14 +142,14 @@ static void SetMemoryLimits (unsigned size) > #endif > } > > -int > -Program::ExecuteAndWait(const Path& path, > +int > +Program::ExecuteAndWait(const Path& path, > const char** args, > const char** envp, > const Path** redirects, > unsigned secondsToWait, > unsigned memoryLimit, > - std::string* ErrMsg) > + std::string* ErrMsg) > { > if (!path.canExecute()) { > if (ErrMsg) > @@ -174,7 +174,7 @@ Program::ExecuteAndWait(const Path& path, > if (RedirectIO(redirects[0], 0, ErrMsg)) { return -1; } > // Redirect stdout > if (RedirectIO(redirects[1], 1, ErrMsg)) { return -1; } > - if (redirects[1] && redirects[2] && > + 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. > @@ -192,7 +192,7 @@ Program::ExecuteAndWait(const Path& path, > if (memoryLimit!=0) { > SetMemoryLimits(memoryLimit); > } > - > + > // Execute! > if (envp != 0) > execve (path.c_str(), (char**)args, (char**)envp); > @@ -233,7 +233,7 @@ Program::ExecuteAndWait(const Path& path, > if (secondsToWait && errno == EINTR) { > // Kill the child. > kill(child, SIGKILL); > - > + > // Turn off the alarm and restore the signal handler > alarm(0); > sigaction(SIGALRM, &Old, 0); > @@ -271,16 +271,16 @@ Program::ExecuteAndWait(const Path& path, > #else > return -99; > #endif > - > + > } > > void > -Program::ExecuteNoWait(const Path& path, > +Program::ExecuteNoWait(const Path& path, > const char** args, > const char** envp, > const Path** redirects, > unsigned memoryLimit, > - std::string* ErrMsg) > + std::string* ErrMsg) > { > if (!path.canExecute()) { > if (ErrMsg) > @@ -304,7 +304,7 @@ Program::ExecuteNoWait(const Path& path, > if (RedirectIO(redirects[0], 0, ErrMsg)) { return; } > // Redirect stdout > if (RedirectIO(redirects[1], 1, ErrMsg)) { return; } > - if (redirects[1] && redirects[2] && > + 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. > @@ -322,7 +322,7 @@ Program::ExecuteNoWait(const Path& path, > if (memoryLimit!=0) { > SetMemoryLimits(memoryLimit); > } > - > + > // Execute! > if (envp != 0) > execve (path.c_str(), (char**)args, (char**)envp); > -- > 1.6.3.3 > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >
Daniel Dunbar
2009-Jul-17 02:27 UTC
[LLVMdev] [PATCH 2/2] Make Program::ExecuteNoWait return a process ID.
I don't think this is the right direction for the system library to go. The System library is supposed to expose generic OS independent interfaces, not be a generic way for clients to get OS information. 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. That said, designing and implementing that is more work. However, I'd rather either see that done, or clients do their own process stuff, than have the llvm::sys API expose operating system dependent information. - Daniel On Thu, Jul 16, 2009 at 6:04 PM, Mikhail Glushenkov<foldr at codedgers.com> wrote:> --- > 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 > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >
Apparently Analagous Threads
- [LLVMdev] [PATCH 1/2] Trailing whitespace.
- [LLVMdev] [PATCH 2/2] Make Program::ExecuteNoWait return a process ID.
- [LLVMdev] [PATCH 2/2] Make Program::ExecuteNoWait return a process ID.
- [LLVMdev] [PATCH 2/2] Make Program::ExecuteNoWait return a process ID.
- [LLVMdev] [llvm-commits] [llvm] r89765 - in /llvm/trunk: include/llvm/System/Path.h lib/System/Unix/Path.inc lib/System/Win32/Path.inc