highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-04-16 08:02:24 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#29865: util: remove unused cpp-subprocess options

Browse Source 
13adbf733f09c73c3cf0025d94c52f9cec5dba3b remove unneeded environment option from cpp-subprocess (Sebastian Falbesoner)
2088777ba0f9ad3f6d4ab8b0b6ff8aad71117307 remove unneeded cwd option from cpp-subprocess (Sebastian Falbesoner)
03ffb09c31aa04cc296c0ce10d07109e22a8dd75 remove unneeded bufsize option from cpp-subprocess (Sebastian Falbesoner)
79c30363733503a1fb7d4c98aa0d56ced0be6e32 remove unneeded close_fds option from cpp-subprocess (Sebastian Falbesoner)
62db8f8e5a6cfe19d905afc91731d6bc8a665f61 remove unneeded session_leader option from cpp-subprocess (Sebastian Falbesoner)
80d008c66d00d3496cd8549daee6775cf2c6b782 remove unneeded defer_spawn option from cpp-subprocess (Sebastian Falbesoner)
cececad7b29e2ca3de1216db1c541dba6dc81bfa remove unneeded preexec function option from cpp-subprocess (Sebastian Falbesoner)
633e45b2e2728efcb0637afa94fcbd5756dfbe76 remove unneeded shell option from cpp-subprocess (Sebastian Falbesoner)

Pull request description:

  The newly introduced cpp-subprocess library provides a good number of options for the `Popen` class:
  0de63b8b46/src/util/subprocess.hpp (L1009-L1020)
  Some of them are either not fully implemented (`shell`, missing an implementation on Windows), implemented in an ugly way (e.g. using "Impoverished, meager, needy, truly needy version of type erasure" for `preexec_func` according to the author's own words) or simply unlikely to be ever needed for our external signer use-case (`defer_spawn`). Instead of maintaining incomplete and/or unneeded code, I'd suggest to get rid of it and only keep support for options if there is a strong reason for it.

ACKs for top commit:
  achow101:
    ACK 13adbf733f09c73c3cf0025d94c52f9cec5dba3b
  hebasto:
    re-ACK 13adbf733f09c73c3cf0025d94c52f9cec5dba3b.

Tree-SHA512: 8270da27891cb659da2ef6062a23f4b86331859b15ac27b79ae7433b14f5bd7efaba621f2b3ba1953708d0f38377a8bd23ef1cc0f28b9c152ac8958dd9eec6b0
This commit is contained in:
Ava Chow 2024-04-23 12:58:50 -04:00
commit 2cecbbb986
No known key found for this signature in database
GPG Key ID: 17565732E08E5E41
1 changed files with 9 additions and 385 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

394
src/util/subprocess.hpp
View File

@ -154,19 +154,6 @@ public:
{}
};
//--------------------------------------------------------------------
//Environment Variable types
#ifndef _MSC_VER
using env_string_t = std::string;
using env_char_t = char;
#else
using env_string_t = std::wstring;
using env_char_t = wchar_t;
#endif
using env_map_t = std::map<env_string_t, env_string_t>;
using env_vector_t = std::vector<env_char_t>;
//--------------------------------------------------------------------
namespace util
{
@ -305,100 +292,6 @@ namespace util
if (!SetHandleInformation(*child_handle, HANDLE_FLAG_INHERIT, 0))
throw OSError("SetHandleInformation", 0);
}
// env_map_t MapFromWindowsEnvironment()
// * Imports current Environment in a C-string table
// * Parses the strings by splitting on the first "=" per line
// * Creates a map of the variables
// * Returns the map
inline env_map_t MapFromWindowsEnvironment(){
wchar_t *variable_strings_ptr;
wchar_t *environment_strings_ptr;
std::wstring delimiter(L"=");
int del_len = delimiter.length();
env_map_t mapped_environment;
// Get a pointer to the environment block.
environment_strings_ptr = GetEnvironmentStringsW();
// If the returned pointer is NULL, exit.
if (environment_strings_ptr == NULL)
{
throw OSError("GetEnvironmentStringsW", 0);
}
// Variable strings are separated by NULL byte, and the block is
// terminated by a NULL byte.
variable_strings_ptr = (wchar_t *) environment_strings_ptr;
//Since the environment map ends with a null, we can loop until we find it.
while (*variable_strings_ptr)
{
// Create a string from Variable String
env_string_t current_line(variable_strings_ptr);
// Find the first "equals" sign.
auto pos = current_line.find(delimiter);
// 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);
// Map the entry.
mapped_environment[key] = value;
}
// Jump to next line in the environment map.
variable_strings_ptr += std::wcslen(variable_strings_ptr) + 1;
}
// We're done with the old environment map buffer.
FreeEnvironmentStringsW(environment_strings_ptr);
// Return the map.
return mapped_environment;
}
// env_vector_t WindowsEnvironmentVectorFromMap(const env_map_t &source_map)
// * Creates a vector buffer for the new environment string table
// * Copies in the mapped variables
// * Returns the vector
inline env_vector_t WindowsEnvironmentVectorFromMap(const env_map_t &source_map)
{
// Make a new environment map buffer.
env_vector_t environment_map_buffer;
// Give it some space.
environment_map_buffer.reserve(4096);
// 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;
// Add the line to the buffer.
std::copy(current_line.begin(), current_line.end(), std::back_inserter(environment_map_buffer));
// Append a null
environment_map_buffer.push_back(0);
}
// Append one last null because of how Windows does it's environment maps.
environment_map_buffer.push_back(0);
return environment_map_buffer;
}
// env_vector_t CreateUpdatedWindowsEnvironmentVector(const env_map_t &changes_map)
// * Merges host environment with new mapped variables
// * Creates and returns string vector based on map
inline env_vector_t CreateUpdatedWindowsEnvironmentVector(const env_map_t &changes_map){
// Import the environment map
env_map_t environment_map = MapFromWindowsEnvironment();
// Merge in the changes with overwrite
for(auto& it: changes_map)
{
environment_map[it.first] = it.second;
}
// Create a Windows-usable Environment Map Buffer
env_vector_t environment_map_strings_vector = WindowsEnvironmentVectorFromMap(environment_map);
return environment_map_strings_vector;
}
#endif
/*!
@ -431,26 +324,6 @@ namespace util
}
/*!
* Function: join
* Parameters:
* [in] vec : Vector of strings which needs to be joined to form
* a single string with words separated by a separator char.
* [in] sep : String used to separate 2 words in the joined string.
* Default constructed to ' ' (space).
* [out] string: Joined string.
*/
static inline
std::string join(const std::vector<std::string>& vec,
const std::string& sep = " ")
{
std::string res;
for (auto& elem : vec) res.append(elem + sep);
res.erase(--res.end());
return res;
}
#ifndef __USING_WINDOWS__
/*!
* Function: set_clo_on_exec
@ -651,56 +524,6 @@ namespace util
* -------------------------------
*/
/*!
* The buffer size of the stdin/stdout/stderr
* streams of the child process.
* Default value is 0.
*/
struct bufsize {
explicit bufsize(int sz): bufsiz(sz) {}
int bufsiz = 0;
};
/*!
* Option to defer spawning of the child process
* till `Popen::start_process` API is called.
* Default value is false.
*/
struct defer_spawn {
explicit defer_spawn(bool d): defer(d) {}
bool defer = false;
};
/*!
* Option to close all file descriptors
* when the child process is spawned.
* The close fd list does not include
* input/output/error if they are explicitly
* set as part of the Popen arguments.
*
* Default value is false.
*/
struct close_fds {
explicit close_fds(bool c): close_all(c) {}
bool close_all = false;
};
/*!
* Option to make the child process as the
* session leader and thus the process
* group leader.
* Default value is false.
*/
struct session_leader {
explicit session_leader(bool sl): leader_(sl) {}
bool leader_ = false;
};
struct shell {
explicit shell(bool s): shell_(s) {}
bool shell_ = false;
};
/*!
* Base class for all arguments involving string value.
*/
@ -726,34 +549,6 @@ struct executable: string_arg
executable(T&& arg): string_arg(std::forward<T>(arg)) {}
};
/*!
* Option to set the current working directory
* of the spawned process.
*
* Eg: cwd{"/some/path/x"}
*/
struct cwd: string_arg
{
template <typename T>
cwd(T&& arg): string_arg(std::forward<T>(arg)) {}
};
/*!
* Option to specify environment variables required by
* the spawned process.
*
* Eg: environment{{ {"K1", "V1"}, {"K2", "V2"},... }}
*/
struct environment
{
environment(env_map_t&& env):
env_(std::move(env)) {}
explicit environment(const env_map_t& env):
env_(env) {}
env_map_t env_;
};
/*!
* Used for redirecting input/output/error
*/
@ -870,44 +665,6 @@ struct error
int wr_ch_ = -1;
};
// Impoverished, meager, needy, truly needy
// version of type erasure to store function pointers
// needed to provide the functionality of preexec_func
// ATTN: Can be used only to execute functions with no
// arguments and returning void.
// Could have used more efficient methods, ofcourse, but
// that won't yield me the consistent syntax which I am
// aiming for. If you know, then please do let me know.
class preexec_func
{
public:
preexec_func() {}
template <typename Func>
explicit preexec_func(Func f): holder_(new FuncHolder<Func>(std::move(f)))
{}
void operator()() {
(*holder_)();
}
private:
struct HolderBase {
virtual void operator()() const = 0;
virtual ~HolderBase(){};
};
template <typename T>
struct FuncHolder: HolderBase {
FuncHolder(T func): func_(std::move(func)) {}
void operator()() const override { func_(); }
// The function pointer/reference
T func_;
};
std::unique_ptr<HolderBase> holder_ = nullptr;
};
// ~~~~ End Popen Args ~~~~
@ -1007,17 +764,9 @@ struct ArgumentDeducer
ArgumentDeducer(Popen* p): popen_(p) {}
void set_option(executable&& exe);
void set_option(cwd&& cwdir);
void set_option(bufsize&& bsiz);
void set_option(environment&& env);
void set_option(defer_spawn&& defer);
void set_option(shell&& sh);
void set_option(input&& inp);
void set_option(output&& out);
void set_option(error&& err);
void set_option(close_fds&& cfds);
void set_option(preexec_func&& prefunc);
void set_option(session_leader&& sleader);
private:
Popen* popen_ = nullptr;
@ -1168,9 +917,6 @@ public:// Yes they are public
HANDLE g_hChildStd_ERR_Wr = nullptr;
#endif
// Buffer size for the IO streams
int bufsiz_ = 0;
// Pipes for communicating with child
// Emulates stdin
@ -1200,9 +946,9 @@ private:
* interface to the client.
*
* API's provided by the class:
* 1. Popen({"cmd"}, output{..}, error{..}, cwd{..}, ....)
* 1. Popen({"cmd"}, output{..}, error{..}, ....)
* Command provided as a sequence.
* 2. Popen("cmd arg1"m output{..}, error{..}, cwd{..}, ....)
* 2. Popen("cmd arg1"m output{..}, error{..}, ....)
* Command provided in a single string.
* 3. wait() - Wait for the child to exit.
* 4. retcode() - The return code of the exited child.
@ -1218,8 +964,6 @@ private:
in case of redirection. See piping examples.
*12. error() - Get the error channel/File pointer. Usually used
in case of redirection.
*13. start_process() - Start the child process. Only to be used when
* `defer_spawn` option was provided in Popen constructor.
*/
class Popen
{
@ -1237,7 +981,7 @@ public:
// Setup the communication channels of the Popen class
stream_.setup_comm_channels();
if (!defer_process_start_) execute_process();
execute_process();
}
template <typename... Args>
@ -1249,7 +993,7 @@ public:
// Setup the communication channels of the Popen class
stream_.setup_comm_channels();
if (!defer_process_start_) execute_process();
execute_process();
}
template <typename... Args>
@ -1260,7 +1004,7 @@ public:
// Setup the communication channels of the Popen class
stream_.setup_comm_channels();
if (!defer_process_start_) execute_process();
execute_process();
}
/*
@ -1272,8 +1016,6 @@ public:
}
*/
void start_process() noexcept(false);
int pid() const noexcept { return child_pid_; }
int retcode() const noexcept { return retcode_; }
@ -1347,16 +1089,7 @@ private:
std::future<void> cleanup_future_;
#endif
bool defer_process_start_ = false;
bool close_fds_ = false;
bool has_preexec_fn_ = false;
bool shell_ = false;
bool session_leader_ = false;
std::string exe_name_;
std::string cwd_;
env_map_t env_;
preexec_func preexec_fn_;
// Command in string format
std::string args_;
@ -1391,20 +1124,6 @@ inline void Popen::populate_c_argv()
cargv_.push_back(nullptr);
}
inline void Popen::start_process() noexcept(false)
{
// The process was started/tried to be started
// in the constructor itself.
// For explicitly calling this API to start the
// process, 'defer_spawn' argument must be set to
// true in the constructor.
if (!defer_process_start_) {
assert (0);
return;
}
execute_process();
}
inline int Popen::wait() noexcept(false)
{
#ifdef __USING_WINDOWS__
@ -1483,8 +1202,7 @@ inline void Popen::kill(int sig_num)
throw OSError("TerminateProcess", 0);
}
#else
if (session_leader_) killpg(child_pid_, sig_num);
else ::kill(child_pid_, sig_num);
::kill(child_pid_, sig_num);
#endif
}
@ -1492,17 +1210,6 @@ inline void Popen::kill(int sig_num)
inline void Popen::execute_process() noexcept(false)
{
#ifdef __USING_WINDOWS__
if (this->shell_) {
throw OSError("shell not currently supported on windows", 0);
}
void* environment_string_table_ptr = nullptr;
env_vector_t environment_string_vector;
if(this->env_.size()){
environment_string_vector = util::CreateUpdatedWindowsEnvironmentVector(env_);
environment_string_table_ptr = (void*)environment_string_vector.data();
}
if (exe_name_.length()) {
this->vargs_.insert(this->vargs_.begin(), this->exe_name_);
this->populate_c_argv();
@ -1549,7 +1256,7 @@ inline void Popen::execute_process() noexcept(false)
NULL, // primary thread security attributes
TRUE, // handles are inherited
creation_flags, // creation flags
environment_string_table_ptr, // use provided environment
NULL, // use parent's environment
NULL, // use parent's current directory
&siStartInfo, // STARTUPINFOW pointer
&piProcInfo); // receives PROCESS_INFORMATION
@ -1588,14 +1295,6 @@ inline void Popen::execute_process() noexcept(false)
int err_rd_pipe, err_wr_pipe;
std::tie(err_rd_pipe, err_wr_pipe) = util::pipe_cloexec();
if (shell_) {
auto new_cmd = util::join(vargs_);
vargs_.clear();
vargs_.insert(vargs_.begin(), {"/bin/sh", "-c"});
vargs_.push_back(new_cmd);
populate_c_argv();
}
if (exe_name_.length()) {
vargs_.insert(vargs_.begin(), exe_name_);
populate_c_argv();
@ -1662,30 +1361,6 @@ namespace detail {
popen_->exe_name_ = std::move(exe.arg_value);
}
inline void ArgumentDeducer::set_option(cwd&& cwdir) {
popen_->cwd_ = std::move(cwdir.arg_value);
}
inline void ArgumentDeducer::set_option(bufsize&& bsiz) {
popen_->stream_.bufsiz_ = bsiz.bufsiz;
}
inline void ArgumentDeducer::set_option(environment&& env) {
popen_->env_ = std::move(env.env_);
}
inline void ArgumentDeducer::set_option(defer_spawn&& defer) {
popen_->defer_process_start_ = defer.defer;
}
inline void ArgumentDeducer::set_option(shell&& sh) {
popen_->shell_ = sh.shell_;
}
inline void ArgumentDeducer::set_option(session_leader&& sleader) {
popen_->session_leader_ = sleader.leader_;
}
inline void ArgumentDeducer::set_option(input&& inp) {
if (inp.rd_ch_ != -1) popen_->stream_.read_from_parent_ = inp.rd_ch_;
if (inp.wr_ch_ != -1) popen_->stream_.write_to_child_ = inp.wr_ch_;
@ -1708,15 +1383,6 @@ namespace detail {
if (err.rd_ch_ != -1) popen_->stream_.err_read_ = err.rd_ch_;
}
inline void ArgumentDeducer::set_option(close_fds&& cfds) {
popen_->close_fds_ = cfds.close_all;
}
inline void ArgumentDeducer::set_option(preexec_func&& prefunc) {
popen_->preexec_fn_ = std::move(prefunc);
popen_->has_preexec_fn_ = true;
}
inline void Child::execute_child() {
#ifndef __USING_WINDOWS__
@ -1763,41 +1429,8 @@ namespace detail {
if (stream.err_write_ != -1 && stream.err_write_ > 2)
close(stream.err_write_);
// Close all the inherited fd's except the error write pipe
if (parent_->close_fds_) {
int max_fd = sysconf(_SC_OPEN_MAX);
if (max_fd == -1) throw OSError("sysconf failed", errno);
for (int i = 3; i < max_fd; i++) {
if (i == err_wr_pipe_) continue;
close(i);
}
}
// Change the working directory if provided
if (parent_->cwd_.length()) {
sys_ret = chdir(parent_->cwd_.c_str());
if (sys_ret == -1) throw OSError("chdir failed", errno);
}
if (parent_->has_preexec_fn_) {
parent_->preexec_fn_();
}
if (parent_->session_leader_) {
sys_ret = setsid();
if (sys_ret == -1) throw OSError("setsid failed", errno);
}
// Replace the current image with the executable
if (parent_->env_.size()) {
for (auto& kv : parent_->env_) {
setenv(kv.first.c_str(), kv.second.c_str(), 1);
}
sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data());
} else {
sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data());
}
sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data());
if (sys_ret == -1) throw OSError("execve failed", errno);
@ -1840,16 +1473,7 @@ namespace detail {
for (auto& h : handles) {
if (h == nullptr) continue;
switch (bufsiz_) {
case 0:
setvbuf(h, nullptr, _IONBF, BUFSIZ);
break;
case 1:
setvbuf(h, nullptr, _IONBF, BUFSIZ);
break;
default:
setvbuf(h, nullptr, _IOFBF, bufsiz_);
};
setvbuf(h, nullptr, _IONBF, BUFSIZ);
}
#endif
}