refactor: tr_env_get_string() now returns a std::string (#3527)

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 };
+        return dir;
-        tr_free(dir);
-        return ret;
     }
 
 #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 };
+        return dir;
-        tr_free(dir);
-        return ret;
     }
 
     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 };
+        return dir;
-        tr_free(dir);
-        return ret;
     }
 
-    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 };
+        return dir;
-        tr_free(dir);
-        return ret;
     }
 
 #ifdef BUILD_MAC_CLIENT
@@ -298,23 +290,20 @@ std::string tr_getWebClientDir([[maybe_unused]] tr_session const* session)
     auto candidates = std::list<std::string>{};
 
     /* XDG_DATA_HOME should be the first in the list of candidates */
-    char* tmp = tr_env_get_string("XDG_DATA_HOME", nullptr);
+    if (auto tmp = tr_env_get_string("XDG_DATA_HOME"sv); !std::empty(tmp))
-    if (!tr_str_is_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{};

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);
+    if (auto* const wide_key = tr_win32_utf8_to_native(std::data(key), std::size(key)); wide_key != nullptr)
-    char* value = nullptr;
-
-    if (wide_key != nullptr)
     {
-        DWORD const size = GetEnvironmentVariableW(wide_key, nullptr, 0);
+        if (auto const size = GetEnvironmentVariableW(wide_key, nullptr, 0); size != 0)
-
-        if (size != 0)
         {
-            wchar_t* const wide_value = tr_new(wchar_t, size);
+            auto wide_val = std::vector<wchar_t>{};
+            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<char, 256>{ 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 };
 }
 
 /***

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). */
+/** @brief Get environment variable value as string. */
-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 = {});
 
 /***
 ****

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;
+            curl_ca_bundle = std::move(bundle);
-            tr_free(bundle);
         }
 
         shareEverything();

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], "<null>");
+            auto const value = tr_env_get_string(argv[i], "<null>");
             tr_sys_file_write_line(fd, value);
-            tr_free(value);
         }
     }
     else if (test_action == "--dump-cwd")

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));
+    EXPECT_EQ(""sv, tr_env_get_string(test_key));
-    auto s = makeString(tr_env_get_string(test_key, "a"));
+    EXPECT_EQ("a"sv, tr_env_get_string(test_key, "a"sv));
-    EXPECT_EQ("a", s);
 
     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("", tr_env_get_string(test_key, ""));
-    EXPECT_EQ("", s);
+    EXPECT_EQ("", tr_env_get_string(test_key, "b"));
-    s = makeString(tr_env_get_string(test_key, "b"));
-    EXPECT_EQ("", s);
 
     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", tr_env_get_string(test_key, ""));
-    EXPECT_EQ("135", s);
+    EXPECT_EQ("135", tr_env_get_string(test_key, "c"));
-    s = makeString(tr_env_get_string(test_key, "c"));
-    EXPECT_EQ("135", s);
 }
 
 TEST_F(UtilsTest, mimeTypes)

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 (auto const authstr = tr_env_get_string("TR_AUTH"); !std::empty(authstr))
-
+                {
-                if (auth == nullptr)
+                    auth = tr_strvDup(authstr);
+                }
+                else
                 {
                     fprintf(stderr, "The TR_AUTH environment variable is not set\n");
                     exit(0);