Implemented smartctl process environment changes without changing the main program's environment.

Removed ScopedEnv class - it's not thread-safe and very easy to misuse.
This commit is contained in:
Alexander Shaduri
2021-04-21 16:48:46 +04:00
parent 3b7576d95c
commit b78f8573b5
2 changed files with 11 additions and 79 deletions
+8 -5
View File
@@ -25,7 +25,6 @@ Copyright:
#include "hz/process_signal.h" // hz::process_signal_send, win32's W*
#include "hz/debug.h"
#include "hz/env_tools.h" // hz::ScopedEnv
#include "async_command_executor.h"
@@ -147,7 +146,7 @@ bool AsyncCommandExecutor::execute()
// Set the locale for a child to Classic - otherwise it may mangle the output.
// TODO: make this controllable.
// TODO: Disable this for JSON format.
bool change_lang = true;
#ifdef _WIN32
// LANG is posix-only, so it has no effect on win32.
@@ -156,15 +155,19 @@ bool AsyncCommandExecutor::execute()
change_lang = false;
#endif
hz::ScopedEnv lang_env("LANG", "C", change_lang);
std::unique_ptr<gchar*, decltype(&g_strfreev)> child_env(g_get_environ(), &g_strfreev);
if (change_lang) {
child_env.reset(g_environ_setenv(child_env.release(), "LANG", "C", TRUE));
}
std::vector<std::string> envp = Glib::ArrayHandler<std::string>::array_to_vector(child_env.release(),
Glib::OWNERSHIP_DEEP);
debug_out_info("app", DBG_FUNC_MSG << "Executing \"" << cmd << "\".\n");
// Execute the command
try {
Glib::spawn_async_with_pipes(Glib::get_current_dir(), argvp, std::vector<std::string>(),
Glib::spawn_async_with_pipes(Glib::get_current_dir(), argvp, envp,
Glib::SpawnFlags::SPAWN_SEARCH_PATH | Glib::SpawnFlags::SPAWN_DO_NOT_REAP_CHILD,
Glib::SlotSpawnChildSetup(),
&this->pid_, nullptr, &fd_stdout_, &fd_stderr_);
+3 -74
View File
@@ -130,6 +130,7 @@ inline bool env_get_value(const std::string& name, std::string& value)
/// Set environment variable. If \c overwrite is false, the value won't
/// be overwritten if it already exists.
/// NOTE: This function is not thread-safe in GLibc and should only be invoked at the start of the program.
/// \return true if the value was written successfully.
inline bool env_set_value(const std::string& name, const std::string& value, bool overwrite = true)
{
@@ -179,7 +180,8 @@ inline bool env_set_value(const std::string& name, const std::string& value, boo
/// Unset an environment variable
/// Unset an environment variable.
/// NOTE: This function is not thread-safe in GLibc and should only be invoked at the start of the program.
inline bool env_unset_value(const std::string& name)
{
if (name.empty() || name.find('=') != std::string::npos)
@@ -213,79 +215,6 @@ inline bool env_unset_value(const std::string& name)
/// Temporarily change a value of an environment variable (for as long
/// as an object of this class exists).
class ScopedEnv {
public:
/// Constructor.
/// \param name variable name
/// \param value variable value to set
/// \param do_change if false, no operation will be performed. This is useful if you need
/// to conditionally set a variable (you can't practically declare a scoped variable inside
/// a conditional block to be used outside it).
/// \param overwrite if false and the variable already exists, don't change it.
ScopedEnv(std::string name, const std::string& value, bool do_change = true, bool overwrite = true)
: name_(std::move(name)), do_change_(do_change), old_set_(false), error_(false)
{
if (do_change_) {
old_set_ = env_get_value(name_, old_value_);
if (old_set_ && !overwrite) {
do_change_ = false;
} else {
error_ = !env_set_value(name_, value, true);
}
}
}
/// Destructor, changes back the variable to the old value
~ScopedEnv()
{
if (do_change_) {
if (old_set_) {
env_set_value(name_, old_value_, true);
} else {
env_unset_value(name_);
}
}
}
/// If true, there was an error setting the value.
[[nodiscard]] bool bad() const
{
return error_;
}
/// Check if there was a value before we set it
[[nodiscard]] bool get_old_set() const
{
return old_set_;
}
/// Get the old variable value
[[nodiscard]] std::string get_old_value() const
{
return old_value_;
}
private:
std::string name_; ///< Variable name
std::string old_value_; ///< Old value
bool do_change_ = false; ///< If false, don't do anything
bool old_set_ = false; ///< If false, there was no variable before we set it
bool error_ = false; ///< If true, there was an error setting the value
};
} // ns