From 5a1c4b81297cba27f1bb6dd5bd8a1bebe619b1ef Mon Sep 17 00:00:00 2001 From: Kishi85 Date: Sun, 31 May 2026 11:45:33 +0200 Subject: [PATCH] refactor(linux): Move environment variable access to wrapper functions --- src/platform/common.h | 31 +++++++++++ src/platform/linux/input/inputtino_seat.cpp | 8 ++- src/platform/linux/kwingrab.cpp | 23 +++----- src/platform/linux/misc.cpp | 59 +++++++++++++-------- src/platform/linux/wayland.cpp | 4 +- 5 files changed, 82 insertions(+), 43 deletions(-) diff --git a/src/platform/common.h b/src/platform/common.h index 2bbcee8ff07..b2d7c24f06f 100644 --- a/src/platform/common.h +++ b/src/platform/common.h @@ -953,6 +953,14 @@ namespace platf { void restart(); + /** + * @brief Get an environment variable. + * @param name The name of the environment variable. + * @param value Reference to write the env variable value into. + * @return true if parameter value was updated, false if the environment variable didn't exist + */ + bool get_env(const std::string &name, std::string &value); + /** * @brief Set an environment variable. * @param name The name of the environment variable. @@ -961,6 +969,29 @@ namespace platf { */ int set_env(const std::string &name, const std::string &value); + /** + * @brief Append string to an environment variable if it does not already contain it. + * @param name The name of the environment variable. + * @param value The value to set the environment variable to. + * @param separator Optional separator for the new value if it is not the first one (default: "") + * @return 0 on success, non-zero on failure. + */ + inline int append_env(const std::string &name, const std::string &value, const std::string &separator = "") { + // This function is platform-independent and just uses other plaf:: functions so we can implement it inline + std::string old_value; + get_env(name, old_value); + if (old_value.contains(value)) { + // Value is already part of the string. Return success. + return 0; + } + if (old_value.empty()) { + // Environment variable does not exist or is empty. Just set it. + return set_env(name, value); + } + // Append to previous non-empty value with separator + return set_env(name, old_value + separator + value); + } + /** * @brief Unset an environment variable. * @param name The name of the environment variable. diff --git a/src/platform/linux/input/inputtino_seat.cpp b/src/platform/linux/input/inputtino_seat.cpp index beb38083803..4958fc46515 100644 --- a/src/platform/linux/input/inputtino_seat.cpp +++ b/src/platform/linux/input/inputtino_seat.cpp @@ -7,16 +7,14 @@ // local includes #include "inputtino_seat.h" +#include "src/platform/common.h" namespace platf::inputtino_seat { std::string get_target_seat() { - if (const char *seat = std::getenv("XDG_SEAT")) { - if (seat[0] != '\0') { - return seat; - } + if (std::string seat; platf::get_env("XDG_SEAT", seat) && !seat.empty()) { + return seat; } - return {}; } diff --git a/src/platform/linux/kwingrab.cpp b/src/platform/linux/kwingrab.cpp index fd31248708e..a3effacf512 100644 --- a/src/platform/linux/kwingrab.cpp +++ b/src/platform/linux/kwingrab.cpp @@ -55,7 +55,8 @@ namespace kwin { * @return True when KWin reports that the permission system is disabled. */ static bool is_permission_system_deactivated() { - return getenvstr("KWIN_WAYLAND_NO_PERMISSION_CHECKS") == "1"; + std::string v; + return platf::get_env("KWIN_WAYLAND_NO_PERMISSION_CHECKS", v) && v == "1"; } /** @@ -160,17 +161,9 @@ namespace kwin { static inline bool initialized = false; static inline bool create_file = true; - static std::string getenvstr(std::string const &key) { - char const *val = std::getenv(key.c_str()); - if (!val) { - return ""; - } - return val; - } - static std::filesystem::path get_home_dir() { // Check HOME environment variable - if (std::string homedir = getenvstr("HOME"); !homedir.empty()) { + if (std::string homedir; platf::get_env("HOME", homedir) && !homedir.empty()) { return homedir; } // Fall back to home directory from NSS passwd @@ -182,7 +175,7 @@ namespace kwin { // Follow the XDG base directory specification for user data home: // https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html std::filesystem::path xdg_data_home; - if (std::string dir = getenvstr("XDG_DATA_HOME"); !dir.empty()) { + if (std::string dir; platf::get_env("XDG_DATA_HOME", dir) && !dir.empty()) { xdg_data_home = std::filesystem::path(dir); } else { const auto homedir = get_home_dir(); @@ -222,7 +215,7 @@ namespace kwin { static bool check_kwin_system_permissions(const std::string_view &filenameprefix, const std::string_view &executablepath) { // Find data dirs to check from XDG_DATA_DIRS std::vector xdg_data_dirs; - if (const std::string e = getenvstr("XDG_DATA_DIRS"); !e.empty()) { + if (std::string e; platf::get_env("XDG_DATA_DIRS", e) && !e.empty()) { std::stringstream ss(e); std::string item; @@ -322,13 +315,13 @@ namespace kwin { screencast_permission_helper_t::setup(); } - const char *wl_name = std::getenv("WAYLAND_DISPLAY"); - if (!wl_name) { + std::string wl_name; + if (!platf::get_env("WAYLAND_DISPLAY", wl_name)) { BOOST_LOG(error) << "[kwingrab] WAYLAND_DISPLAY not set"sv; return -1; } - wl_display = wl_display_connect(wl_name); + wl_display = wl_display_connect(wl_name.c_str()); if (!wl_display) { BOOST_LOG(error) << "[kwingrab] cannot connect to Wayland display: "sv << wl_name; return -1; diff --git a/src/platform/linux/misc.cpp b/src/platform/linux/misc.cpp index 1965e9f20ae..bcf0021a620 100644 --- a/src/platform/linux/misc.cpp +++ b/src/platform/linux/misc.cpp @@ -179,24 +179,22 @@ namespace platf { std::call_once(migration_flag, []() { bool found = false; bool migrate_config = true; - const char *dir; - const char *homedir; - const char *migrate_envvar; + std::string homedir; // NOSONAR(cpp:S6010) - For get_env this needs to be a std::string reference // Get the home directory - if ((homedir = getenv("HOME")) == nullptr || strlen(homedir) == 0) { + if (!get_env("HOME", homedir) || homedir.empty()) { // If HOME is empty or not set, use the current user's home directory homedir = getpwuid(geteuid())->pw_dir; } // May be set if running under a systemd service with the ConfigurationDirectory= option set. - if ((dir = getenv("CONFIGURATION_DIRECTORY")) != nullptr && strlen(dir) > 0) { + if (std::string dir; get_env("CONFIGURATION_DIRECTORY", dir) && !dir.empty()) { found = true; config_path = fs::path(dir) / "sunshine"sv; } // Otherwise, follow the XDG base directory specification: // https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html - if (!found && (dir = getenv("XDG_CONFIG_HOME")) != nullptr && strlen(dir) > 0) { + if (std::string dir; !found && get_env("XDG_CONFIG_HOME", dir) && !dir.empty()) { found = true; config_path = fs::path(dir) / "sunshine"sv; } @@ -207,8 +205,7 @@ namespace platf { } // migrate from the old config location if necessary - migrate_envvar = getenv("SUNSHINE_MIGRATE_CONFIG"); - if (migrate_config && found && migrate_envvar && strcmp(migrate_envvar, "1") == 0) { + if (std::string migrate_envvar; migrate_config && found && get_env("SUNSHINE_MIGRATE_CONFIG", migrate_envvar) && (migrate_envvar == "1")) { std::error_code ec; fs::path old_config_path = fs::path(homedir) / ".config/sunshine"sv; if (old_config_path != config_path && fs::exists(old_config_path, ec)) { @@ -365,7 +362,9 @@ namespace platf { */ void open_url(const std::string &url) { // set working dir to user home directory - auto working_dir = boost::filesystem::path(std::getenv("HOME")); + std::string homedir; + get_env("HOME", homedir); + auto working_dir = boost::filesystem::path(homedir); std::string cmd = R"(xdg-open ")" + url + R"(")"; boost::process::v1::environment _env = boost::this_process::environment(); @@ -507,18 +506,28 @@ namespace platf { } /** - * @brief Read an environment variable as an optional string. + * @brief Read an environment variable as std::string. * - * @param name Human-readable name to assign. - * @return Environment variable value, or an empty string when unset. + * @param name Environment variable name + * @param value Environment variable value out/return parameter + * @return true if the environment variable was store in value parameter, false otherwise */ - std::string get_env(const std::string &name) { - if (const auto value = getenv(name.c_str()); value != nullptr) { - return value; + bool get_env(const std::string &name, std::string &value) { + const auto value_ = getenv(name.c_str()); // NOSONAR(cpp:S1874) - _dupenv_s is only available on WIN32 + if (value_ == nullptr) { + return false; } - return ""; + value = std::string(value_); + return true; } + /** + * @brief Set an environment variable to a specified value + * + * @param name Environment variable name + * @param value Environment variable value + * @return 0 if environment variable was successfully set + */ int set_env(const std::string &name, const std::string &value) { return setenv(name.c_str(), value.c_str(), 1); } @@ -526,18 +535,24 @@ namespace platf { /** * @brief Append a value to a separator-delimited environment variable. * - * @param name Human-readable name to assign. + * @param name Environment variable name * @param value Entry to add when it is not already present. * @param separator Character used to join or split the value. - * @return Result from updating the environment variable. + * @return 0 if environment variable was successfully appended */ int append_env(const std::string &name, const std::string &value, const std::string &separator) { - if (const std::string old_value = get_env(name); !old_value.contains(value)) { + if (std::string old_value; get_env(name, old_value) && !old_value.contains(value)) { return set_env(name, old_value.empty() ? value : old_value + separator + value); } return 0; } + /** + * @brief Unset an environment variable + * + * @param name Environment variable name + * @return 0 if environment variable was successfully unset + */ int unset_env(const std::string &name) { return unsetenv(name.c_str()); } @@ -1279,13 +1294,13 @@ namespace platf { window_system = window_system_e::NONE; #ifdef SUNSHINE_BUILD_WAYLAND - if (std::getenv("WAYLAND_DISPLAY")) { + if (std::string v; get_env("WAYLAND_DISPLAY", v)) { window_system = window_system_e::WAYLAND; } #endif #if defined(SUNSHINE_BUILD_X11) || defined(SUNSHINE_BUILD_CUDA) - if (std::getenv("DISPLAY") && window_system != window_system_e::WAYLAND) { - if (std::getenv("WAYLAND_DISPLAY")) { + if (std::string v; get_env("DISPLAY", v) && window_system != window_system_e::WAYLAND) { + if (get_env("WAYLAND_DISPLAY", v)) { BOOST_LOG(warning) << "Wayland detected, yet sunshine will use X11 for screencasting, screencasting will only work on XWayland applications"sv; } diff --git a/src/platform/linux/wayland.cpp b/src/platform/linux/wayland.cpp index cdef9ae78d6..21bebe08c4c 100644 --- a/src/platform/linux/wayland.cpp +++ b/src/platform/linux/wayland.cpp @@ -54,7 +54,9 @@ namespace wl { int display_t::init(const char *display_name) { if (!display_name) { - display_name = std::getenv("WAYLAND_DISPLAY"); + if (std::string v; platf::get_env("WAYLAND_DISPLAY", v)) { + display_name = v.c_str(); + } } if (!display_name) {