From 3140619c63d205d79b8eac1d88afa918981fa258 Mon Sep 17 00:00:00 2001 From: Mikhail Glushenkov Date: Sat, 18 Jul 2009 21:43:12 +0000 Subject: [PATCH] Remove duplication in Program::Execute{And,No}Wait. Implemented by moving the code out of static functions into methods of Program class. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@76340 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/System/Program.h | 197 ++++++++++++++++-------------- lib/System/Program.cpp | 27 ++++ lib/System/Unix/Program.inc | 116 +++++------------- lib/System/Win32/Program.inc | 224 ++++++---------------------------- 4 files changed, 195 insertions(+), 369 deletions(-) diff --git a/include/llvm/System/Program.h b/include/llvm/System/Program.h index 14f9e9ecfed..6b4c3a2062a 100644 --- a/include/llvm/System/Program.h +++ b/include/llvm/System/Program.h @@ -19,6 +19,9 @@ namespace llvm { namespace sys { + // TODO: Add operations to communicate with the process, redirect its I/O, + // etc. + /// This class provides an abstraction for programs that are executable by the /// operating system. It provides a platform generic way to find executable /// programs from the path and to execute them in various ways. The sys::Path @@ -26,104 +29,112 @@ namespace sys { /// @since 1.4 /// @brief An abstraction for finding and executing programs. class Program { + unsigned Pid_; + + // Noncopyable. + Program(const Program& other); + Program& operator=(const Program& other); + /// @name Methods /// @{ - public: - /// This static constructor (factory) will attempt to locate a program in - /// the operating system's file system using some pre-determined set of - /// locations to search (e.g. the PATH on Unix). - /// @returns A Path object initialized to the path of the program or a - /// Path object that is empty (invalid) if the program could not be found. - /// @throws nothing - /// @brief Construct a Program by finding it by name. - static Path FindProgramByName(const std::string& name); - - /// 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 - /// inherit the stdin, stdout, and stderr file descriptors, the - /// 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. - /// @see FindProgrambyName - /// @brief Executes the program with the given set of \p args. - static int ExecuteAndWait( - 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. - ///< The list *must* be terminated by a null char* entry. - const char ** env = 0, ///< An optional vector of strings to use for - ///< the program's environment. If not provided, the current program's - ///< environment will be used. - const sys::Path** redirects = 0, ///< An optional array of pointers to - ///< Paths. If the array is null, no redirection is done. The array - ///< should have a size of at least three. If the pointer in the array - ///< are not null, then the inferior process's stdin(0), stdout(1), - ///< and stderr(2) will be redirected to the corresponding Paths. - ///< When an empty Path is passed in, the corresponding file - ///< descriptor will be disconnected (ie, /dev/null'd) in a portable - ///< way. - unsigned secondsToWait = 0, ///< If non-zero, this specifies the amount - ///< of time to wait for the child process to exit. If the time - ///< expires, the child is killed and this call returns. If zero, - ///< this function will wait until the child finishes or forever if - ///< it doesn't. - unsigned memoryLimit = 0, ///< If non-zero, this specifies max. amount - ///< of memory can be allocated by process. If memory usage will be - ///< 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 - ///< is non-empty upon return an error occurred while invoking the - ///< program. + public: + + Program() : Pid_(0) + {} + + /// This function executes the program using the \p arguments provided. The + /// invoked program will inherit the stdin, stdout, and stderr file + /// descriptors, the 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 false in case of error, true otherwise. + /// @see FindProgramByName + /// @brief Executes the program with the given set of \p args. + bool Execute + ( 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. + ///< The list *must* be terminated by a null char* entry. + const char ** env = 0, ///< An optional vector of strings to use for + ///< the program's environment. If not provided, the current program's + ///< environment will be used. + const sys::Path** redirects = 0, ///< An optional array of pointers to + ///< Paths. If the array is null, no redirection is done. The array + ///< should have a size of at least three. If the pointer in the array + ///< are not null, then the inferior process's stdin(0), stdout(1), + ///< and stderr(2) will be redirected to the corresponding Paths. + ///< When an empty Path is passed in, the corresponding file + ///< descriptor will be disconnected (ie, /dev/null'd) in a portable + ///< way. + unsigned memoryLimit = 0, ///< If non-zero, this specifies max. amount + ///< of memory can be allocated by process. If memory usage will be + ///< 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 + ///< is non-empty upon return an error occurred while invoking the + ///< program. + ); + + /// This function waits for the program to exit. This function will block + /// the current program until the invoked program exits. + /// @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. + /// @see Execute + /// @brief Waits for the program to exit. + int Wait + ( unsigned secondsToWait = 0, ///< If non-zero, this specifies the amount + ///< of time to wait for the child process to exit. If the time + ///< expires, the child is killed and this call returns. If zero, + ///< this function will wait until the child finishes or forever if + ///< it doesn't. + std::string* ErrMsg = 0 ///< If non-zero, provides a pointer to a string + ///< instance in which error messages will be returned. If the string + ///< is non-empty upon return an error occurred while invoking the + ///< program. ); - // These methods change the specified standard stream (stdin or stdout) to - // binary mode. They return true if an error occurred - static bool ChangeStdinToBinary(); - static bool ChangeStdoutToBinary(); + + /// This static constructor (factory) will attempt to locate a program in + /// the operating system's file system using some pre-determined set of + /// locations to search (e.g. the PATH on Unix). + /// @returns A Path object initialized to the path of the program or a + /// Path object that is empty (invalid) if the program could not be found. + /// @throws nothing + /// @brief Construct a Program by finding it by name. + static Path FindProgramByName(const std::string& name); + + // These methods change the specified standard stream (stdin or stdout) to + // binary mode. They return true if an error occurred + static bool ChangeStdinToBinary(); + static bool ChangeStdoutToBinary(); + + /// A convenience function equivalent to Program prg; prg.Execute(..); + /// prg.Wait(..); + /// @throws nothing + /// @see Execute, Wait + static int ExecuteAndWait(const Path& path, + const char** args, + const char ** env = 0, + const sys::Path** redirects = 0, + unsigned secondsToWait = 0, + unsigned memoryLimit = 0, + std::string* ErrMsg = 0); + + /// A convenience function equivalent to Program prg; prg.Execute(..); + /// @throws nothing + /// @see Execute + static void ExecuteNoWait(const Path& path, + const char** args, + const char ** env = 0, + const sys::Path** redirects = 0, + unsigned memoryLimit = 0, + std::string* ErrMsg = 0); + /// @} - /// 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 - /// inherit the stdin, stdout, and stderr file descriptors, the - /// 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. - /// @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 - ///< 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. - ///< The list *must* be terminated by a null char* entry. - const char ** env = 0, ///< An optional vector of strings to use for - ///< the program's environment. If not provided, the current program's - ///< environment will be used. - const sys::Path** redirects = 0, ///< An optional array of pointers to - ///< Paths. If the array is null, no redirection is done. The array - ///< should have a size of at least three. If the pointer in the array - ///< are not null, then the inferior process's stdin(0), stdout(1), - ///< and stderr(2) will be redirected to the corresponding Paths. - unsigned memoryLimit = 0, ///< If non-zero, this specifies max. amount - ///< of memory can be allocated by process. If memory usage will be - ///< 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 - ///< is non-empty upon return an error occurred while invoking the - ///< program. - ); }; } } diff --git a/lib/System/Program.cpp b/lib/System/Program.cpp index eb289d81b2e..a3049d46fd6 100644 --- a/lib/System/Program.cpp +++ b/lib/System/Program.cpp @@ -22,6 +22,33 @@ using namespace sys; //=== independent code. //===----------------------------------------------------------------------===// +int +Program::ExecuteAndWait(const Path& path, + const char** args, + const char** envp, + const Path** redirects, + unsigned secondsToWait, + unsigned memoryLimit, + std::string* ErrMsg) { + Program prg; + if (prg.Execute(path, args, envp, redirects, memoryLimit, ErrMsg)) + return prg.Wait(secondsToWait, ErrMsg); + else + return -1; +} + +void +Program::ExecuteNoWait(const Path& path, + const char** args, + const char** envp, + const Path** redirects, + unsigned memoryLimit, + std::string* ErrMsg) { + Program prg; + prg.Execute(path, args, envp, redirects, memoryLimit, ErrMsg); +} + + } // Include the platform-specific parts of this class. diff --git a/lib/System/Unix/Program.inc b/lib/System/Unix/Program.inc index 342b45cdd34..d4e88fd6d13 100644 --- a/lib/System/Unix/Program.inc +++ b/lib/System/Unix/Program.inc @@ -142,49 +142,47 @@ static void SetMemoryLimits (unsigned size) #endif } -int -Program::ExecuteAndWait(const Path& path, - const char** args, - const char** envp, - const Path** redirects, - unsigned secondsToWait, - unsigned memoryLimit, - std::string* ErrMsg) +bool +Program::Execute(const Path& path, + const char** args, + const char** envp, + const Path** redirects, + unsigned memoryLimit, + std::string* ErrMsg) { if (!path.canExecute()) { if (ErrMsg) *ErrMsg = path.toString() + " is not executable"; - return -1; + return false; } -#ifdef HAVE_SYS_WAIT_H // Create a child process. int child = fork(); switch (child) { // An error occured: Return to the caller. case -1: MakeErrMsg(ErrMsg, "Couldn't fork"); - return -1; + return false; // Child process: Execute the program. case 0: { // Redirect file descriptors... if (redirects) { // Redirect stdin - if (RedirectIO(redirects[0], 0, ErrMsg)) { return -1; } + if (RedirectIO(redirects[0], 0, ErrMsg)) { return false; } // Redirect stdout - if (RedirectIO(redirects[1], 1, ErrMsg)) { return -1; } + if (RedirectIO(redirects[1], 1, ErrMsg)) { return false; } 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 -1; + return false; } } else { // Just redirect stderr - if (RedirectIO(redirects[2], 2, ErrMsg)) { return -1; } + if (RedirectIO(redirects[2], 2, ErrMsg)) { return false; } } } @@ -214,8 +212,23 @@ Program::ExecuteAndWait(const Path& path, fsync(1); fsync(2); + Pid_ = child; + + return true; +} + +int +Program::Wait(unsigned secondsToWait, + std::string* ErrMsg) +{ +#ifdef HAVE_SYS_WAIT_H struct sigaction Act, Old; + if (Pid_ == 0) { + MakeErrMsg(ErrMsg, "Process not started!"); + return -1; + } + // Install a timeout handler. if (secondsToWait) { Timeout = false; @@ -229,6 +242,7 @@ Program::ExecuteAndWait(const Path& path, // Parent process: Wait for the child process to terminate. int status; + int child = this->Pid_; while (wait(&status) != child) if (secondsToWait && errno == EINTR) { // Kill the child. @@ -274,78 +288,6 @@ Program::ExecuteAndWait(const Path& path, } -void -Program::ExecuteNoWait(const Path& path, - const char** args, - const char** envp, - const Path** redirects, - unsigned memoryLimit, - std::string* ErrMsg) -{ - if (!path.canExecute()) { - if (ErrMsg) - *ErrMsg = path.toString() + " is not executable"; - return; - } - - // Create a child process. - int child = fork(); - switch (child) { - // An error occured: Return to the caller. - case -1: - MakeErrMsg(ErrMsg, "Couldn't fork"); - return; - - // Child process: Execute the program. - case 0: { - // Redirect file descriptors... - if (redirects) { - // Redirect stdin - if (RedirectIO(redirects[0], 0, ErrMsg)) { return; } - // Redirect stdout - if (RedirectIO(redirects[1], 1, ErrMsg)) { return; } - 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; - } - } else { - // Just redirect stderr - if (RedirectIO(redirects[2], 2, ErrMsg)) { return; } - } - } - - // Set memory limits - if (memoryLimit!=0) { - SetMemoryLimits(memoryLimit); - } - - // Execute! - if (envp != 0) - execve (path.c_str(), (char**)args, (char**)envp); - else - execv (path.c_str(), (char**)args); - // If the execve() failed, we should exit and let the parent pick up - // our non-zero exit status. - exit (errno); - } - - // Parent process: Break out of the switch to do our processing. - default: - break; - } - - // Make sure stderr and stdout have been flushed - std::cerr << std::flush; - std::cout << std::flush; - fsync(1); - fsync(2); - -} - bool Program::ChangeStdinToBinary(){ // Do nothing, as Unix doesn't differentiate between text and binary. return false; diff --git a/lib/System/Win32/Program.inc b/lib/System/Win32/Program.inc index 71f686ca02d..7dbfb3e6323 100644 --- a/lib/System/Win32/Program.inc +++ b/lib/System/Win32/Program.inc @@ -109,18 +109,17 @@ static HANDLE RedirectIO(const Path *path, int fd, std::string* ErrMsg) { DWORD cbJobObjectInfoLength); #endif -int -Program::ExecuteAndWait(const Path& path, - const char** args, - const char** envp, - const Path** redirects, - unsigned secondsToWait, - unsigned memoryLimit, - std::string* ErrMsg) { +bool +Program::Execute(const Path& path, + const char** args, + const char** envp, + const Path** redirects, + unsigned memoryLimit, + std::string* ErrMsg) { if (!path.canExecute()) { if (ErrMsg) *ErrMsg = "program not executable"; - return -1; + return false; } // Windows wants a command line, not an array of args, to pass to the new @@ -195,13 +194,13 @@ Program::ExecuteAndWait(const Path& path, si.hStdInput = RedirectIO(redirects[0], 0, ErrMsg); if (si.hStdInput == INVALID_HANDLE_VALUE) { MakeErrMsg(ErrMsg, "can't redirect stdin"); - return -1; + return false; } si.hStdOutput = RedirectIO(redirects[1], 1, ErrMsg); if (si.hStdOutput == INVALID_HANDLE_VALUE) { CloseHandle(si.hStdInput); MakeErrMsg(ErrMsg, "can't redirect stdout"); - return -1; + return false; } if (redirects[1] && redirects[2] && *(redirects[1]) == *(redirects[2])) { // If stdout and stderr should go to the same place, redirect stderr @@ -216,7 +215,7 @@ Program::ExecuteAndWait(const Path& path, CloseHandle(si.hStdInput); CloseHandle(si.hStdOutput); MakeErrMsg(ErrMsg, "can't redirect stderr"); - return -1; + return false; } } } @@ -242,8 +241,9 @@ Program::ExecuteAndWait(const Path& path, SetLastError(err); MakeErrMsg(ErrMsg, std::string("Couldn't execute program '") + path.toString() + "'"); - return -1; + return false; } + Pid_ = pi.dwProcessId; // Make sure these get closed no matter what. AutoHandle hProcess(pi.hProcess); @@ -270,204 +270,50 @@ Program::ExecuteAndWait(const Path& path, MakeErrMsg(ErrMsg, std::string("Unable to set memory limit")); TerminateProcess(pi.hProcess, 1); WaitForSingleObject(pi.hProcess, INFINITE); - return -1; + return false; } } - // Wait for it to terminate. + return true; +} + +int +Program::Wait(unsigned secondsToWait, + std::string* ErrMsg) { + if (Pid_ == 0) { + MakeErrMsg(ErrMsg, "Process not started!"); + return -1; + } + + AutoHandle hProcess = OpenProcess(SYNCHRONIZE, FALSE, Pid_); + + // Wait for the process to terminate. DWORD millisecondsToWait = INFINITE; if (secondsToWait > 0) millisecondsToWait = secondsToWait * 1000; - if (WaitForSingleObject(pi.hProcess, millisecondsToWait) == WAIT_TIMEOUT) { - if (!TerminateProcess(pi.hProcess, 1)) { - MakeErrMsg(ErrMsg, std::string("Failed to terminate timed-out program '") - + path.toString() + "'"); + if (WaitForSingleObject(hProcess, millisecondsToWait) == WAIT_TIMEOUT) { + if (!TerminateProcess(hProcess, 1)) { + MakeErrMsg(ErrMsg, "Failed to terminate timed-out program."); return -1; } - WaitForSingleObject(pi.hProcess, INFINITE); + WaitForSingleObject(hProcess, INFINITE); } // Get its exit status. DWORD status; - rc = GetExitCodeProcess(pi.hProcess, &status); - err = GetLastError(); + BOOL rc = GetExitCodeProcess(hProcess, &status); + DWORD err = GetLastError(); if (!rc) { SetLastError(err); - MakeErrMsg(ErrMsg, std::string("Failed getting status for program '") + - path.toString() + "'"); + MakeErrMsg(ErrMsg, "Failed getting status for program."); return -1; } return status; } -void -Program::ExecuteNoWait(const Path& path, - const char** args, - const char** envp, - const Path** redirects, - unsigned memoryLimit, - std::string* ErrMsg) { - if (!path.canExecute()) { - if (ErrMsg) - *ErrMsg = "program not executable"; - return; - } - - // Windows wants a command line, not an array of args, to pass to the new - // process. We have to concatenate them all, while quoting the args that - // have embedded spaces. - - // First, determine the length of the command line. - unsigned len = 0; - for (unsigned i = 0; args[i]; i++) { - len += strlen(args[i]) + 1; - if (strchr(args[i], ' ')) - len += 2; - } - - // Now build the command line. - char *command = reinterpret_cast(_alloca(len+1)); - char *p = command; - - for (unsigned i = 0; args[i]; i++) { - const char *arg = args[i]; - size_t len = strlen(arg); - bool needsQuoting = strchr(arg, ' ') != 0; - if (needsQuoting) - *p++ = '"'; - memcpy(p, arg, len); - p += len; - if (needsQuoting) - *p++ = '"'; - *p++ = ' '; - } - - *p = 0; - - // The pointer to the environment block for the new process. - char *envblock = 0; - - if (envp) { - // An environment block consists of a null-terminated block of - // null-terminated strings. Convert the array of environment variables to - // an environment block by concatenating them. - - // First, determine the length of the environment block. - len = 0; - for (unsigned i = 0; envp[i]; i++) - len += strlen(envp[i]) + 1; - - // Now build the environment block. - envblock = reinterpret_cast(_alloca(len+1)); - p = envblock; - - for (unsigned i = 0; envp[i]; i++) { - const char *ev = envp[i]; - size_t len = strlen(ev) + 1; - memcpy(p, ev, len); - p += len; - } - - *p = 0; - } - - // Create a child process. - STARTUPINFO si; - memset(&si, 0, sizeof(si)); - si.cb = sizeof(si); - si.hStdInput = INVALID_HANDLE_VALUE; - si.hStdOutput = INVALID_HANDLE_VALUE; - si.hStdError = INVALID_HANDLE_VALUE; - - if (redirects) { - si.dwFlags = STARTF_USESTDHANDLES; - - si.hStdInput = RedirectIO(redirects[0], 0, ErrMsg); - if (si.hStdInput == INVALID_HANDLE_VALUE) { - MakeErrMsg(ErrMsg, "can't redirect stdin"); - return; - } - si.hStdOutput = RedirectIO(redirects[1], 1, ErrMsg); - if (si.hStdOutput == INVALID_HANDLE_VALUE) { - CloseHandle(si.hStdInput); - MakeErrMsg(ErrMsg, "can't redirect stdout"); - return; - } - if (redirects[1] && redirects[2] && *(redirects[1]) == *(redirects[2])) { - // If stdout and stderr should go to the same place, redirect stderr - // to the handle already open for stdout. - DuplicateHandle(GetCurrentProcess(), si.hStdOutput, - GetCurrentProcess(), &si.hStdError, - 0, TRUE, DUPLICATE_SAME_ACCESS); - } else { - // Just redirect stderr - si.hStdError = RedirectIO(redirects[2], 2, ErrMsg); - if (si.hStdError == INVALID_HANDLE_VALUE) { - CloseHandle(si.hStdInput); - CloseHandle(si.hStdOutput); - MakeErrMsg(ErrMsg, "can't redirect stderr"); - return; - } - } - } - - PROCESS_INFORMATION pi; - memset(&pi, 0, sizeof(pi)); - - fflush(stdout); - fflush(stderr); - BOOL rc = CreateProcess(path.c_str(), command, NULL, NULL, TRUE, 0, - envblock, NULL, &si, &pi); - DWORD err = GetLastError(); - - // Regardless of whether the process got created or not, we are done with - // the handles we created for it to inherit. - CloseHandle(si.hStdInput); - CloseHandle(si.hStdOutput); - CloseHandle(si.hStdError); - - // Now return an error if the process didn't get created. - if (!rc) - { - SetLastError(err); - MakeErrMsg(ErrMsg, std::string("Couldn't execute program '") + - path.toString() + "'"); - return; - } - - // Make sure these get closed no matter what. - AutoHandle hProcess(pi.hProcess); - AutoHandle hThread(pi.hThread); - - // Assign the process to a job if a memory limit is defined. - AutoHandle hJob(0); - if (memoryLimit != 0) { - hJob = CreateJobObject(0, 0); - bool success = false; - if (hJob != 0) { - JOBOBJECT_EXTENDED_LIMIT_INFORMATION jeli; - memset(&jeli, 0, sizeof(jeli)); - jeli.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_PROCESS_MEMORY; - jeli.ProcessMemoryLimit = uintptr_t(memoryLimit) * 1048576; - if (SetInformationJobObject(hJob, JobObjectExtendedLimitInformation, - &jeli, sizeof(jeli))) { - if (AssignProcessToJobObject(hJob, pi.hProcess)) - success = true; - } - } - if (!success) { - SetLastError(GetLastError()); - MakeErrMsg(ErrMsg, std::string("Unable to set memory limit")); - TerminateProcess(pi.hProcess, 1); - WaitForSingleObject(pi.hProcess, INFINITE); - return; - } - } -} - bool Program::ChangeStdinToBinary(){ int result = _setmode( _fileno(stdin), _O_BINARY ); return result == -1; -- 2.34.1