From 0e7ef51d5ccae1205cbffd272fa6b145b4fbee8c Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 26 Jul 2022 23:26:37 -0500 Subject: [PATCH] refactor: tr_env_get_string() now returns a std::string (#3527) --- libtransmission/platform.cc | 37 ++++++---------- libtransmission/utils.cc | 43 +++++++------------ libtransmission/utils.h | 4 +- libtransmission/web.cc | 5 +-- .../subprocess-test-program.cc | 3 +- tests/libtransmission/utils-test.cc | 18 +++----- utils/remote.cc | 8 ++-- 7 files changed, 45 insertions(+), 73 deletions(-) diff --git a/libtransmission/platform.cc b/libtransmission/platform.cc index 8fe73cbf4..2ff496849 100644 --- a/libtransmission/platform.cc +++ b/libtransmission/platform.cc @@ -76,11 +76,9 @@ static auto win32_get_known_folder(REFKNOWNFOLDERID folder_id) static std::string getHomeDir() { - if (auto* const dir = tr_env_get_string("HOME", nullptr); dir != nullptr) + if (auto dir = tr_env_get_string("HOME"sv); !std::empty(dir)) { - auto ret = std::string{ dir }; - tr_free(dir); - return ret; + return dir; } #ifdef _WIN32 @@ -108,11 +106,9 @@ static std::string getHomeDir() static std::string xdgConfigHome() { - if (auto* const dir = tr_env_get_string("XDG_CONFIG_HOME", nullptr); dir != nullptr) + if (auto dir = tr_env_get_string("XDG_CONFIG_HOME"sv); !std::empty(dir)) { - auto ret = std::string{ dir }; - tr_free(dir); - return ret; + return dir; } return fmt::format("{:s}/.config"sv, getHomeDir()); @@ -120,9 +116,9 @@ static std::string xdgConfigHome() char* tr_getDefaultConfigDir(char const* appname) { - if (auto* dir = tr_env_get_string("TRANSMISSION_HOME", nullptr); dir != nullptr) + if (auto dir = tr_env_get_string("TRANSMISSION_HOME"sv); !std::empty(dir)) { - return dir; + return tr_strvDup(dir); } if (tr_str_is_empty(appname)) @@ -221,18 +217,14 @@ static bool isWebClientDir(std::string_view path) std::string tr_getWebClientDir([[maybe_unused]] tr_session const* session) { - if (auto* const dir = tr_env_get_string("CLUTCH_HOME", nullptr); dir != nullptr) + if (auto dir = tr_env_get_string("CLUTCH_HOME"sv); !std::empty(dir)) { - auto ret = std::string{ dir }; - tr_free(dir); - return ret; + return dir; } - if (auto* const dir = tr_env_get_string("TRANSMISSION_WEB_HOME", nullptr); dir != nullptr) + if (auto dir = tr_env_get_string("TRANSMISSION_WEB_HOME"sv); !std::empty(dir)) { - auto ret = std::string{ dir }; - tr_free(dir); - return ret; + return dir; } #ifdef BUILD_MAC_CLIENT @@ -298,23 +290,20 @@ std::string tr_getWebClientDir([[maybe_unused]] tr_session const* session) auto candidates = std::list{}; /* XDG_DATA_HOME should be the first in the list of candidates */ - char* tmp = tr_env_get_string("XDG_DATA_HOME", nullptr); - if (!tr_str_is_empty(tmp)) + if (auto tmp = tr_env_get_string("XDG_DATA_HOME"sv); !std::empty(tmp)) { - candidates.emplace_back(tmp); + candidates.emplace_back(std::move(tmp)); } else { candidates.emplace_back(fmt::format("{:s}/.local/share"sv, getHomeDir())); } - tr_free(tmp); /* XDG_DATA_DIRS are the backup directories */ { char const* const pkg = PACKAGE_DATA_DIR; - auto* xdg = tr_env_get_string("XDG_DATA_DIRS", ""); + auto const xdg = tr_env_get_string("XDG_DATA_DIRS"sv); auto const buf = fmt::format(FMT_STRING("{:s}:{:s}:/usr/local/share:/usr/share"), pkg, xdg); - tr_free(xdg); auto sv = std::string_view{ buf }; auto token = std::string_view{}; diff --git a/libtransmission/utils.cc b/libtransmission/utils.cc index 13e1de0e1..7f55d8474 100644 --- a/libtransmission/utils.cc +++ b/libtransmission/utils.cc @@ -1269,53 +1269,42 @@ int tr_env_get_int(char const* key, int default_value) return default_value; } -char* tr_env_get_string(char const* key, char const* default_value) +std::string tr_env_get_string(std::string_view key, std::string_view default_value) { - TR_ASSERT(key != nullptr); - #ifdef _WIN32 - wchar_t* wide_key = tr_win32_utf8_to_native(key, -1); - char* value = nullptr; - - if (wide_key != nullptr) + if (auto* const wide_key = tr_win32_utf8_to_native(std::data(key), std::size(key)); wide_key != nullptr) { - DWORD const size = GetEnvironmentVariableW(wide_key, nullptr, 0); - - if (size != 0) + if (auto const size = GetEnvironmentVariableW(wide_key, nullptr, 0); size != 0) { - wchar_t* const wide_value = tr_new(wchar_t, size); + auto wide_val = std::vector{}; + wide_val.resize(size); - if (GetEnvironmentVariableW(wide_key, wide_value, size) == size - 1) + if (GetEnvironmentVariableW(wide_key, std::data(wide_val), std::size(wide_val)) == std::size(wide_val) - 1) { - value = tr_win32_native_to_utf8(wide_value, size); + char* const val = tr_win32_native_to_utf8(std::data(wide_val), std::size(wide_val)); + auto ret = std::string{ val }; + tr_free(val); + tr_free(wide_key); + return ret; } - - tr_free(wide_value); } tr_free(wide_key); } - if (value == nullptr && default_value != nullptr) - { - value = tr_strdup(default_value); - } - - return value; - #else - char const* value = getenv(key); + auto const szkey = tr_strbuf{ key }; - if (value == nullptr) + if (auto const* const value = getenv(szkey); value != nullptr) { - value = default_value; + return value; } - return value != nullptr ? tr_strvDup(value) : nullptr; - #endif + + return std::string{ default_value }; } /*** diff --git a/libtransmission/utils.h b/libtransmission/utils.h index 726297359..6acdbb2a8 100644 --- a/libtransmission/utils.h +++ b/libtransmission/utils.h @@ -437,8 +437,8 @@ bool tr_env_key_exists(char const* key); /** @brief Get environment variable value as int. */ int tr_env_get_int(char const* key, int default_value); -/** @brief Get environment variable value as string (should be freed afterwards). */ -char* tr_env_get_string(char const* key, char const* default_value); +/** @brief Get environment variable value as string. */ +std::string tr_env_get_string(std::string_view key, std::string_view default_value = {}); /*** **** diff --git a/libtransmission/web.cc b/libtransmission/web.cc index 77f7ff319..c9d51a4ba 100644 --- a/libtransmission/web.cc +++ b/libtransmission/web.cc @@ -109,10 +109,9 @@ public: { std::call_once(curl_init_flag, curlInit); - if (auto* bundle = tr_env_get_string("CURL_CA_BUNDLE", nullptr); bundle != nullptr) + if (auto bundle = tr_env_get_string("CURL_CA_BUNDLE"); !std::empty(bundle)) { - curl_ca_bundle = bundle; - tr_free(bundle); + curl_ca_bundle = std::move(bundle); } shareEverything(); diff --git a/tests/libtransmission/subprocess-test-program.cc b/tests/libtransmission/subprocess-test-program.cc index 9a9294fdf..d8abfb9a5 100644 --- a/tests/libtransmission/subprocess-test-program.cc +++ b/tests/libtransmission/subprocess-test-program.cc @@ -43,9 +43,8 @@ int main(int argc, char** argv) { for (int i = 3; i < argc; ++i) { - char* const value = tr_env_get_string(argv[i], ""); + auto const value = tr_env_get_string(argv[i], ""); tr_sys_file_write_line(fd, value); - tr_free(value); } } else if (test_action == "--dump-cwd") diff --git a/tests/libtransmission/utils-test.cc b/tests/libtransmission/utils-test.cc index 3b3cb8126..e972d0342 100644 --- a/tests/libtransmission/utils-test.cc +++ b/tests/libtransmission/utils-test.cc @@ -28,7 +28,6 @@ #include "test-fixtures.h" -using ::libtransmission::test::makeString; using UtilsTest = ::testing::Test; using namespace std::literals; @@ -315,27 +314,22 @@ TEST_F(UtilsTest, env) EXPECT_FALSE(tr_env_key_exists(test_key)); EXPECT_EQ(123, tr_env_get_int(test_key, 123)); - EXPECT_EQ(nullptr, tr_env_get_string(test_key, nullptr)); - auto s = makeString(tr_env_get_string(test_key, "a")); - EXPECT_EQ("a", s); + EXPECT_EQ(""sv, tr_env_get_string(test_key)); + EXPECT_EQ("a"sv, tr_env_get_string(test_key, "a"sv)); setenv(test_key, "", 1); EXPECT_TRUE(tr_env_key_exists(test_key)); EXPECT_EQ(456, tr_env_get_int(test_key, 456)); - s = makeString(tr_env_get_string(test_key, nullptr)); - EXPECT_EQ("", s); - s = makeString(tr_env_get_string(test_key, "b")); - EXPECT_EQ("", s); + EXPECT_EQ("", tr_env_get_string(test_key, "")); + EXPECT_EQ("", tr_env_get_string(test_key, "b")); setenv(test_key, "135", 1); EXPECT_TRUE(tr_env_key_exists(test_key)); EXPECT_EQ(135, tr_env_get_int(test_key, 789)); - s = makeString(tr_env_get_string(test_key, nullptr)); - EXPECT_EQ("135", s); - s = makeString(tr_env_get_string(test_key, "c")); - EXPECT_EQ("135", s); + EXPECT_EQ("135", tr_env_get_string(test_key, "")); + EXPECT_EQ("135", tr_env_get_string(test_key, "c")); } TEST_F(UtilsTest, mimeTypes) diff --git a/utils/remote.cc b/utils/remote.cc index 6fa1c6d11..2216656b8 100644 --- a/utils/remote.cc +++ b/utils/remote.cc @@ -2451,9 +2451,11 @@ static int processArgs(char const* rpcurl, int argc, char const* const* argv) break; case 810: /* authenv */ - auth = tr_env_get_string("TR_AUTH", nullptr); - - if (auth == nullptr) + if (auto const authstr = tr_env_get_string("TR_AUTH"); !std::empty(authstr)) + { + auth = tr_strvDup(authstr); + } + else { fprintf(stderr, "The TR_AUTH environment variable is not set\n"); exit(0);