From fece4137c7fd4522a4998facf7cb68e8ed1d2433 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 9 Dec 2025 14:58:15 -0600 Subject: [PATCH] refactor: add `tr_variant::unmanaged_string(tr_quark)` (#7906) * feat: add tr_variant::unmanaged_string(tr_quark) * refactor: use tr_variant::unmanaged_string(tr_quark) --- libtransmission/variant.h | 5 ++ tests/libtransmission/rpc-test.cc | 32 ++++++------- tests/libtransmission/variant-test.cc | 35 +++++++++++++- utils/remote.cc | 66 ++++++++++----------------- 4 files changed, 77 insertions(+), 61 deletions(-) diff --git a/libtransmission/variant.h b/libtransmission/variant.h index 690d43040..2421646de 100644 --- a/libtransmission/variant.h +++ b/libtransmission/variant.h @@ -272,6 +272,11 @@ public: return ret; } + [[nodiscard]] static tr_variant unmanaged_string(tr_quark const key) + { + return unmanaged_string(tr_quark_get_string_view(key)); + } + template tr_variant& operator=(Val value) { diff --git a/tests/libtransmission/rpc-test.cc b/tests/libtransmission/rpc-test.cc index 3e333a134..1afe7f03a 100644 --- a/tests/libtransmission/rpc-test.cc +++ b/tests/libtransmission/rpc-test.cc @@ -99,7 +99,7 @@ TEST_F(RpcTest, JsonRpcWrongVersion) { auto request_map = tr_variant::Map{ 3U }; request_map.try_emplace(TR_KEY_jsonrpc, "1.0"); - request_map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_session_stats))); + request_map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_session_stats)); request_map.try_emplace(TR_KEY_id, 12345); auto response = tr_variant{}; @@ -141,9 +141,7 @@ TEST_F(RpcTest, idSync) { auto request_map = tr_variant::Map{ 3U }; request_map.try_emplace(TR_KEY_jsonrpc, JsonRpc::Version); - request_map.try_emplace( - TR_KEY_method, - tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_session_stats_kebab))); + request_map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_session_stats_kebab)); request_map[TR_KEY_id].merge(request_id); // copy auto response = tr_variant{}; @@ -189,7 +187,7 @@ TEST_F(RpcTest, idWrongType) { auto request_map = tr_variant::Map{ 3U }; request_map.try_emplace(TR_KEY_jsonrpc, JsonRpc::Version); - request_map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_session_stats))); + request_map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_session_stats)); request_map[TR_KEY_id].merge(request_id); // copy auto response = tr_variant{}; @@ -223,7 +221,7 @@ TEST_F(RpcTest, idWrongType) TEST_F(RpcTest, tagSyncLegacy) { auto request_map = tr_variant::Map{ 2U }; - request_map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_session_stats))); + request_map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_session_stats)); request_map.try_emplace(TR_KEY_tag, 12345); auto response = tr_variant{}; @@ -257,9 +255,7 @@ TEST_F(RpcTest, idAsync) auto request_map = tr_variant::Map{ 3U }; request_map.try_emplace(TR_KEY_jsonrpc, JsonRpc::Version); - request_map.try_emplace( - TR_KEY_method, - tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_torrent_rename_path_kebab))); + request_map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_torrent_rename_path_kebab)); request_map[TR_KEY_id].merge(request_id); // copy auto params_map = tr_variant::Map{ 2U }; @@ -310,7 +306,7 @@ TEST_F(RpcTest, tagAsyncLegacy) EXPECT_NE(nullptr, tor); auto request_map = tr_variant::Map{ 3U }; - request_map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_torrent_rename_path))); + request_map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_torrent_rename_path)); request_map.try_emplace(TR_KEY_tag, 12345); auto arguments_map = tr_variant::Map{ 2U }; @@ -343,7 +339,7 @@ TEST_F(RpcTest, NotificationSync) { auto request_map = tr_variant::Map{ 2U }; request_map.try_emplace(TR_KEY_jsonrpc, JsonRpc::Version); - request_map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_session_stats))); + request_map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_session_stats)); auto response = tr_variant{}; tr_rpc_request_exec( @@ -361,7 +357,7 @@ TEST_F(RpcTest, NotificationAsync) auto request_map = tr_variant::Map{ 2U }; request_map.try_emplace(TR_KEY_jsonrpc, JsonRpc::Version); - request_map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_torrent_rename_path))); + request_map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_torrent_rename_path)); auto params_map = tr_variant::Map{ 2U }; params_map.try_emplace(TR_KEY_path, "files-filled-with-zeroes/512"); @@ -444,18 +440,18 @@ TEST_F(RpcTest, batch) auto request = tr_variant::Map{ 3U }; request.try_emplace(TR_KEY_jsonrpc, JsonRpc::Version); - request.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_session_stats_kebab))); + request.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_session_stats_kebab)); request.try_emplace(TR_KEY_id, 12345); request_vec.emplace_back(std::move(request)); request = tr_variant::Map{ 2U }; request.try_emplace(TR_KEY_jsonrpc, JsonRpc::Version); - request.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_session_set_kebab))); + request.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_session_set_kebab)); request_vec.emplace_back(std::move(request)); request = tr_variant::Map{ 3U }; request.try_emplace(TR_KEY_jsonrpc, JsonRpc::Version); - request.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_session_stats_kebab))); + request.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_session_stats_kebab)); request.try_emplace(TR_KEY_id, "12345"sv); request_vec.emplace_back(std::move(request)); @@ -477,7 +473,7 @@ TEST_F(RpcTest, batch) request_vec.emplace_back(std::move(request)); request = tr_variant::Map{ 2U }; - request.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_session_stats_kebab))); + request.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_session_stats_kebab)); request.try_emplace(TR_KEY_tag, 12345); request_vec.emplace_back(std::move(request)); @@ -596,7 +592,7 @@ TEST_F(RpcTest, sessionGet) auto request_map = tr_variant::Map{ 3U }; request_map.try_emplace(TR_KEY_jsonrpc, JsonRpc::Version); - request_map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_session_get))); + request_map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_session_get)); request_map.try_emplace(TR_KEY_id, 12345); auto response = tr_variant{}; tr_rpc_request_exec( @@ -768,7 +764,7 @@ TEST_F(RpcTest, torrentGet) auto request = tr_variant::Map{ 3U }; request.try_emplace(TR_KEY_jsonrpc, JsonRpc::Version); - request.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_torrent_get))); + request.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_torrent_get)); request.try_emplace(TR_KEY_id, 12345); auto params = tr_variant::Map{ 1U }; diff --git a/tests/libtransmission/variant-test.cc b/tests/libtransmission/variant-test.cc index 28a7c98d9..989ee41ab 100644 --- a/tests/libtransmission/variant-test.cc +++ b/tests/libtransmission/variant-test.cc @@ -24,7 +24,11 @@ using namespace std::literals; -using VariantTest = ::testing::Test; +class VariantTest : public ::testing::Test +{ +protected: + static void expectVariantMatchesQuark(tr_quark key); +}; #ifndef _WIN32 #define STACK_SMASH_DEPTH (1 * 1000 * 1000) @@ -82,6 +86,35 @@ TEST_F(VariantTest, getType) EXPECT_EQ(strkey, *sv); } +// static +void VariantTest::expectVariantMatchesQuark(tr_quark const key) +{ + auto const key_sv = tr_quark_get_string_view(key); + + auto const var = tr_variant::unmanaged_string(key); + auto const var_sv = var.value_if(); + ASSERT_TRUE(var_sv); + + // The strings should not just be equal, + // but should point to literally the same memory + EXPECT_EQ(key_sv, *var_sv); + EXPECT_EQ(std::data(key_sv), std::data(*var_sv)); +} + +TEST_F(VariantTest, unmanagedStringFromPredefinedQuark) +{ + expectVariantMatchesQuark(TR_KEY_name); +} + +TEST_F(VariantTest, unmanagedStringFromNewQuark) +{ + static auto constexpr NewString = std::string_view{ "this-string-is-not-already-interned" }; + ASSERT_FALSE(tr_quark_lookup(NewString)); + + auto const key = tr_quark_new(NewString); + expectVariantMatchesQuark(key); +} + TEST_F(VariantTest, parseInt) { static auto constexpr Benc = "i64e"sv; diff --git a/utils/remote.cc b/utils/remote.cc index 4b76565a1..7800cc3e1 100644 --- a/utils/remote.cc +++ b/utils/remote.cc @@ -2525,7 +2525,7 @@ tr_variant::Map& ensure_sset(tr_variant& sset) { sset = tr_variant::Map{ 3 }; map = sset.get_if(); - map->try_emplace(TR_KEY_method, tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_session_set_kebab))); + map->try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_session_set_kebab)); } auto* args = map->find_if(TR_KEY_arguments); @@ -2543,7 +2543,7 @@ tr_variant::Map& ensure_tset(tr_variant& tset) { tset = tr_variant::Map{ 3 }; map = tset.get_if(); - map->try_emplace(TR_KEY_method, tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_torrent_set_kebab))); + map->try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_torrent_set_kebab)); } auto* args = map->find_if(TR_KEY_arguments); @@ -2561,7 +2561,7 @@ tr_variant::Map& ensure_tadd(tr_variant& tadd) { tadd = tr_variant::Map{ 3 }; map = tadd.get_if(); - map->try_emplace(TR_KEY_method, tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_torrent_add_kebab))); + map->try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_torrent_add_kebab)); map->try_emplace(TR_KEY_tag, TAG_TORRENT_ADD); } @@ -2745,7 +2745,7 @@ int process_args(char const* rpcurl, int argc, char const* const* argv, RemoteCo for (auto const& key : DetailsKeys) { - fields.emplace_back(tr_variant::unmanaged_string(tr_quark_get_string_view(key))); + fields.emplace_back(tr_variant::unmanaged_string(key)); } add_id_arg(args, config, "all"); @@ -2755,7 +2755,7 @@ int process_args(char const* rpcurl, int argc, char const* const* argv, RemoteCo for (auto const& key : DetailsKeys) { - fields.emplace_back(tr_variant::unmanaged_string(tr_quark_get_string_view(key))); + fields.emplace_back(tr_variant::unmanaged_string(key)); } add_id_arg(args, config); @@ -2766,7 +2766,7 @@ int process_args(char const* rpcurl, int argc, char const* const* argv, RemoteCo for (auto const& key : ListKeys) { - fields.emplace_back(tr_variant::unmanaged_string(tr_quark_get_string_view(key))); + fields.emplace_back(tr_variant::unmanaged_string(key)); } add_id_arg(args, config, "all"); @@ -2777,7 +2777,7 @@ int process_args(char const* rpcurl, int argc, char const* const* argv, RemoteCo for (auto const& key : FilesKeys) { - fields.emplace_back(tr_variant::unmanaged_string(tr_quark_get_string_view(key))); + fields.emplace_back(tr_variant::unmanaged_string(key)); } add_id_arg(args, config); @@ -2785,20 +2785,20 @@ int process_args(char const* rpcurl, int argc, char const* const* argv, RemoteCo case 941: map.insert_or_assign(TR_KEY_tag, TAG_PEERS); - fields.emplace_back(tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_peers))); + fields.emplace_back(tr_variant::unmanaged_string(TR_KEY_peers)); add_id_arg(args, config); break; case 942: map.insert_or_assign(TR_KEY_tag, TAG_PIECES); - fields.emplace_back(tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_pieces))); - fields.emplace_back(tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_piece_count_camel))); + fields.emplace_back(tr_variant::unmanaged_string(TR_KEY_pieces)); + fields.emplace_back(tr_variant::unmanaged_string(TR_KEY_piece_count_camel)); add_id_arg(args, config); break; case 943: map.insert_or_assign(TR_KEY_tag, TAG_TRACKERS); - fields.emplace_back(tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_tracker_stats_camel))); + fields.emplace_back(tr_variant::unmanaged_string(TR_KEY_tracker_stats_camel)); add_id_arg(args, config); break; @@ -2808,7 +2808,7 @@ int process_args(char const* rpcurl, int argc, char const* const* argv, RemoteCo } args.insert_or_assign(TR_KEY_fields, std::move(fields)); - fields.emplace_back(tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_torrent_get_kebab))); + fields.emplace_back(tr_variant::unmanaged_string(TR_KEY_torrent_get_kebab)); map.insert_or_assign(TR_KEY_arguments, std::move(args)); auto top = tr_variant{ std::move(map) }; status |= flush(rpcurl, &top, config); @@ -3240,7 +3240,7 @@ int process_args(char const* rpcurl, int argc, char const* const* argv, RemoteCo args.try_emplace(TR_KEY_delete_local_data_kebab, c == 840); add_id_arg(args, config); - map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_torrent_remove_kebab))); + map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_torrent_remove_kebab)); map.try_emplace(TR_KEY_arguments, std::move(args)); auto top = tr_variant{ std::move(map) }; @@ -3264,7 +3264,7 @@ int process_args(char const* rpcurl, int argc, char const* const* argv, RemoteCo auto args = tr_variant::Map{ 1 }; add_id_arg(args, config); auto const key = is_stop ? TR_KEY_torrent_stop_kebab : TR_KEY_torrent_start_kebab; - map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(tr_quark_get_string_view(key))); + map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(key)); map.try_emplace(TR_KEY_arguments, std::move(args)); auto top = tr_variant{ std::move(map) }; @@ -3302,7 +3302,7 @@ int process_args(char const* rpcurl, int argc, char const* const* argv, RemoteCo auto args = tr_variant::Map{ 1 }; add_id_arg(args, config); auto const key = Method(c); - map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(tr_quark_get_string_view(key))); + map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(key)); map.try_emplace(TR_KEY_arguments, std::move(args)); auto top = tr_variant{ std::move(map) }; status |= flush(rpcurl, &top, config); @@ -3314,9 +3314,7 @@ int process_args(char const* rpcurl, int argc, char const* const* argv, RemoteCo case 920: /* session-info */ { auto map = tr_variant::Map{ 2 }; - map.try_emplace( - TR_KEY_method, - tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_session_get_kebab))); + map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_session_get_kebab)); map.try_emplace(TR_KEY_tag, TAG_SESSION); auto top = tr_variant{ std::move(map) }; @@ -3334,9 +3332,7 @@ int process_args(char const* rpcurl, int argc, char const* const* argv, RemoteCo case 850: { auto map = tr_variant::Map{ 1 }; - map.try_emplace( - TR_KEY_method, - tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_session_close_kebab))); + map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_session_close_kebab)); auto top = tr_variant{ std::move(map) }; status |= flush(rpcurl, &top, config); } @@ -3345,9 +3341,7 @@ int process_args(char const* rpcurl, int argc, char const* const* argv, RemoteCo case 963: { auto map = tr_variant::Map{ 1 }; - map.try_emplace( - TR_KEY_method, - tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_blocklist_update_kebab))); + map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_blocklist_update_kebab)); auto top = tr_variant{ std::move(map) }; status |= flush(rpcurl, &top, config); } @@ -3356,9 +3350,7 @@ int process_args(char const* rpcurl, int argc, char const* const* argv, RemoteCo case 921: { auto map = tr_variant::Map{ 2 }; - map.try_emplace( - TR_KEY_method, - tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_session_stats_kebab))); + map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_session_stats_kebab)); map.try_emplace(TR_KEY_tag, TAG_STATS); auto top = tr_variant{ std::move(map) }; status |= flush(rpcurl, &top, config); @@ -3368,9 +3360,7 @@ int process_args(char const* rpcurl, int argc, char const* const* argv, RemoteCo case 962: { auto map = tr_variant::Map{ 2 }; - map.try_emplace( - TR_KEY_method, - tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_port_test_kebab))); + map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_port_test_kebab)); map.try_emplace(TR_KEY_tag, TAG_PORTTEST); auto top = tr_variant{ std::move(map) }; status |= flush(rpcurl, &top, config); @@ -3384,9 +3374,7 @@ int process_args(char const* rpcurl, int argc, char const* const* argv, RemoteCo args.try_emplace(TR_KEY_location, optarg_sv); args.try_emplace(TR_KEY_move, true); add_id_arg(args, config); - map.try_emplace( - TR_KEY_method, - tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_torrent_set_location_kebab))); + map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_torrent_set_location_kebab)); map.try_emplace(TR_KEY_arguments, std::move(args)); auto top = tr_variant{ std::move(map) }; status |= flush(rpcurl, &top, config); @@ -3413,9 +3401,7 @@ int process_args(char const* rpcurl, int argc, char const* const* argv, RemoteCo args.try_emplace(TR_KEY_location, optarg_sv); args.try_emplace(TR_KEY_move, false); add_id_arg(args, config); - map.try_emplace( - TR_KEY_method, - tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_torrent_set_location_kebab))); + map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_torrent_set_location_kebab)); map.try_emplace(TR_KEY_arguments, std::move(args)); auto top = tr_variant{ std::move(map) }; status |= flush(rpcurl, &top, config); @@ -3429,9 +3415,7 @@ int process_args(char const* rpcurl, int argc, char const* const* argv, RemoteCo args.try_emplace(TR_KEY_path, rename_from); args.try_emplace(TR_KEY_name, optarg_sv); add_id_arg(args, config); - map.try_emplace( - TR_KEY_method, - tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_torrent_rename_path_kebab))); + map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_torrent_rename_path_kebab)); map.try_emplace(TR_KEY_arguments, std::move(args)); auto top = tr_variant{ std::move(map) }; status |= flush(rpcurl, &top, config); @@ -3446,9 +3430,7 @@ int process_args(char const* rpcurl, int argc, char const* const* argv, RemoteCo case 732: { auto map = tr_variant::Map{ 2 }; - map.try_emplace( - TR_KEY_method, - tr_variant::unmanaged_string(tr_quark_get_string_view(TR_KEY_group_get_kebab))); + map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(TR_KEY_group_get_kebab)); map.try_emplace(TR_KEY_tag, TAG_GROUPS); auto top = tr_variant{ std::move(map) }; status |= flush(rpcurl, &top, config);