From 006cc6f52ade3fde5eeaa16b1377b13162a3c5be Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 11 Apr 2022 14:33:35 +0200 Subject: [PATCH] Read the Wine version without Boost.Process By spawning the process using posix_spawn and manually reading from a pipe. --- src/common/process.cpp | 87 ++++++++++++++++++++++++++++++++- src/common/process.h | 97 +++++++++++++++++++++++++++++++++++++ src/plugin/utils.cpp | 107 +++++++++++++++++++++++++---------------- src/plugin/utils.h | 3 ++ 4 files changed, 251 insertions(+), 43 deletions(-) diff --git a/src/common/process.cpp b/src/common/process.cpp index a9827ecc..0419e4d6 100644 --- a/src/common/process.cpp +++ b/src/common/process.cpp @@ -18,7 +18,8 @@ #include -#include +#include +#include namespace fs = ghc::filesystem; @@ -91,3 +92,87 @@ char* const* ProcessEnvironment::make_environ() const { return const_cast(recreated_environ_.data()); } + +bool Process::Handle::running() const noexcept { + return pid_running(pid); +} + +void Process::Handle::terminate() const noexcept { + kill(pid, SIGINT); + wait(); +} + +std::optional Process::Handle::wait() const noexcept { + int status = 0; + assert(waitpid(pid, &status, 0) > 0); + + if (WIFEXITED(status)) { + return WEXITSTATUS(status); + } else { + return std::nullopt; + } +} + +Process::Process(std::string command) : command_(command) {} + +Process::StringResult Process::spawn_get_stdout_line() { + /// We'll read the results from a pipe. The child writes to the second pipe, + /// we'll read from the first one. + int output_pipe[2]; + ::pipe(output_pipe); + + const auto argv = build_argv(); + const auto envp = env_ ? env_->make_environ() : environ; + + posix_spawn_file_actions_t actions; + posix_spawn_file_actions_init(&actions); + posix_spawn_file_actions_adddup2(&actions, output_pipe[1], STDOUT_FILENO); + posix_spawn_file_actions_addclose(&actions, output_pipe[0]); + posix_spawn_file_actions_addclose(&actions, output_pipe[1]); + + pid_t child_pid = 0; + const auto result = posix_spawnp(&child_pid, command_.c_str(), &actions, + nullptr, argv, envp); + + close(output_pipe[1]); + if (result == 2) { + close(output_pipe[0]); + return Process::CommandNotFound{}; + } else if (result != 0) { + close(output_pipe[0]); + return std::error_code(result, std::system_category()); + } + + // Try to read the first line out the output until the line feed + std::array output{0}; + FILE* output_pipe_stream = fdopen(output_pipe[0], "r"); + assert(output_pipe_stream); + fgets(output.data(), output.size(), output_pipe_stream); + fclose(output_pipe_stream); + + int status = 0; + assert(waitpid(child_pid, &status, 0) > 0); + if (!WIFEXITED(status) || WEXITSTATUS(status) == 127) { + return Process::CommandNotFound{}; + } else { + // `fgets()` returns the line feed, so we'll get rid of that + std::string output_str(output.data()); + if (output_str.back() == '\n') { + output_str.pop_back(); + } + + return output_str; + } +} + +char* const* Process::build_argv() const { + argv_.clear(); + + argv_.push_back(command_.c_str()); + for (const auto& arg : args_) { + argv_.push_back(arg.c_str()); + } + argv_.push_back(nullptr); + + return const_cast(argv_.data()); +} diff --git a/src/common/process.h b/src/common/process.h index 094a947b..3743109e 100644 --- a/src/common/process.h +++ b/src/common/process.h @@ -18,8 +18,18 @@ #include #include +#include +#include #include +#ifdef __WINE__ +#include "../wine-host/asio-fix.h" +#endif + +#include +#include +#include + // A minimal API akin to Boost.Process for launching and managing processes // using plain Linux APIs. Needed so we can implement our chainloader without // pulling in Boost.Process' Boost.Filesystem dependency (which would defeat the @@ -81,3 +91,90 @@ class ProcessEnvironment { */ mutable std::vector recreated_environ_; }; + +/** + * A child process whose output can be captured. Simple wrapper around the Posix + * APIs. + */ +class Process { + public: + /** + * Marker to indicate that the program was not found. + */ + struct CommandNotFound {}; + + /** + * A handle to a running process. + */ + class Handle { + public: + /** + * The process' ID. + */ + const pid_t pid; + + /** + * Whether the process is still running **and not a zombie**. + */ + bool running() const noexcept; + + /** + * Forcefully terminate the process by sending `SIGKILL`. Will reap the + * process zombie after sending the signal. + */ + void terminate() const noexcept; + + /** + * Wait for the process to exit, returning the exit code if it exited + * successfully. Returns a nullopt otherwise. + */ + std::optional wait() const noexcept; + }; + + using StringResult = + std::variant; + using StatusResult = std::variant; + using HandleResult = std::variant; + + /** + * Build a process. Use the other functions to add arguments or to + * launch the process. + * + * @param command The name of the command. `$PATH` will be searched for + * this command if it is not absolute. + */ + Process(std::string command); + + /** + * Add an argument to the command invocation. + */ + inline void arg(std::string arg) { args_.emplace_back(std::move(arg)); } + + /** + * Use the specified environment for this command. + * + * @see environment + */ + inline void environment(ProcessEnvironment env) { env_ = std::move(env); } + + /** + * Spawn the process, leave STDIN and STDERR alone, and return the first + * line (without the trailing linefeed) of STDOUT. The first output line + * will still be returned even if the process exits with a non-zero exit + * code. Uses `posix_spawn()`, leaves file descriptors in tact. + */ + StringResult spawn_get_stdout_line(); + + private: + /** + * Create the `argv` array from the command and the arguments. Only valid as + * long as the pointers in `args_` at the time of calling stay valid. + */ + char* const* build_argv() const; + + std::string command_; + std::vector args_; + std::optional env_; + + mutable std::vector argv_; +}; diff --git a/src/plugin/utils.cpp b/src/plugin/utils.cpp index be71c148..6719c8d8 100644 --- a/src/plugin/utils.cpp +++ b/src/plugin/utils.cpp @@ -83,14 +83,35 @@ bp::environment PluginInfo::create_host_env() const { return env; } +ProcessEnvironment PluginInfo::create_host_env_2() const { + ProcessEnvironment env(environ); + + // Only set the prefix when could auto detect it and it's not being + // overridden (this entire `std::visit` instead of `std::has_alternative` is + // just for clarity's sake) + std::visit(overload{ + [](const OverridenWinePrefix&) {}, + [&](const ghc::filesystem::path& prefix) { + env.insert("WINEPREFIX", prefix.string()); + }, + [](const DefaultWinePrefix&) {}, + }, + wine_prefix_); + + return env; +} + ghc::filesystem::path PluginInfo::normalize_wine_prefix() const { return std::visit( overload{ [](const OverridenWinePrefix& prefix) { return prefix.value; }, [](const ghc::filesystem::path& prefix) { return prefix; }, [](const DefaultWinePrefix&) { - const bp::environment env = boost::this_process::environment(); - return fs::path(env.at("HOME").to_string()) / ".wine"; + // NOLINTNEXTLINE(concurrency-mt-unsafe) + const char* home_dir = getenv("HOME"); + assert(home_dir); + + return fs::path(home_dir) / ".wine"; }, }, wine_prefix_); @@ -99,38 +120,40 @@ ghc::filesystem::path PluginInfo::normalize_wine_prefix() const { std::string PluginInfo::wine_version() const { // The '*.exe' scripts generated by winegcc allow you to override the binary // used to run Wine, so will will handle this in the same way for our Wine - // version detection - // FIXME: Replace Boost.Filesystem usage - boost::filesystem::path wine_path; - bp::environment env = create_host_env(); - if (const std::string wineloader_path = env["WINELOADER"].to_string(); - access(wineloader_path.c_str(), X_OK) == 0) { + // version detection. We'll be using `execvpe` + std::string wine_path = "wine"; + // NOLINTNEXTLINE(concurrency-mt-unsafe) + if (const char* wineloader_path = getenv("WINELOADER"); + wineloader_path && access(wineloader_path, X_OK) == 0) { wine_path = wineloader_path; - } else { - wine_path = bp::search_path("wine").string(); } - bp::ipstream output; - try { - bp::system(wine_path, "--version", bp::std_out = output, bp::env = env, - bp::posix::use_vfork); - } catch (const std::system_error&) { - return ""; - } + Process process(wine_path); + process.arg("--version"); + process.environment(create_host_env_2()); - // `wine --version` might contain additional output in certain custom Wine - // builds, so we only want to look at the first line - std::string version_string; - std::getline(output, version_string); + auto result = process.spawn_get_stdout_line(); + return std::visit( + overload{ + [](std::string version_string) -> std::string { + // Strip the `wine-` prefix from the output, could potentially + // be absent in custom Wine builds + constexpr std::string_view version_prefix("wine-"); + if (version_string.starts_with(version_prefix)) { + version_string = + version_string.substr(version_prefix.size()); + } - // Strip the `wine-` prefix from the output, could potentially be absent in - // custom Wine builds - constexpr std::string_view version_prefix("wine-"); - if (version_string.starts_with(version_prefix)) { - version_string = version_string.substr(version_prefix.size()); - } - - return version_string; + return version_string; + }, + [](const Process::CommandNotFound&) -> std::string { + return ""; + }, + [](const std::error_code& err) -> std::string { + return ""; + }, + }, + result); } fs::path find_plugin_library(const fs::path& this_plugin_path, @@ -256,9 +279,9 @@ fs::path normalize_plugin_path(const fs::path& windows_library_path, std::variant find_wine_prefix( fs::path windows_plugin_path) { - const bp::environment env = boost::this_process::environment(); - if (const auto prefix = env.find("WINEPREFIX"); prefix != env.end()) { - return OverridenWinePrefix{prefix->to_string()}; + // NOLINTNEXTLINE(concurrency-mt-unsafe) + if (const auto prefix = getenv("WINEPREFIX")) { + return OverridenWinePrefix{prefix}; } const std::optional dosdevices_dir = find_dominating_file( @@ -411,16 +434,14 @@ std::vector get_augmented_search_path() { std::vector search_path = boost::this_process::path(); - const bp::environment environment = boost::this_process::environment(); - if (auto xdg_data_home = environment.find("XDG_DATA_HOME"); - xdg_data_home != environment.end()) { - search_path.push_back( - boost::filesystem::path(xdg_data_home->to_string()) / "yabridge"); - } else if (auto home_directory = environment.find("HOME"); - home_directory != environment.end()) { - search_path.push_back( - boost::filesystem::path(home_directory->to_string()) / ".local" / - "share" / "yabridge"); + // NOLINTNEXTLINE(concurrency-mt-unsafe) + if (const char* xdg_data_home = getenv("XDG_DATA_HOME")) { + search_path.push_back(boost::filesystem::path(xdg_data_home) / + "yabridge"); + // NOLINTNEXTLINE(concurrency-mt-unsafe) + } else if (const char* home_directory = getenv("HOME")) { + search_path.push_back(boost::filesystem::path(home_directory) / + ".local" / "share" / "yabridge"); } return search_path; @@ -472,6 +493,8 @@ bool send_notification(const std::string& title, } } + // TODO: Also use a custom process here, and wrap the above code in a + // class try { return bp::system(notify_send_path, "--urgency=normal", "--expire-time=15000", "--app-name=yabridge", title, diff --git a/src/plugin/utils.h b/src/plugin/utils.h index 9eeedc7f..7c2dec55 100644 --- a/src/plugin/utils.h +++ b/src/plugin/utils.h @@ -22,6 +22,7 @@ #include "../common/configuration.h" #include "../common/plugins.h" +#include "../common/process.h" #include "../common/utils.h" /** @@ -76,6 +77,8 @@ struct PluginInfo { * unset if we could not detect a prefix. */ boost::process::environment create_host_env() const; + // FIXME: Replace create_host_env with this one + ProcessEnvironment create_host_env_2() const; /** * Return the path to the actual Wine prefix in use, taking into account