From 3afe581c1f22f106d59cf54b9b65251e6c554671 Mon Sep 17 00:00:00 2001 From: Haoran Peng Date: Sat, 22 Mar 2025 10:55:37 -0700 Subject: [PATCH 1/4] Fix issues #103 and #104 (#106) --- subprocess.hpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/subprocess.hpp b/subprocess.hpp index 10c3a87..24fa303 100755 --- a/subprocess.hpp +++ b/subprocess.hpp @@ -1630,11 +1630,13 @@ inline void Popen::execute_process() noexcept(false) try { char err_buf[SP_MAX_ERR_BUF_SIZ] = {0,}; - int read_bytes = util::read_atmost_n( - fdopen(err_rd_pipe, "r"), - err_buf, - SP_MAX_ERR_BUF_SIZ); - close(err_rd_pipe); + FILE* err_fp = fdopen(err_rd_pipe, "r"); + if (!err_fp) { + close(err_rd_pipe); + throw OSError("fdopen failed", errno); + } + int read_bytes = util::read_atmost_n(err_fp, err_buf, SP_MAX_ERR_BUF_SIZ); + fclose(err_fp); if (read_bytes || strlen(err_buf)) { // Call waitpid to reap the child process From beda7b5244f15b6e71d7c42c4551a2e2d0697898 Mon Sep 17 00:00:00 2001 From: Haowen Liu <35328328+lunacd@users.noreply.github.com> Date: Tue, 22 Apr 2025 08:01:03 -0400 Subject: [PATCH 2/4] Support cwd on Windows (#108) This requires making cwd arg be either narrow or wide char depending on platform and plumbing the argument to CreateProcessW. Since `env_char_t` and `env_str_t` is now being used beyond just env, I've renamed those types to `platform_char_t` and `platform_str_t`. `env_char_t` and `env_str_t` are provided as alias to the new name for backwards compatibility. --- subprocess.hpp | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/subprocess.hpp b/subprocess.hpp index 24fa303..c479199 100755 --- a/subprocess.hpp +++ b/subprocess.hpp @@ -156,14 +156,16 @@ public: //Environment Variable types #ifndef _MSC_VER - using env_string_t = std::string; - using env_char_t = char; + using platform_str_t = std::string; + using platform_char_t = char; #else - using env_string_t = std::wstring; - using env_char_t = wchar_t; + using platform_str_t = std::wstring; + using platform_char_t = wchar_t; #endif -using env_map_t = std::map; -using env_vector_t = std::vector; +using env_str_t = platform_str_t; +using env_char_t = platform_char_t; +using env_map_t = std::map; +using env_vector_t = std::vector; //-------------------------------------------------------------------- namespace util @@ -333,14 +335,14 @@ namespace util while (*variable_strings_ptr) { // Create a string from Variable String - env_string_t current_line(variable_strings_ptr); + platform_str_t current_line(variable_strings_ptr); // Find the first "equals" sign. auto pos = current_line.find(delimeter); // Assuming it's not missing ... if(pos!=std::wstring::npos){ // ... parse the key and value. - env_string_t key = current_line.substr(0, pos); - env_string_t value = current_line.substr(pos + del_len); + platform_str_t key = current_line.substr(0, pos); + platform_str_t value = current_line.substr(pos + del_len); // Map the entry. mapped_environment[key] = value; } @@ -368,7 +370,7 @@ namespace util // And fill'er up. for(auto kv: source_map){ // Create the line - env_string_t current_line(kv.first); current_line += L"="; current_line += kv.second; + platform_str_t current_line(kv.first); current_line += L"="; current_line += kv.second; // Add the line to the buffer. std::copy(current_line.begin(), current_line.end(), std::back_inserter(environment_map_buffer)); // Append a null @@ -730,10 +732,11 @@ struct executable: string_arg * * Eg: cwd{"/som/path/x"} */ -struct cwd: string_arg +struct cwd { - template - cwd(T&& arg): string_arg(std::forward(arg)) {} + explicit cwd(const platform_str_t &cwd): cwd_(cwd) {} + explicit cwd(platform_str_t &&cwd): cwd_(std::move(cwd)) {} + platform_str_t cwd_; }; /*! @@ -1352,7 +1355,7 @@ private: bool session_leader_ = false; std::string exe_name_; - std::string cwd_; + platform_str_t cwd_; env_map_t env_; preexec_func preexec_fn_; @@ -1540,6 +1543,8 @@ inline void Popen::execute_process() noexcept(false) siStartInfo.dwFlags |= STARTF_USESTDHANDLES; + const wchar_t *cwd_arg = this->cwd_.empty() ? NULL : cwd_.c_str(); + // Create the child process. bSuccess = CreateProcessW(NULL, szCmdline, // command line @@ -1548,7 +1553,7 @@ inline void Popen::execute_process() noexcept(false) TRUE, // handles are inherited creation_flags, // creation flags environment_string_table_ptr, // use provided environment - NULL, // use parent's current directory + cwd_arg, // use provided current directory &siStartInfo, // STARTUPINFOW pointer &piProcInfo); // receives PROCESS_INFORMATION @@ -1663,7 +1668,7 @@ namespace detail { } inline void ArgumentDeducer::set_option(cwd&& cwdir) { - popen_->cwd_ = std::move(cwdir.arg_value); + popen_->cwd_ = std::move(cwdir.cwd_); } inline void ArgumentDeducer::set_option(bufsize&& bsiz) { From 2d8a8eebb03e509840e2c3c755d1abf32d930f33 Mon Sep 17 00:00:00 2001 From: Haowen Liu <35328328+lunacd@users.noreply.github.com> Date: Tue, 22 Apr 2025 08:02:05 -0400 Subject: [PATCH 3/4] Fix string_arg when used with rref (#110) When passing in a rvalue reference, compiler considers it ambiguous between std::string and std::string&&. Making one of them take a lvalue reference makes compilers correctly pick the right one depending on whether the passed in value binds to a rvalue or lvalue reference. --- subprocess.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subprocess.hpp b/subprocess.hpp index c479199..c92f56a 100755 --- a/subprocess.hpp +++ b/subprocess.hpp @@ -708,7 +708,7 @@ struct string_arg { string_arg(const char* arg): arg_value(arg) {} string_arg(std::string&& arg): arg_value(std::move(arg)) {} - string_arg(std::string arg): arg_value(std::move(arg)) {} + string_arg(const std::string& arg): arg_value(arg) {} std::string arg_value; }; From 04b015a8e52ead4d8bb5f0eb486419c77e418a17 Mon Sep 17 00:00:00 2001 From: Haowen Liu <35328328+lunacd@users.noreply.github.com> Date: Tue, 22 Apr 2025 08:02:49 -0400 Subject: [PATCH 4/4] Get Windows return code in wait() (#109) Currently, wait() returns 0 on windows regardless of the actual return code of processes. --- subprocess.hpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/subprocess.hpp b/subprocess.hpp index c92f56a..1e8efaf 100755 --- a/subprocess.hpp +++ b/subprocess.hpp @@ -1411,7 +1411,12 @@ inline int Popen::wait() noexcept(false) #ifdef __USING_WINDOWS__ int ret = WaitForSingleObject(process_handle_, INFINITE); - return 0; + DWORD dretcode_; + + if (FALSE == GetExitCodeProcess(process_handle_, &dretcode_)) + throw OSError("Failed during call to GetExitCodeProcess", 0); + + return (int)dretcode_; #else int ret, status; std::tie(ret, status) = util::wait_for_child_exit(pid());