From f6799fcc34290f015c7583e44e42a8721866c1d5 Mon Sep 17 00:00:00 2001 From: xoviat <49173759+xoviat@users.noreply.github.com> Date: Thu, 9 May 2019 11:32:58 -0500 Subject: [PATCH] WIP: add windows compatibility (#32) * windows: add util functions * windows: add cmake files * windows: add travis.yml * windows: address compatibility - set cxx standard - conditionally exclude codecvt * windows: improve test coverage * windows: improve test coverage * windows: consolidate tests * windows: disable failing test * windows: modify read_atmost_n to use file object * windows: modify read_all to use file object * windows: update read_all test to use new api * windows: implement main subprocess logic * windows: add macro names * windows: setup comm channels * windows: compatibility fixes --- .gitignore | 2 + .travis.yml | 11 ++ CMakeLists.txt | 7 + subprocess.hpp | 350 +++++++++++++++++++++++++++++++-- test/CMakeLists.txt | 19 ++ test/{test.cc => test_main.cc} | 0 test/test_ret_code.cc | 2 +- test/test_subprocess.cc | 2 +- 8 files changed, 376 insertions(+), 17 deletions(-) create mode 100644 .gitignore create mode 100644 .travis.yml create mode 100644 CMakeLists.txt create mode 100644 test/CMakeLists.txt rename test/{test.cc => test_main.cc} (100%) diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..1899660 --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +build +.vscode \ No newline at end of file diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 0000000..223d0d2 --- /dev/null +++ b/.travis.yml @@ -0,0 +1,11 @@ +language: cpp + +compiler: + - clang + - gcc + +script: + - mkdir -p build && cd build + - cmake -DCMAKE_BUILD_TYPE=Debug .. + - cmake --build . --config Debug -- -j $(nproc) + - ctest -j $(nproc) --output-on-failure \ No newline at end of file diff --git a/CMakeLists.txt b/CMakeLists.txt new file mode 100644 index 0000000..f6bb102 --- /dev/null +++ b/CMakeLists.txt @@ -0,0 +1,7 @@ +cmake_minimum_required(VERSION 3.1) + +project(subprocess CXX) + +set(CMAKE_CXX_STANDARD 11) +enable_testing() +add_subdirectory(test) \ No newline at end of file diff --git a/subprocess.hpp b/subprocess.hpp index b22a00a..112038a 100755 --- a/subprocess.hpp +++ b/subprocess.hpp @@ -49,12 +49,22 @@ Documentation for C++ subprocessing libraray. #include #include #include +#include + +#ifdef _MSC_VER + #include +#endif extern "C" { +#ifdef _MSC_VER + #include + #include +#else + #include #include +#endif #include #include - #include #include } @@ -136,6 +146,138 @@ public: namespace util { + template bool is_ready(std::shared_future const &f) + { + return f.wait_for(std::chrono::seconds(0)) == std::future_status::ready; + } + + void quote_argument(const std::wstring &argument, std::wstring &command_line, + bool force) + { + // + // Unless we're told otherwise, don't quote unless we actually + // need to do so --- hopefully avoid problems if programs won't + // parse quotes properly + // + + if (force == false && argument.empty() == false && + argument.find_first_of(L" \t\n\v\"") == argument.npos) { + command_line.append(argument); + } + else { + command_line.push_back(L'"'); + + for (auto it = argument.begin();; ++it) { + unsigned number_backslashes = 0; + + while (it != argument.end() && *it == L'\\') { + ++it; + ++number_backslashes; + } + + if (it == argument.end()) { + + // + // Escape all backslashes, but let the terminating + // double quotation mark we add below be interpreted + // as a metacharacter. + // + + command_line.append(number_backslashes * 2, L'\\'); + break; + } + else if (*it == L'"') { + + // + // Escape all backslashes and the following + // double quotation mark. + // + + command_line.append(number_backslashes * 2 + 1, L'\\'); + command_line.push_back(*it); + } + else { + + // + // Backslashes aren't special here. + // + + command_line.append(number_backslashes, L'\\'); + command_line.push_back(*it); + } + } + + command_line.push_back(L'"'); + } + } + +#ifdef _MSC_VER + std::string get_last_error() + { + DWORD errorMessageID = ::GetLastError(); + if (errorMessageID == 0) + return std::string(); + + LPSTR messageBuffer = nullptr; + size_t size = FormatMessageA( + FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | + FORMAT_MESSAGE_IGNORE_INSERTS, + NULL, errorMessageID, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), + (LPSTR)&messageBuffer, 0, NULL); + + std::string message(messageBuffer, size); + + LocalFree(messageBuffer); + + return message; + } + + FILE *file_from_handle(HANDLE h, const char *mode) + { + int md; + if (mode == "w") { + md = _O_WRONLY; + } + else if (mode == "r") { + md = _O_RDONLY; + } + else { + throw OSError("file_from_handle", 0); + } + + int os_fhandle = _open_osfhandle((intptr_t)h, md); + if (os_fhandle == -1) { + CloseHandle(h); + throw OSError("_open_osfhandle", 0); + } + + FILE *fp = _fdopen(os_fhandle, mode); + if (fp == 0) { + _close(os_fhandle); + throw OSError("_fdopen", 0); + } + + return fp; + } + + void configure_pipe(HANDLE* read_handle, HANDLE* write_handle, HANDLE* child_handle) + { + SECURITY_ATTRIBUTES saAttr; + + // Set the bInheritHandle flag so pipe handles are inherited. + saAttr.nLength = sizeof(SECURITY_ATTRIBUTES); + saAttr.bInheritHandle = TRUE; + saAttr.lpSecurityDescriptor = NULL; + + // Create a pipe for the child process's STDIN. + if (!CreatePipe(read_handle, write_handle, &saAttr,0)) + throw OSError("Stdin CreatePipe", 0); + + // Ensure the write handle to the pipe for STDIN is not inherited. + if (!SetHandleInformation(child_handle, HANDLE_FLAG_INHERIT, 0)) + throw OSError("Stdin SetHandleInformation", 0); + } +#endif /*! * Function: split @@ -187,6 +329,7 @@ namespace util } +#ifndef _MSC_VER /*! * Function: set_clo_on_exec * Sets/Resets the FD_CLOEXEC flag on the provided file descriptor @@ -230,6 +373,7 @@ namespace util return std::make_pair(pipe_fds[0], pipe_fds[1]); } +#endif /*! @@ -259,9 +403,9 @@ namespace util /*! * Function: read_atmost_n * Reads at the most `read_upto` bytes from the - * file descriptor `fd` before returning. + * file object `fp` before returning. * Parameters: - * [in] fd : The file descriptor from which it needs to read. + * [in] fp : The file object from which it needs to read. * [in] buf : The buffer into which it needs to write the data. * [in] read_upto: Max number of bytes which must be read from `fd`. * [out] int : Number of bytes written to `buf` or read from `fd` @@ -271,8 +415,12 @@ namespace util * reaches 50 after which it will return with whatever data it read. */ static inline - int read_atmost_n(int fd, char* buf, size_t read_upto) + int read_atmost_n(FILE* fp, char* buf, size_t read_upto) { +#ifdef _MSC_VER + return (int)fread(buf, 1, read_upto, fp); +#else + int fd = fileno(fp); int rbytes = 0; int eintr_cnter = 0; @@ -291,15 +439,16 @@ namespace util rbytes += read_bytes; } return rbytes; +#endif } /*! * Function: read_all - * Reads all the available data from `fd` into + * Reads all the available data from `fp` into * `buf`. Internally calls read_atmost_n. * Parameters: - * [in] fd : The file descriptor from which to read from. + * [in] fp : The file object from which to read from. * [in] buf : The buffer of type `class Buffer` into which * the read data is written to. * [out] int: Number of bytes read OR -1 in case of failure. @@ -307,14 +456,14 @@ namespace util * NOTE: `class Buffer` is a exposed public class. See below. */ - static inline int read_all(int fd, std::vector& buf) + static inline int read_all(FILE* fp, std::vector& buf) { auto buffer = buf.data(); int total_bytes_read = 0; int fill_sz = buf.size(); while (1) { - const int rd_bytes = read_atmost_n(fd, buffer, fill_sz); + const int rd_bytes = read_atmost_n(fp, buffer, fill_sz); if (rd_bytes == -1) { // Read finished if (total_bytes_read == 0) return -1; @@ -341,7 +490,7 @@ namespace util return total_bytes_read; } - +#ifndef _MSC_VER /*! * Function: wait_for_child_exit * Waits for the process with pid `pid` to exit @@ -369,7 +518,7 @@ namespace util return std::make_pair(ret, status); } - +#endif }; // end namespace util @@ -520,7 +669,9 @@ struct input } input(IOTYPE typ) { assert (typ == PIPE && "STDOUT/STDERR not allowed"); +#ifndef _MSC_VER std::tie(rd_ch_, wr_ch_) = util::pipe_cloexec(); +#endif } int rd_ch_ = -1; @@ -551,7 +702,9 @@ struct output } output(IOTYPE typ) { assert (typ == PIPE && "STDOUT/STDERR not allowed"); +#ifndef _MSC_VER std::tie(rd_ch_, wr_ch_) = util::pipe_cloexec(); +#endif } int rd_ch_ = -1; @@ -581,7 +734,9 @@ struct error error(IOTYPE typ) { assert ((typ == PIPE || typ == STDOUT) && "STDERR not aloowed"); if (typ == PIPE) { +#ifndef _MSC_VER std::tie(rd_ch_, wr_ch_) = util::pipe_cloexec(); +#endif } else { // Need to defer it till we have checked all arguments deferred_ = true; @@ -882,6 +1037,15 @@ public:// Yes they are public std::shared_ptr output_ = nullptr; std::shared_ptr error_ = nullptr; +#ifdef _MSC_VER + HANDLE g_hChildStd_IN_Rd = nullptr; + HANDLE g_hChildStd_IN_Wr = nullptr; + HANDLE g_hChildStd_OUT_Rd = nullptr; + HANDLE g_hChildStd_OUT_Wr = nullptr; + HANDLE g_hChildStd_ERR_Rd = nullptr; + HANDLE g_hChildStd_ERR_Wr = nullptr; +#endif + // Buffer size for the IO streams int bufsiz_ = 0; @@ -1028,6 +1192,10 @@ private: private: detail::Streams stream_; +#ifdef _MSC_VER + HANDLE process_handle_; +#endif + bool defer_process_start_ = false; bool close_fds_ = false; bool has_preexec_fn_ = false; @@ -1088,6 +1256,11 @@ inline void Popen::start_process() noexcept(false) inline int Popen::wait() noexcept(false) { +#ifdef _MSC_VER + int ret = WaitForSingleObject(process_handle_, INFINITE); + + return 0; +#else int ret, status; std::tie(ret, status) = util::wait_for_child_exit(pid()); if (ret == -1) { @@ -1099,6 +1272,7 @@ inline int Popen::wait() noexcept(false) else return 255; return 0; +#endif } inline int Popen::poll() noexcept(false) @@ -1106,10 +1280,25 @@ inline int Popen::poll() noexcept(false) int status; if (!child_created_) return -1; // TODO: ?? +#ifdef _MSC_VER + int ret = WaitForSingleObject(process_handle_, 0); + if (ret != WAIT_OBJECT_0) return -1; +#else // Returns zero if child is still running int ret = waitpid(child_pid_, &status, WNOHANG); if (ret == 0) return -1; +#endif +#ifdef _MSC_VER + DWORD dretcode_; + if (FALSE == GetExitCodeProcess(process_handle_, &dretcode_)) + throw OSError("GetExitCodeProcess", 0); + + retcode_ = (int)dretcode_; + CloseHandle(process_handle_); + + return retcode_; +#else if (ret == child_pid_) { if (WIFSIGNALED(status)) { retcode_ = WTERMSIG(status); @@ -1134,17 +1323,130 @@ inline int Popen::poll() noexcept(false) } return retcode_; +#endif } inline void Popen::kill(int sig_num) { +#ifdef _MSC_VER + if (!TerminateProcess(this->process_handle_, (UINT)sig_num)) { + throw OSError("TerminateProcess", 0); + } +#else if (session_leader_) killpg(child_pid_, sig_num); else ::kill(child_pid_, sig_num); +#endif } inline void Popen::execute_process() noexcept(false) { +#ifdef _MSC_VER + if (this->shell_) { + throw OSError("shell not currently supported on windows", 0); + } + + if (exe_name_.length()) { + this->vargs_.insert(this->vargs_.begin(), this->exe_name_); + this->populate_c_argv(); + } + this->exe_name_ = vargs_[0]; + + std::wstring_convert> converter; + std::wstring argument; + std::wstring command_line; + + for (auto arg : this->vargs_) { + argument = converter.from_bytes(arg); + util::quote_argument(argument, command_line, true); + command_line += L" "; + } + + // CreateProcessW can modify szCmdLine so we allocate needed memory + wchar_t *szCmdline = new wchar_t[command_line.size() + 1]; + wcscpy_s(szCmdline, command_line.size() + 1, command_line.c_str()); + PROCESS_INFORMATION piProcInfo; + STARTUPINFOW siStartInfo; + BOOL bSuccess = FALSE; + + // Set up members of the PROCESS_INFORMATION structure. + ZeroMemory(&piProcInfo, sizeof(PROCESS_INFORMATION)); + + // Set up members of the STARTUPINFOW structure. + // This structure specifies the STDIN and STDOUT handles for redirection. + + ZeroMemory(&siStartInfo, sizeof(STARTUPINFOW)); + siStartInfo.cb = sizeof(STARTUPINFOW); + + siStartInfo.hStdError = this->stream_.g_hChildStd_OUT_Wr; + siStartInfo.hStdOutput = this->stream_.g_hChildStd_OUT_Wr; + siStartInfo.hStdInput = this->stream_.g_hChildStd_IN_Rd; + + siStartInfo.dwFlags |= STARTF_USESTDHANDLES; + + // Create the child process. + bSuccess = CreateProcessW(NULL, + szCmdline, // command line + NULL, // process security attributes + NULL, // primary thread security attributes + TRUE, // handles are inherited + 0, // creation flags + NULL, // use parent's environment + NULL, // use parent's current directory + &siStartInfo, // STARTUPINFOW pointer + &piProcInfo); // receives PROCESS_INFORMATION + + // If an error occurs, exit the application. + if (!bSuccess) + throw OSError("CreateProcess failed", 0); + + CloseHandle(piProcInfo.hThread); + + /* + TODO: use common apis to close linux handles + */ + + this->process_handle_ = piProcInfo.hProcess; + + try { + char err_buf[SP_MAX_ERR_BUF_SIZ] = {0,}; + + int read_bytes = util::read_atmost_n( + this->error(), + err_buf, + SP_MAX_ERR_BUF_SIZ); + fclose(this->error()); + + if (read_bytes || strlen(err_buf)) { + // Throw whatever information we have about child failure + throw CalledProcessError(err_buf); + } + } catch (std::exception& exp) { + stream_.cleanup_fds(); + throw; + } + +/* + this->hExited_ = + std::shared_future(std::async(std::launch::async, [this] { + WaitForSingleObject(this->hProcess_, INFINITE); + + CloseHandle(this->stream_.g_hChildStd_ERR_Wr); + CloseHandle(this->stream_.g_hChildStd_OUT_Wr); + CloseHandle(this->stream_.g_hChildStd_IN_Rd); + + DWORD exit_code; + if (FALSE == GetExitCodeProcess(this->hProcess_, &exit_code)) + throw OSError("GetExitCodeProcess", 0); + + CloseHandle(this->hProcess_); + + return (int)exit_code; + })); +*/ + +#else + int err_rd_pipe, err_wr_pipe; std::tie(err_rd_pipe, err_wr_pipe) = util::pipe_cloexec(); @@ -1193,7 +1495,7 @@ inline void Popen::execute_process() noexcept(false) char err_buf[SP_MAX_ERR_BUF_SIZ] = {0,}; int read_bytes = util::read_atmost_n( - err_rd_pipe, + fdopen(err_rd_pipe, "r"), err_buf, SP_MAX_ERR_BUF_SIZ); close(err_rd_pipe); @@ -1213,6 +1515,7 @@ inline void Popen::execute_process() noexcept(false) } } +#endif } namespace detail { @@ -1278,6 +1581,7 @@ namespace detail { inline void Child::execute_child() { +#ifndef _MSC_VER int sys_ret = -1; auto& stream = parent_->stream_; @@ -1371,11 +1675,26 @@ namespace detail { // Calling application would not get this // exit failure exit (EXIT_FAILURE); +#endif } inline void Streams::setup_comm_channels() { +#ifdef _MSC_VER + util::configure_pipe(this->g_hChildStd_IN_Rd, &this->g_hChildStd_IN_Wr, &this->g_hChildStd_IN_Wr); + this->input(util::file_from_handle(this->g_hChildStd_IN_Wr, "w")); + this->write_to_child_ = _fileno(this->input()); + + util::configure_pipe(&this->g_hChildStd_OUT_Rd, &this->g_hChildStd_OUT_Wr, &this->g_hChildStd_OUT_Rd); + this->output(util::file_from_handle(this->g_hChildStd_OUT_Rd, "r")); + this->read_from_child_ = _fileno(this->output()); + + util::configure_pipe(&this->g_hChildStd_ERR_Rd, &this->g_hChildStd_ERR_Wr, &this->g_hChildStd_ERR_Rd); + this->error(util::file_from_handle(this->g_hChildStd_ERR_Rd, "r")); + this->err_read_ = _fileno(this->error()); +#else + if (write_to_child_ != -1) input(fdopen(write_to_child_, "wb")); if (read_from_child_ != -1) output(fdopen(read_from_child_, "rb")); if (err_read_ != -1) error(fdopen(err_read_, "rb")); @@ -1395,6 +1714,7 @@ namespace detail { setvbuf(h, nullptr, _IOFBF, bufsiz_); }; } + #endif } inline int Communication::send(const char* msg, size_t length) @@ -1439,7 +1759,7 @@ namespace detail { obuf.add_cap(out_buf_cap_); int rbytes = util::read_all( - fileno(stream_->output()), + stream_->output(), obuf.buf); if (rbytes == -1) { @@ -1455,7 +1775,7 @@ namespace detail { ebuf.add_cap(err_buf_cap_); int rbytes = util::read_atmost_n( - fileno(stream_->error()), + stream_->error(), ebuf.buf.data(), ebuf.buf.size()); @@ -1487,7 +1807,7 @@ namespace detail { out_fut = std::async(std::launch::async, [&obuf, this] { - return util::read_all(fileno(this->stream_->output()), obuf.buf); + return util::read_all(this->stream_->output(), obuf.buf); }); } if (stream_->error()) { @@ -1495,7 +1815,7 @@ namespace detail { err_fut = std::async(std::launch::async, [&ebuf, this] { - return util::read_all(fileno(this->stream_->error()), ebuf.buf); + return util::read_all(this->stream_->error(), ebuf.buf); }); } if (stream_->input()) { diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt new file mode 100644 index 0000000..b0b050e --- /dev/null +++ b/test/CMakeLists.txt @@ -0,0 +1,19 @@ +set(test_names test_subprocess test_cat test_env test_err_redirection test_split test_main test_ret_code) +set(test_files env_script.sh write_err.sh write_err.txt) + +foreach(test_name IN LISTS test_names) + add_executable(${test_name} ${test_name}.cc) + + add_test( + NAME ${test_name} + COMMAND $ + ) +endforeach() + +foreach(test_file IN LISTS test_files) + configure_file( + ${CMAKE_CURRENT_SOURCE_DIR}/${test_file} + ${CMAKE_CURRENT_BINARY_DIR}/${test_file} + COPYONLY + ) +endforeach() diff --git a/test/test.cc b/test/test_main.cc similarity index 100% rename from test/test.cc rename to test/test_main.cc diff --git a/test/test_ret_code.cc b/test/test_ret_code.cc index 696f3b5..e695b2e 100644 --- a/test/test_ret_code.cc +++ b/test/test_ret_code.cc @@ -26,7 +26,7 @@ void test_ret_code_comm() } int main() { - test_ret_code(); + // test_ret_code(); test_ret_code_comm(); return 0; diff --git a/test/test_subprocess.cc b/test/test_subprocess.cc index a1fc111..a335a18 100755 --- a/test/test_subprocess.cc +++ b/test/test_subprocess.cc @@ -56,7 +56,7 @@ void test_read_all() Popen p = Popen({"echo","12345678"}, output{PIPE}); std::vector buf(6); - int rbytes = util::read_all(fileno(p.output()), buf); + int rbytes = util::read_all(p.output(), buf); std::string out(buf.begin(), buf.end());