From 5594b163585763b896e625dd13b16471a738b21e Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles@charleskerr.com>
Date: Mon, 1 Nov 2021 18:29:13 -0500
Subject: [PATCH] refactor: tr_quark_lookup() takes a std::string_view (#2081)

* refactor: tr_quark_lookup takes a std::string_view
---
 libtransmission/quark.cc            | 27 +++++++++++----------------
 libtransmission/quark.h             |  3 ++-
 libtransmission/rpcimpl.cc          | 13 +++++--------
 tests/libtransmission/quark-test.cc |  7 +++----
 4 files changed, 21 insertions(+), 29 deletions(-)

diff --git a/libtransmission/quark.cc b/libtransmission/quark.cc
index 8f50f2d47..dde428dfd 100644
--- a/libtransmission/quark.cc
+++ b/libtransmission/quark.cc
@@ -431,24 +431,20 @@ size_t constexpr quarks_are_sorted = ( //
     })();
 
 static_assert(quarks_are_sorted, "Predefined quarks must be sorted by their string value");
+static_assert(std::size(my_static) == TR_N_KEYS);
 
 auto& my_runtime{ *new std::vector<std::string_view>{} };
 
 } // namespace
 
-bool tr_quark_lookup(void const* str, size_t len, tr_quark* setme)
+std::optional<tr_quark> tr_quark_lookup(std::string_view key)
 {
-    auto constexpr n_static = std::size(my_static);
-    static_assert(n_static == TR_N_KEYS);
-
-    /* is it in our static array? */
-    auto const key = std::string_view{ static_cast<char const*>(str), len };
+    // is it in our static array?
     auto constexpr sbegin = std::begin(my_static), send = std::end(my_static);
     auto const sit = std::lower_bound(sbegin, send, key);
     if (sit != send && *sit == key)
     {
-        *setme = std::distance(sbegin, sit);
-        return true;
+        return std::distance(sbegin, sit);
     }
 
     /* was it added during runtime? */
@@ -456,23 +452,22 @@ bool tr_quark_lookup(void const* str, size_t len, tr_quark* setme)
     auto const rit = std::find(rbegin, rend, key);
     if (rit != rend)
     {
-        *setme = TR_N_KEYS + std::distance(rbegin, rit);
-        return true;
+        return TR_N_KEYS + std::distance(rbegin, rit);
     }
 
-    return false;
+    return {};
 }
 
 tr_quark tr_quark_new(std::string_view str)
 {
-    tr_quark ret = TR_KEY_NONE;
-
-    if (!tr_quark_lookup(std::data(str), std::size(str), &ret))
+    auto const prior = tr_quark_lookup(str);
+    if (prior)
     {
-        ret = TR_N_KEYS + std::size(my_runtime);
-        my_runtime.emplace_back(tr_strndup(std::data(str), std::size(str)), std::size(str));
+        return *prior;
     }
 
+    auto const ret = TR_N_KEYS + std::size(my_runtime);
+    my_runtime.emplace_back(tr_strndup(std::data(str), std::size(str)), std::size(str));
     return ret;
 }
 
diff --git a/libtransmission/quark.h b/libtransmission/quark.h
index 53a1fcd24..cc8f0b314 100644
--- a/libtransmission/quark.h
+++ b/libtransmission/quark.h
@@ -9,6 +9,7 @@
 #pragma once
 
 #include <cstddef> // size_t
+#include <optional>
 #include <string_view>
 
 #include "tr-macros.h"
@@ -425,7 +426,7 @@ enum
  *
  * @return true if the specified string exists as a quark
  */
-bool tr_quark_lookup(void const* str, size_t len, tr_quark* setme);
+std::optional<tr_quark> tr_quark_lookup(std::string_view key);
 
 /**
  * Get the string that corresponds to the specified quark
diff --git a/libtransmission/rpcimpl.cc b/libtransmission/rpcimpl.cc
index 17cd00672..5a3eef665 100644
--- a/libtransmission/rpcimpl.cc
+++ b/libtransmission/rpcimpl.cc
@@ -2443,20 +2443,17 @@ static char const* sessionGet(tr_session* s, tr_variant* args_in, tr_variant* ar
 
         for (size_t i = 0; i < field_count; ++i)
         {
-            char const* field_name = nullptr;
-            auto field_name_len = size_t{};
-            if (!tr_variantGetStr(tr_variantListChild(fields, i), &field_name, &field_name_len))
+            auto field_name = std::string_view{};
+            if (tr_variantGetStrView(tr_variantListChild(fields, i), &field_name))
             {
                 continue;
             }
 
-            auto field_id = tr_quark{};
-            if (!tr_quark_lookup(field_name, field_name_len, &field_id))
+            auto const field_id = tr_quark_lookup(field_name);
+            if (field_id)
             {
-                continue;
+                addSessionField(s, args_out, *field_id);
             }
-
-            addSessionField(s, args_out, field_id);
         }
     }
     else
diff --git a/tests/libtransmission/quark-test.cc b/tests/libtransmission/quark-test.cc
index 55a2d19f6..6c0bb97ff 100644
--- a/tests/libtransmission/quark-test.cc
+++ b/tests/libtransmission/quark-test.cc
@@ -33,10 +33,9 @@ TEST_F(QuarkTest, allPredefinedKeysCanBeLookedUp)
     for (int i = 0; i < TR_N_KEYS; i++)
     {
         auto const str = quarkGetString(i);
-
-        tr_quark q;
-        EXPECT_TRUE(tr_quark_lookup(str.data(), str.size(), &q));
-        EXPECT_EQ(i, q);
+        auto const q = tr_quark_lookup(str);
+        EXPECT_TRUE(q);
+        EXPECT_EQ(i, *q);
     }
 }