From 917faea7458195ae34351b12bc636a0027126b4b Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Wed, 1 Nov 2023 14:06:52 +0000 Subject: [PATCH 01/10] Add new functions for manipulating the environment This adds functions for using collections with the environment. It allows for conveniently retreiving and setting the environment variables via a map, rather than individually. This also adds a clearenv and printenv for completeness. Signed-off-by: Michael Carroll --- include/gz/utils/Environment.hh | 44 ++++++++++++++++++++- src/Environment.cc | 70 +++++++++++++++++++++++++++++++++ src/Environment_TEST.cc | 43 +++++++++++++++++++- 3 files changed, 155 insertions(+), 2 deletions(-) diff --git a/include/gz/utils/Environment.hh b/include/gz/utils/Environment.hh index 8ac06a4..a6dfa5f 100644 --- a/include/gz/utils/Environment.hh +++ b/include/gz/utils/Environment.hh @@ -22,6 +22,7 @@ #include #include +#include namespace gz { @@ -66,7 +67,48 @@ bool GZ_UTILS_VISIBLE setenv( /// \return True if the variable was unset or false otherwise. bool GZ_UTILS_VISIBLE unsetenv(const std::string &_name); -} +/// \brief Unset all environment variables +/// +/// Note: This function is not thread-safe and should not be called +/// concurrently with `env` or `setenv` +/// +/// \return True if the environment was unset or false otherwise. +bool GZ_UTILS_VISIBLE clearenv(); + +/// \brief Type alias for a collection of environment variables +using EnvironmentMap = std::unordered_map; + +/// \brief Retrieve all current environment variables +/// +/// Note: This function is not thread-safe and should not be called +/// concurrently with `setenv` or `unsetenv` +/// +/// \return A collection of current environment variables +EnvironmentMap GZ_UTILS_VISIBLE env(); + +/// \brief Set the environment variable '_name'. +/// +/// Note: On Windows setting an empty string (_value=="") +/// is the equivalent of unsetting the variable. +// +/// Note: This function is not thread-safe and should not be called +/// concurrently with `env` or `unsetenv` +/// +/// \param[in] _vars Collection of environment variables to set +/// \return True if all variables were set or false otherwise. +bool GZ_UTILS_VISIBLE setenv(const EnvironmentMap &_vars); + +/// \brief Print the entire current environment to a string +/// +/// This prints each variable in the form KEY=VALUE\n +/// +/// Note: This function is not thread-safe and should not be called +/// concurrently with `setenv` or `unsetenv` +/// +/// \return A string containing all environment variables +std::string printenv(); + +} // namespace GZ_UTILS_VERSION_NAMESPACE } // namespace utils } // namespace gz diff --git a/src/Environment.cc b/src/Environment.cc index 63e0746..3300845 100644 --- a/src/Environment.cc +++ b/src/Environment.cc @@ -20,6 +20,8 @@ #include #include +extern char ** environ; + namespace gz { namespace utils @@ -98,6 +100,74 @@ bool unsetenv(const std::string &_name) #endif return true; } + +///////////////////////////////////////////////// +bool clearenv() +{ + ::clearenv(); + return true; + /* + bool success = true; + for (const auto &[key, value] : env()) + { + success &= unsetenv(key); + } + return success; + */ } + +///////////////////////////////////////////////// +EnvironmentMap env() +{ + // Portable method for reading environment variables + // Ref: https://stackoverflow.com/a/71483564/460065 + char **currentEnv {nullptr}; +#if defined(WIN) && (_MSC_VER >= 1900) + currentEnv = *__p_environ(); +#else + currentEnv = environ; +#endif + + // In the case that clearenv() was just called + // currentEnv will be nullptr + if (currentEnv == nullptr) + return {}; + + EnvironmentMap ret; + for (; *currentEnv; ++currentEnv) + { + std::string var(*currentEnv); + auto key = var.substr(0, var.find('=')); + var.erase(0, var.find('=') + 1); + ret[key] = var; + } + return ret; } + +///////////////////////////////////////////////// +bool setenv(const EnvironmentMap &_vars) +{ + bool success = true; + for (const auto &[key, value] : _vars) + { + success &= setenv(key, value); + } + return success; +} + +///////////////////////////////////////////////// +std::string printenv() +{ + std::string ret; + for (const auto &[key, value] : env()) + { + ret.append(key); + ret.append("="); + ret.append(value); + ret.append("\n"); + } + return ret; } +} // namespace GZ_UTILS_VERSION_NAMESPACE +} // namespace utils +} // namespace gz diff --git a/src/Environment_TEST.cc b/src/Environment_TEST.cc index 8168153..24da134 100644 --- a/src/Environment_TEST.cc +++ b/src/Environment_TEST.cc @@ -26,6 +26,8 @@ using namespace gz; ///////////////////////////////////////////////// TEST(Environment, emptyENV) { + gz::utils::clearenv(); + std::string var; EXPECT_FALSE(utils::env("!!SHOULD_NOT_EXIST!!", var)); EXPECT_TRUE(var.empty()); @@ -34,6 +36,8 @@ TEST(Environment, emptyENV) ///////////////////////////////////////////////// TEST(Environment, envSet) { + gz::utils::clearenv(); + const auto key = "GZ_ENV_SET"; ASSERT_TRUE(utils::setenv(key, "VALUE")); @@ -67,6 +71,8 @@ TEST(Environment, envSet) ///////////////////////////////////////////////// TEST(Environment, envUnset) { + gz::utils::clearenv(); + const auto key = "GZ_ENV_UNSET"; ASSERT_TRUE(utils::unsetenv(key)); @@ -94,8 +100,10 @@ TEST(Environment, envUnset) } ///////////////////////////////////////////////// -TEST(Util_TEST, envSetEmpty) +TEST(Environment, envSetEmpty) { + gz::utils::clearenv(); + const auto key = "GZ_ENV_SET_EMPTY"; ASSERT_TRUE(utils::setenv(key, "")); @@ -133,3 +141,36 @@ TEST(Util_TEST, envSetEmpty) } ASSERT_TRUE(utils::unsetenv(key)); } + +///////////////////////////////////////////////// +TEST(Environment, envGetCollection) +{ + gz::utils::clearenv(); + auto currentEnv = gz::utils::env(); + EXPECT_EQ(currentEnv.size(), 0); + + ASSERT_TRUE(gz::utils::setenv("GZ_FOO_KEY", "GZ_FOO_VAL")); + ASSERT_TRUE(gz::utils::setenv("GZ_BAR_KEY", "GZ_BAR_VAL")); + ASSERT_TRUE(gz::utils::setenv("GZ_BAZ_KEY", "GZ_BAZ_VAL")); + + currentEnv = gz::utils::env(); + EXPECT_EQ(currentEnv.size(), 3); + + EXPECT_EQ(currentEnv["GZ_FOO_KEY"], "GZ_FOO_VAL"); + EXPECT_EQ(currentEnv["GZ_BAR_KEY"], "GZ_BAR_VAL"); + EXPECT_EQ(currentEnv["GZ_BAZ_KEY"], "GZ_BAZ_VAL"); +} + +///////////////////////////////////////////////// +TEST(Environment, printenv) +{ + gz::utils::clearenv(); + EXPECT_EQ(gz::utils::printenv(), ""); + + ASSERT_TRUE(gz::utils::setenv("GZ_FOO_KEY", "GZ_FOO_VAL")); + ASSERT_TRUE(gz::utils::setenv("GZ_BAR_KEY", "GZ_BAR_VAL")); + ASSERT_TRUE(gz::utils::setenv("GZ_BAZ_KEY", "GZ_BAZ_VAL")); + + EXPECT_EQ(gz::utils::printenv(), + "GZ_BAZ_KEY=GZ_BAZ_VAL\nGZ_BAR_KEY=GZ_BAR_VAL\nGZ_FOO_KEY=GZ_FOO_VAL\n"); +} From 65f81d74d49616a374301a58a610420181d0d8fe Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Wed, 1 Nov 2023 15:22:19 +0000 Subject: [PATCH 02/10] Fix Windows and macOS clearenv implementation Signed-off-by: Michael Carroll --- include/gz/utils/Environment.hh | 4 ++-- src/Environment.cc | 14 ++++++++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/include/gz/utils/Environment.hh b/include/gz/utils/Environment.hh index a6dfa5f..9a7a062 100644 --- a/include/gz/utils/Environment.hh +++ b/include/gz/utils/Environment.hh @@ -106,8 +106,8 @@ bool GZ_UTILS_VISIBLE setenv(const EnvironmentMap &_vars); /// concurrently with `setenv` or `unsetenv` /// /// \return A string containing all environment variables -std::string printenv(); - +/// NOLINTNEXTLINE - This is incorrectly parsed as a global variable +std::string GZ_UTILS_VISIBLE printenv(); } // namespace GZ_UTILS_VERSION_NAMESPACE } // namespace utils } // namespace gz diff --git a/src/Environment.cc b/src/Environment.cc index 3300845..7c2f02f 100644 --- a/src/Environment.cc +++ b/src/Environment.cc @@ -104,16 +104,22 @@ bool unsetenv(const std::string &_name) ///////////////////////////////////////////////// bool clearenv() { - ::clearenv(); - return true; - /* bool success = true; +#if __linux__ + if (0 != ::clearenv()) + { + success = false; + } +#else + // Windows and macOS don't have clearenv + // so iterate and clear one-by-one for (const auto &[key, value] : env()) { success &= unsetenv(key); } +#endif return success; - */ + } ///////////////////////////////////////////////// From 55451cc21474ea8f2f862fef80c299b08ddcfc84 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Wed, 1 Nov 2023 16:07:56 +0000 Subject: [PATCH 03/10] Sort the environment variables Signed-off-by: Michael Carroll --- src/Environment.cc | 10 +++++++++- src/Environment_TEST.cc | 3 ++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/Environment.cc b/src/Environment.cc index 7c2f02f..b18824f 100644 --- a/src/Environment.cc +++ b/src/Environment.cc @@ -17,8 +17,10 @@ #include +#include #include #include +#include extern char ** environ; @@ -165,7 +167,13 @@ bool setenv(const EnvironmentMap &_vars) std::string printenv() { std::string ret; - for (const auto &[key, value] : env()) + // Variables are in an unordered_map as we generally don't + // care, but for printing sort for consistent display + auto currentEnv = env(); + auto sorted = std::vector>( + currentEnv.begin(), currentEnv.end()); + std::sort(sorted.begin(), sorted.end()); + for (const auto &[key, value] : sorted) { ret.append(key); ret.append("="); diff --git a/src/Environment_TEST.cc b/src/Environment_TEST.cc index 24da134..6d09923 100644 --- a/src/Environment_TEST.cc +++ b/src/Environment_TEST.cc @@ -171,6 +171,7 @@ TEST(Environment, printenv) ASSERT_TRUE(gz::utils::setenv("GZ_BAR_KEY", "GZ_BAR_VAL")); ASSERT_TRUE(gz::utils::setenv("GZ_BAZ_KEY", "GZ_BAZ_VAL")); + // Always returned in sorted order EXPECT_EQ(gz::utils::printenv(), - "GZ_BAZ_KEY=GZ_BAZ_VAL\nGZ_BAR_KEY=GZ_BAR_VAL\nGZ_FOO_KEY=GZ_FOO_VAL\n"); + "GZ_BAR_KEY=GZ_BAR_VAL\nGZ_BAZ_KEY=GZ_BAZ_VAL\nGZ_FOO_KEY=GZ_FOO_VAL\n"); } From 9d7eb95eb76d65e9d9ef17785e600adc2005fb3d Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Wed, 1 Nov 2023 16:30:22 +0000 Subject: [PATCH 04/10] Avoid __p_error on Windows Signed-off-by: Michael Carroll --- src/Environment.cc | 45 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/src/Environment.cc b/src/Environment.cc index b18824f..a0f62ba 100644 --- a/src/Environment.cc +++ b/src/Environment.cc @@ -127,21 +127,47 @@ bool clearenv() ///////////////////////////////////////////////// EnvironmentMap env() { - // Portable method for reading environment variables - // Ref: https://stackoverflow.com/a/71483564/460065 - char **currentEnv {nullptr}; -#if defined(WIN) && (_MSC_VER >= 1900) - currentEnv = *__p_environ(); -#else - currentEnv = environ; -#endif + EnvironmentMap ret; +#ifdef _WIN32 + DWORD environment_block_size = GetEnvironmentBlockSize(); + + // Allocate a buffer to store the environment block. + LPCH env_buf = (LPCH)malloc(environment_block_size); + if (env_buf == nullptr) { + return {}; + } + // Get the environment block. + if (!GetEnvironmentVariables(env_buf, environment_block_size)) { + free(env_buf); + return {}; + } + // Parse the environment block into an unordered_map. + LPCH env_var = env_buf; + while (*env_var != '\0') { + // Split the environment variable into a key-value pair. + char* equal_sign = strchr(env_var, '='); + if (equal_sign == nullptr) { + continue; + } + + // Get the key and value of the environment variable. + std::string key(env_var, equal_sign - env_var); + std::string value(equal_sign + 1); + + // Add the key-value pair to the unordered_map. + ret[key] = value; + // Advance to the next environment variable. + env_var = equal_sign + 1; + } + free(env_buf); +#else + char **currentEnv = environ; // In the case that clearenv() was just called // currentEnv will be nullptr if (currentEnv == nullptr) return {}; - EnvironmentMap ret; for (; *currentEnv; ++currentEnv) { std::string var(*currentEnv); @@ -149,6 +175,7 @@ EnvironmentMap env() var.erase(0, var.find('=') + 1); ret[key] = var; } +#endif return ret; } From c0f05423790f437b5d6fce989694e686f1207b17 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Wed, 1 Nov 2023 16:38:29 +0000 Subject: [PATCH 05/10] Fix headers Signed-off-by: Michael Carroll --- src/Environment.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Environment.cc b/src/Environment.cc index a0f62ba..ba4a239 100644 --- a/src/Environment.cc +++ b/src/Environment.cc @@ -22,7 +22,14 @@ #include #include +#ifdef _WIN32 +#include +#include +#endif + +#ifndef _WIN32 extern char ** environ; +#endif namespace gz { From 1756d83b07e28ea0a9f3f55bbc27f7551a4912b8 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Wed, 1 Nov 2023 16:50:50 +0000 Subject: [PATCH 06/10] Back to original implementation Signed-off-by: Michael Carroll --- src/Environment.cc | 49 ++++++++-------------------------------------- 1 file changed, 8 insertions(+), 41 deletions(-) diff --git a/src/Environment.cc b/src/Environment.cc index ba4a239..e3108e5 100644 --- a/src/Environment.cc +++ b/src/Environment.cc @@ -22,12 +22,7 @@ #include #include -#ifdef _WIN32 -#include -#include -#endif - -#ifndef _WIN32 +#ifndef _win32 extern char ** environ; #endif @@ -134,47 +129,20 @@ bool clearenv() ///////////////////////////////////////////////// EnvironmentMap env() { - EnvironmentMap ret; + // Portable method for reading environment variables + // Ref: https://stackoverflow.com/a/71483564/460065 + char **currentEnv {nullptr}; #ifdef _WIN32 - DWORD environment_block_size = GetEnvironmentBlockSize(); - - // Allocate a buffer to store the environment block. - LPCH env_buf = (LPCH)malloc(environment_block_size); - if (env_buf == nullptr) { - return {}; - } - // Get the environment block. - if (!GetEnvironmentVariables(env_buf, environment_block_size)) { - free(env_buf); - return {}; - } - // Parse the environment block into an unordered_map. - LPCH env_var = env_buf; - while (*env_var != '\0') { - // Split the environment variable into a key-value pair. - char* equal_sign = strchr(env_var, '='); - if (equal_sign == nullptr) { - continue; - } - - // Get the key and value of the environment variable. - std::string key(env_var, equal_sign - env_var); - std::string value(equal_sign + 1); - - // Add the key-value pair to the unordered_map. - ret[key] = value; - - // Advance to the next environment variable. - env_var = equal_sign + 1; - } - free(env_buf); + currentEnv = *__p_environ(); #else - char **currentEnv = environ; + currentEnv = environ; +#endif // In the case that clearenv() was just called // currentEnv will be nullptr if (currentEnv == nullptr) return {}; + EnvironmentMap ret; for (; *currentEnv; ++currentEnv) { std::string var(*currentEnv); @@ -182,7 +150,6 @@ EnvironmentMap env() var.erase(0, var.find('=') + 1); ret[key] = var; } -#endif return ret; } From 72b193b88af9734fdafdc70b6e74af7b388be177 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Wed, 1 Nov 2023 19:21:29 +0000 Subject: [PATCH 07/10] Fix Windows implementation Signed-off-by: Michael Carroll --- src/Environment.cc | 42 ++++++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/src/Environment.cc b/src/Environment.cc index e3108e5..18858a5 100644 --- a/src/Environment.cc +++ b/src/Environment.cc @@ -22,7 +22,12 @@ #include #include -#ifndef _win32 +#ifdef _WIN32 +#include +#include +#endif + +#ifndef _WIN32 extern char ** environ; #endif @@ -129,27 +134,40 @@ bool clearenv() ///////////////////////////////////////////////// EnvironmentMap env() { - // Portable method for reading environment variables - // Ref: https://stackoverflow.com/a/71483564/460065 - char **currentEnv {nullptr}; + EnvironmentMap ret; + + // Helper function to split KEY=VAL + auto split = [](const std::string &_inp) + { + return std::make_pair( + _inp.substr(0, _inp.find('=')), + _inp.substr(_inp.find('='))); + }; + #ifdef _WIN32 - currentEnv = *__p_environ(); + LPCH currentEnv = GetEnvironmentStrings(); + if (currentEnv == nullptr) + return {}; + + LPCH env_var = env_buf; + while (*env_var != '\0') + { + ret.emplace(split(*env_var)); + env_var += strlen(env_var) + 1; + } + FreeEnvironmentStrings(currentEnv); #else - currentEnv = environ; -#endif + char **currentEnv = environ; // In the case that clearenv() was just called // currentEnv will be nullptr if (currentEnv == nullptr) return {}; - EnvironmentMap ret; for (; *currentEnv; ++currentEnv) { - std::string var(*currentEnv); - auto key = var.substr(0, var.find('=')); - var.erase(0, var.find('=') + 1); - ret[key] = var; + ret.emplace(split(*currentEnv)); } +#endif return ret; } From e153ac9760d5390bc93b8f46c7d1264b419de65e Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Wed, 1 Nov 2023 19:37:23 +0000 Subject: [PATCH 08/10] Off by one Signed-off-by: Michael Carroll --- src/Environment.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Environment.cc b/src/Environment.cc index 18858a5..e06ab88 100644 --- a/src/Environment.cc +++ b/src/Environment.cc @@ -141,7 +141,7 @@ EnvironmentMap env() { return std::make_pair( _inp.substr(0, _inp.find('=')), - _inp.substr(_inp.find('='))); + _inp.substr(_inp.find('=') + 1)); }; #ifdef _WIN32 From 0d549f83ddc9c4af756e2292de53271816e87d57 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Wed, 1 Nov 2023 20:20:26 +0000 Subject: [PATCH 09/10] One more try Signed-off-by: Michael Carroll --- src/Environment.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Environment.cc b/src/Environment.cc index e06ab88..0aede95 100644 --- a/src/Environment.cc +++ b/src/Environment.cc @@ -149,10 +149,10 @@ EnvironmentMap env() if (currentEnv == nullptr) return {}; - LPCH env_var = env_buf; + LPCH env_var = currentEnv; while (*env_var != '\0') { - ret.emplace(split(*env_var)); + ret.emplace(split(env_var)); env_var += strlen(env_var) + 1; } FreeEnvironmentStrings(currentEnv); From b9fc005feafa29154a8ba7b5094b5ed64f6bf749 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Thu, 2 Nov 2023 14:48:00 +0000 Subject: [PATCH 10/10] Turns out I was making this too complicated Signed-off-by: Michael Carroll --- src/Environment.cc | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/src/Environment.cc b/src/Environment.cc index 0aede95..4d86e0b 100644 --- a/src/Environment.cc +++ b/src/Environment.cc @@ -144,20 +144,12 @@ EnvironmentMap env() _inp.substr(_inp.find('=') + 1)); }; + char **currentEnv = nullptr; #ifdef _WIN32 - LPCH currentEnv = GetEnvironmentStrings(); - if (currentEnv == nullptr) - return {}; - - LPCH env_var = currentEnv; - while (*env_var != '\0') - { - ret.emplace(split(env_var)); - env_var += strlen(env_var) + 1; - } - FreeEnvironmentStrings(currentEnv); + currentEnv = *__p__environ(); #else - char **currentEnv = environ; + currentEnv = environ; +#endif // In the case that clearenv() was just called // currentEnv will be nullptr if (currentEnv == nullptr) @@ -167,7 +159,6 @@ EnvironmentMap env() { ret.emplace(split(*currentEnv)); } -#endif return ret; }