Rewrite handling of subprocess environment variables

This commit is contained in:
TheAssassin 2022-07-19 03:39:28 +02:00
parent 0ae13c0875
commit 98aa3479d0
9 changed files with 104 additions and 58 deletions

View File

@ -28,11 +28,25 @@ namespace linuxdeploy {
public:
/**
* Create a child process.
* This constructor passes the system environment to the child process.
* @param args parameters for process
* @param env additional environment variables (current environment will be copied)
*/
process(std::initializer_list<std::string> args);
/**
* Create a child process.
* @param args parameters for process
* @param env map of all environment variables (caller must include the existing vars if they want to)
*/
process(std::initializer_list<std::string> args, const subprocess_env_map_t& env);
/**
* Create a child process.
* This constructor passes the system environment to the child process.
* @param args parameters for process
*/
process(const std::vector<std::string>& args);
/**
* Create a child process.
* @param args parameters for process

View File

@ -9,20 +9,23 @@
// local headers
#include "subprocess_result.h"
#include "util.h"
namespace linuxdeploy {
namespace subprocess {
typedef std::unordered_map<std::string, std::string> subprocess_env_map_t;
class subprocess {
private:
std::vector<std::string> args_{};
std::unordered_map<std::string, std::string> env_{};
public:
subprocess(std::initializer_list<std::string> args, subprocess_env_map_t env = {});
explicit subprocess(std::initializer_list<std::string> args);
explicit subprocess(std::vector<std::string> args, subprocess_env_map_t env = {});
explicit subprocess(std::initializer_list<std::string> args, subprocess_env_map_t env);
explicit subprocess(std::vector<std::string> args);
explicit subprocess(std::vector<std::string> args, subprocess_env_map_t env);
subprocess_result run() const;

View File

@ -0,0 +1,13 @@
#include <string>
#include <unordered_map>
#include <vector>
namespace linuxdeploy::subprocess {
using subprocess_env_map_t = std::unordered_map<std::string, std::string>;
/**
* Parse current process's environment and store the variables in a map.
* @return map of environment variables
*/
subprocess_env_map_t get_environment();
}

View File

@ -250,8 +250,8 @@ namespace linuxdeploy {
} else {
ldLog() << "Calling strip on library" << filePath << std::endl;
subprocess::subprocess_env_map_t env;
env.insert(std::make_pair(std::string("LC_ALL"), std::string("C")));
auto env = subprocess::get_environment();
env["LC_ALL"] = "C";
subprocess::subprocess proc({stripPath, filePath.string()}, env);

View File

@ -192,7 +192,7 @@ namespace linuxdeploy {
std::vector<bf::path> paths;
subprocess::subprocess_env_map_t env;
auto env = subprocess::get_environment();
env["LC_ALL"] = "C";
// workaround for https://sourceware.org/bugzilla/show_bug.cgi?id=25263

View File

@ -5,12 +5,14 @@ add_library(linuxdeploy_subprocess STATIC
subprocess_result.cpp
process.cpp
pipe_reader.cpp
util.cpp
${headers_dir}/subprocess.h
${headers_dir}/subprocess_result.h
${headers_dir}/process.h
${headers_dir}/pipe_reader.h
${headers_dir}/util.h
)
target_include_directories(linuxdeploy_subprocess PUBLIC ${PROJECT_SOURCE_DIR}/include)
add_executable(subprocess_demo subprocess_demo.cpp)
target_link_libraries(subprocess_demo PUBLIC linuxdeploy_subprocess)
target_link_libraries(subprocess_demo PUBLIC linuxdeploy_subprocess ${BOOST_LIBS})

View File

@ -1,17 +1,15 @@
// system headers
#include <algorithm>
#include <iostream>
#include <memory>
#include <sstream>
#include <stdexcept>
#include <utility>
#include <unistd.h>
#include <memory.h>
#include <wait.h>
#include <sstream>
// local headers
#include "linuxdeploy/subprocess/process.h"
#include "linuxdeploy/subprocess/subprocess.h"
#include "linuxdeploy/subprocess/util.h"
#include "linuxdeploy/util/assert.h"
// shorter than using namespace ...
@ -32,50 +30,6 @@ namespace {
return rv;
}
std::vector<char*> make_env_vector(const subprocess_env_map_t& env) {
std::vector<char*> rv;
// first, copy existing environment
// we cannot reserve space in the vector unfortunately, as we don't know the size of environ before the iteration
if (environ != nullptr) {
for (auto** current_env_var = environ; *current_env_var != nullptr; ++current_env_var) {
rv.emplace_back(strdup(*current_env_var));
}
}
// add own environment variables, overwriting existing ones if necessary
for (const auto& env_var : env) {
const auto& key = env_var.first;
const auto& value = env_var.second;
auto predicate = [&key](char* existing_env_var) {
char* equal_sign = strstr(existing_env_var, "=");
if (equal_sign == nullptr) {
throw std::runtime_error{"no equal sign in environment variable"};
}
return strncmp(existing_env_var, key.c_str(), std::distance(equal_sign, existing_env_var)) == 0;
};
// delete existing env var, if any
rv.erase(std::remove_if(rv.begin(), rv.end(), predicate), rv.end());
// insert new value
std::ostringstream oss;
oss << key;
oss << "=";
oss << value;
rv.emplace_back(strdup(oss.str().c_str()));
}
// exec*e want a nullptr-terminated array
rv.emplace_back(nullptr);
return rv;
}
int check_waitpid_status(int status) {
if (WIFSIGNALED(status) != 0) {
// TODO: consider treating child exit caused by signals separately
@ -100,6 +54,27 @@ namespace {
}
}
using env_vector_t = std::vector<char*>;
/**
* Convert a map of environment variables to a vector usable within
* @param map
* @return
*/
env_vector_t env_map_to_vector(const subprocess_env_map_t& map) {
env_vector_t out;
for (const auto& [name, value] : map) {
std::ostringstream oss;
oss << name << '=' << value;
out.emplace_back(strdup(oss.str().c_str()));
}
// must be null terminated, of course
out.emplace_back(nullptr);
return out;
}
}
int process::pid() const {
@ -114,9 +89,15 @@ int process::stderr_fd() const {
return stderr_fd_;
}
process::process(std::initializer_list<std::string> args)
: process(std::vector<std::string>(args), get_environment()) {}
process::process(std::initializer_list<std::string> args, const subprocess_env_map_t& env)
: process(std::vector<std::string>(args), env) {}
process::process(const std::vector<std::string>& args)
: process(args, get_environment()) {}
process::process(const std::vector<std::string>& args, const subprocess_env_map_t& env) {
// preconditions
util::assert::assert_not_empty(args);
@ -177,7 +158,7 @@ process::process(const std::vector<std::string>& args, const subprocess_env_map_
// prepare arguments for exec*
auto exec_args = make_args_vector(args);
auto exec_env = make_env_vector(env);
auto exec_env = env_map_to_vector(env);
// call subprocess
execvpe(args.front().c_str(), exec_args.data(), exec_env.data());

View File

@ -15,9 +15,16 @@
namespace linuxdeploy {
namespace subprocess {
subprocess::subprocess(std::initializer_list<std::string> args)
: subprocess(std::vector<std::string>(args), get_environment()) {}
subprocess::subprocess(std::initializer_list<std::string> args, subprocess_env_map_t env)
: subprocess(std::vector<std::string>(args), std::move(env)) {}
subprocess::subprocess(std::vector<std::string> args)
: args_(std::move(args)), env_(get_environment()) {}
subprocess::subprocess(std::vector<std::string> args, subprocess_env_map_t env)
: args_(std::move(args)), env_(std::move(env)) {
// preconditions

26
src/subprocess/util.cpp Normal file
View File

@ -0,0 +1,26 @@
// global headers
#include <unistd.h>
// local headers
#include "linuxdeploy/subprocess/util.h"
#include "linuxdeploy/util/misc.h"
namespace linuxdeploy::subprocess {
subprocess_env_map_t get_environment() {
subprocess_env_map_t result;
// first, copy existing environment
// we cannot reserve space in the vector unfortunately, as we don't know the size of environ before the iteration
if (environ != nullptr) {
for (auto** current_env_var = environ; *current_env_var != nullptr; ++current_env_var) {
std::string current_env_var_str(*current_env_var);
const auto first_eq = current_env_var_str.find_first_of('=');
const auto env_var_name = current_env_var_str.substr(0, first_eq);
const auto env_var_value = current_env_var_str.substr(first_eq);
result[env_var_name] = env_var_value;
}
}
return result;
}
}