From 462651bb8e2e05c8f15d1f12e09bf7c44e040701 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 18 Oct 2021 09:48:45 -0500 Subject: [PATCH] refactor: rpcimpl.cc getTorrents() returns a std::vector (#1986) * refactor: rpcimpl.cc getTorrents() returns a std::vector * refactor: tr_torrentQueueMove*() uses std::vector --- libtransmission/rpcimpl.cc | 259 +++++++++++---------------------- libtransmission/torrent.cc | 67 ++++----- libtransmission/transmission.h | 8 +- 3 files changed, 115 insertions(+), 219 deletions(-) diff --git a/libtransmission/rpcimpl.cc b/libtransmission/rpcimpl.cc index 4a3c5b213..0ad699fd3 100644 --- a/libtransmission/rpcimpl.cc +++ b/libtransmission/rpcimpl.cc @@ -11,7 +11,9 @@ #include #include /* strtol */ #include /* strcmp */ +#include #include +#include #ifndef ZLIB_CONST #define ZLIB_CONST @@ -110,24 +112,23 @@ static void tr_idle_function_done(struct tr_rpc_idle_data* data, char const* res **** ***/ -static tr_torrent** getTorrents(tr_session* session, tr_variant* args, int* setmeCount) +static auto getTorrents(tr_session* session, tr_variant* args) { - int torrentCount = 0; - int64_t id; - tr_torrent** torrents = nullptr; - tr_variant* ids; - char const* str; + auto torrents = std::vector{}; + + auto id = int64_t{}; + char const* str = nullptr; + tr_variant* ids = nullptr; if (tr_variantDictFindList(args, TR_KEY_ids, &ids)) { size_t const n = tr_variantListSize(ids); - - torrents = tr_new0(tr_torrent*, n); + torrents.reserve(n); for (size_t i = 0; i < n; ++i) { - tr_torrent* tor; tr_variant const* const node = tr_variantListChild(ids, i); + tr_torrent* tor = nullptr; if (tr_variantGetInt(node, &id)) { @@ -137,73 +138,59 @@ static tr_torrent** getTorrents(tr_session* session, tr_variant* args, int* setm { tor = tr_torrentFindFromHashString(session, str); } - else - { - tor = nullptr; - } if (tor != nullptr) { - torrents[torrentCount++] = tor; + torrents.push_back(tor); } } } else if (tr_variantDictFindInt(args, TR_KEY_ids, &id) || tr_variantDictFindInt(args, TR_KEY_id, &id)) { - tr_torrent* tor; - torrents = tr_new0(tr_torrent*, 1); + tr_torrent* const tor = tr_torrentFindFromId(session, id); - if ((tor = tr_torrentFindFromId(session, id)) != nullptr) + if (tor != nullptr) { - torrents[torrentCount++] = tor; + torrents.push_back(tor); } } else if (tr_variantDictFindStr(args, TR_KEY_ids, &str, nullptr)) { if (strcmp(str, "recently-active") == 0) { - time_t const now = tr_time(); - time_t const window = RECENTLY_ACTIVE_SECONDS; - int const n = tr_sessionCountTorrents(session); - torrents = tr_new0(tr_torrent*, n); + time_t const cutoff = tr_time() - RECENTLY_ACTIVE_SECONDS; - for (auto* tor : session->torrents) - { - if (tor->anyDate >= now - window) - { - torrents[torrentCount++] = tor; - } - } + torrents.reserve(std::size(session->torrents)); + std::copy_if( + std::begin(session->torrents), + std::end(session->torrents), + std::back_inserter(torrents), + [&cutoff](tr_torrent* tor) { return tor->anyDate >= cutoff; }); } else { - tr_torrent* tor; - torrents = tr_new0(tr_torrent*, 1); + tr_torrent* const tor = tr_torrentFindFromHashString(session, str); - if ((tor = tr_torrentFindFromHashString(session, str)) != nullptr) + if (tor != nullptr) { - torrents[torrentCount++] = tor; + torrents.push_back(tor); } } } - else /* all of them */ + else // all of them { - // TODO: getTorrents() should return a std::vector - auto tmp = tr_sessionGetTorrents(session); - torrentCount = std::size(tmp); - torrents = tr_new(tr_torrent*, torrentCount); - std::copy_n(std::begin(tmp), torrentCount, torrents); + torrents.reserve(std::size(session->torrents)); + std::copy(std::begin(session->torrents), std::end(session->torrents), std::back_inserter(torrents)); } - *setmeCount = torrentCount; return torrents; } -static void notifyBatchQueueChange(tr_session* session, tr_torrent** torrents, int n) +static void notifyBatchQueueChange(tr_session* session, std::vector const& torrents) { - for (int i = 0; i < n; ++i) + for (auto* tor : torrents) { - notify(session, TR_RPC_TORRENT_CHANGED, torrents[i]); + notify(session, TR_RPC_TORRENT_CHANGED, tor); } notify(session, TR_RPC_SESSION_QUEUE_POSITIONS_CHANGED, nullptr); @@ -215,11 +202,9 @@ static char const* queueMoveTop( [[maybe_unused]] tr_variant* args_out, [[maybe_unused]] struct tr_rpc_idle_data* idle_data) { - int n; - tr_torrent** torrents = getTorrents(session, args_in, &n); - tr_torrentsQueueMoveTop(torrents, n); - notifyBatchQueueChange(session, torrents, n); - tr_free(torrents); + auto const torrents = getTorrents(session, args_in); + tr_torrentsQueueMoveTop(std::data(torrents), std::size(torrents)); + notifyBatchQueueChange(session, torrents); return nullptr; } @@ -229,11 +214,9 @@ static char const* queueMoveUp( [[maybe_unused]] tr_variant* args_out, [[maybe_unused]] struct tr_rpc_idle_data* idle_data) { - int n; - tr_torrent** torrents = getTorrents(session, args_in, &n); - tr_torrentsQueueMoveUp(torrents, n); - notifyBatchQueueChange(session, torrents, n); - tr_free(torrents); + auto const torrents = getTorrents(session, args_in); + tr_torrentsQueueMoveUp(std::data(torrents), std::size(torrents)); + notifyBatchQueueChange(session, torrents); return nullptr; } @@ -243,11 +226,9 @@ static char const* queueMoveDown( [[maybe_unused]] tr_variant* args_out, [[maybe_unused]] struct tr_rpc_idle_data* idle_data) { - int n; - tr_torrent** torrents = getTorrents(session, args_in, &n); - tr_torrentsQueueMoveDown(torrents, n); - notifyBatchQueueChange(session, torrents, n); - tr_free(torrents); + auto const torrents = getTorrents(session, args_in); + tr_torrentsQueueMoveDown(std::data(torrents), std::size(torrents)); + notifyBatchQueueChange(session, torrents); return nullptr; } @@ -257,21 +238,19 @@ static char const* queueMoveBottom( [[maybe_unused]] tr_variant* args_out, [[maybe_unused]] struct tr_rpc_idle_data* idle_data) { - int n; - tr_torrent** torrents = getTorrents(session, args_in, &n); - tr_torrentsQueueMoveBottom(torrents, n); - notifyBatchQueueChange(session, torrents, n); - tr_free(torrents); + auto const torrents = getTorrents(session, args_in); + tr_torrentsQueueMoveBottom(std::data(torrents), std::size(torrents)); + notifyBatchQueueChange(session, torrents); return nullptr; } -static int compareTorrentByQueuePosition(void const* va, void const* vb) +struct CompareTorrentByQueuePosition { - tr_torrent const* a = *(tr_torrent const* const*)va; - tr_torrent const* b = *(tr_torrent const* const*)vb; - - return a->queuePosition - b->queuePosition; -} + bool operator()(tr_torrent const* a, tr_torrent const* b) const + { + return a->queuePosition < b->queuePosition; + } +}; static char const* torrentStart( tr_session* session, @@ -279,15 +258,10 @@ static char const* torrentStart( [[maybe_unused]] tr_variant* args_out, [[maybe_unused]] struct tr_rpc_idle_data* idle_data) { - int torrentCount; - tr_torrent** torrents = getTorrents(session, args_in, &torrentCount); - - qsort(torrents, torrentCount, sizeof(tr_torrent*), compareTorrentByQueuePosition); - - for (int i = 0; i < torrentCount; ++i) + auto torrents = getTorrents(session, args_in); + std::sort(std::begin(torrents), std::end(torrents), CompareTorrentByQueuePosition{}); + for (auto* tor : torrents) { - tr_torrent* tor = torrents[i]; - if (!tor->isRunning) { tr_torrentStart(tor); @@ -295,7 +269,6 @@ static char const* torrentStart( } } - tr_free(torrents); return nullptr; } @@ -305,15 +278,10 @@ static char const* torrentStartNow( [[maybe_unused]] tr_variant* args_out, [[maybe_unused]] struct tr_rpc_idle_data* idle_data) { - int torrentCount; - tr_torrent** torrents = getTorrents(session, args_in, &torrentCount); - - qsort(torrents, torrentCount, sizeof(tr_torrent*), compareTorrentByQueuePosition); - - for (int i = 0; i < torrentCount; ++i) + auto torrents = getTorrents(session, args_in); + std::sort(std::begin(torrents), std::end(torrents), CompareTorrentByQueuePosition{}); + for (auto* tor : torrents) { - tr_torrent* tor = torrents[i]; - if (!tor->isRunning) { tr_torrentStartNow(tor); @@ -321,7 +289,6 @@ static char const* torrentStartNow( } } - tr_free(torrents); return nullptr; } @@ -331,13 +298,8 @@ static char const* torrentStop( [[maybe_unused]] tr_variant* args_out, [[maybe_unused]] struct tr_rpc_idle_data* idle_data) { - int torrentCount; - tr_torrent** torrents = getTorrents(session, args_in, &torrentCount); - - for (int i = 0; i < torrentCount; ++i) + for (auto* tor : getTorrents(session, args_in)) { - tr_torrent* tor = torrents[i]; - if (tor->isRunning || tr_torrentIsQueued(tor)) { tor->isStopping = true; @@ -345,7 +307,6 @@ static char const* torrentStop( } } - tr_free(torrents); return nullptr; } @@ -364,12 +325,8 @@ static char const* torrentRemove( tr_rpc_callback_type type = deleteFlag ? TR_RPC_TORRENT_TRASHING : TR_RPC_TORRENT_REMOVING; - int torrentCount; - tr_torrent** torrents = getTorrents(session, args_in, &torrentCount); - - for (int i = 0; i < torrentCount; ++i) + for (auto* tor : getTorrents(session, args_in)) { - tr_torrent* tor = torrents[i]; tr_rpc_callback_status const status = notify(session, type, tor); if ((status & TR_RPC_NOREMOVE) == 0) @@ -378,7 +335,6 @@ static char const* torrentRemove( } } - tr_free(torrents); return nullptr; } @@ -388,13 +344,8 @@ static char const* torrentReannounce( [[maybe_unused]] tr_variant* args_out, [[maybe_unused]] struct tr_rpc_idle_data* idle_data) { - int torrentCount; - tr_torrent** torrents = getTorrents(session, args_in, &torrentCount); - - for (int i = 0; i < torrentCount; ++i) + for (auto* tor : getTorrents(session, args_in)) { - tr_torrent* tor = torrents[i]; - if (tr_torrentCanManualUpdate(tor)) { tr_torrentManualUpdate(tor); @@ -402,7 +353,6 @@ static char const* torrentReannounce( } } - tr_free(torrents); return nullptr; } @@ -412,17 +362,12 @@ static char const* torrentVerify( [[maybe_unused]] tr_variant* args_out, [[maybe_unused]] struct tr_rpc_idle_data* idle_data) { - int torrentCount; - tr_torrent** torrents = getTorrents(session, args_in, &torrentCount); - - for (int i = 0; i < torrentCount; ++i) + for (auto* tor : getTorrents(session, args_in)) { - tr_torrent* tor = torrents[i]; tr_torrentVerify(tor, nullptr, nullptr); notify(session, TR_RPC_TORRENT_CHANGED, tor); } - tr_free(torrents); return nullptr; } @@ -942,22 +887,13 @@ static char const* torrentGet( tr_variant* args_out, [[maybe_unused]] struct tr_rpc_idle_data* idle_data) { - int torrentCount; - tr_torrent** torrents = getTorrents(session, args_in, &torrentCount); - tr_variant* list = tr_variantDictAddList(args_out, TR_KEY_torrents, torrentCount + 1); - tr_variant* fields; - char const* strVal; - char const* errmsg = nullptr; - tr_format format; + auto const torrents = getTorrents(session, args_in); + tr_variant* const list = tr_variantDictAddList(args_out, TR_KEY_torrents, std::size(torrents) + 1); - if (tr_variantDictFindStr(args_in, TR_KEY_format, &strVal, nullptr) && strcmp(strVal, "table") == 0) - { - format = TR_FORMAT_TABLE; - } - else /* default value */ - { - format = TR_FORMAT_OBJECT; - } + char const* strVal = nullptr; + tr_format const format = tr_variantDictFindStr(args_in, TR_KEY_format, &strVal, nullptr) && strcmp(strVal, "table") ? + TR_FORMAT_TABLE : + TR_FORMAT_OBJECT; if (tr_variantDictFindStr(args_in, TR_KEY_ids, &strVal, nullptr) && strcmp(strVal, "recently-active") == 0) { @@ -982,6 +918,8 @@ static char const* torrentGet( } } + tr_variant* fields = nullptr; + char const* errmsg = nullptr; if (!tr_variantDictFindList(args_in, TR_KEY_fields, &fields)) { errmsg = "no fields specified"; @@ -1011,15 +949,14 @@ static char const* torrentGet( } } - for (int i = 0; i < torrentCount; ++i) + for (auto* tor : torrents) { - addTorrentInfo(torrents[i], format, tr_variantListAdd(list), keys, keyCount); + addTorrentInfo(tor, format, tr_variantListAdd(list), keys, keyCount); } tr_free(keys); } - tr_free(torrents); return errmsg; } @@ -1359,20 +1296,14 @@ static char const* torrentSet( [[maybe_unused]] tr_variant* args_out, [[maybe_unused]] struct tr_rpc_idle_data* idle_data) { - int torrentCount; - tr_torrent** torrents = getTorrents(session, args_in, &torrentCount); - char const* errmsg = nullptr; - for (int i = 0; i < torrentCount; ++i) + for (auto* tor : getTorrents(session, args_in)) { - int64_t tmp; - double d; - tr_variant* tmp_variant; - bool boolVal; - tr_torrent* tor; - - tor = torrents[i]; + auto tmp = int64_t{}; + auto d = double{}; + auto boolVal = bool{}; + tr_variant* tmp_variant = nullptr; if (tr_variantDictFindInt(args_in, TR_KEY_bandwidthPriority, &tmp)) { @@ -1487,7 +1418,6 @@ static char const* torrentSet( notify(session, TR_RPC_TORRENT_CHANGED, tor); } - tr_free(torrents); return errmsg; } @@ -1509,24 +1439,15 @@ static char const* torrentSetLocation( return "new location path is not absolute"; } - bool move; - int torrentCount; - tr_torrent** torrents = getTorrents(session, args_in, &torrentCount); + auto move = bool{}; + tr_variantDictFindBool(args_in, TR_KEY_move, &move); - if (!tr_variantDictFindBool(args_in, TR_KEY_move, &move)) + for (auto* tor : getTorrents(session, args_in)) { - move = false; - } - - for (int i = 0; i < torrentCount; ++i) - { - tr_torrent* tor = torrents[i]; tr_torrentSetLocation(tor, location, move, nullptr, nullptr); notify(session, TR_RPC_TORRENT_MOVED, tor); } - tr_free(torrents); - return nullptr; } @@ -1536,22 +1457,13 @@ static char const* torrentSetLocation( static void torrentRenamePathDone(tr_torrent* tor, char const* oldpath, char const* newname, int error, void* user_data) { - char const* result; auto* data = static_cast(user_data); tr_variantDictAddInt(data->args_out, TR_KEY_id, tr_torrentId(tor)); tr_variantDictAddStr(data->args_out, TR_KEY_path, oldpath); tr_variantDictAddStr(data->args_out, TR_KEY_name, newname); - if (error == 0) - { - result = nullptr; - } - else - { - result = tr_strerror(error); - } - + char const* const result = error == 0 ? nullptr : tr_strerror(error); tr_idle_function_done(data, result); } @@ -1568,9 +1480,8 @@ static char const* torrentRenamePath( char const* newname = nullptr; (void)tr_variantDictFindStr(args_in, TR_KEY_name, &newname, nullptr); - int torrentCount = 0; - tr_torrent** torrents = getTorrents(session, args_in, &torrentCount); - if (torrentCount == 1) + auto const torrents = getTorrents(session, args_in); + if (std::size(torrents) == 1) { tr_torrentRenamePath(torrents[0], oldpath, newname, torrentRenamePathDone, idle_data); } @@ -1580,7 +1491,6 @@ static char const* torrentRenamePath( } /* cleanup */ - tr_free(torrents); return errmsg; } @@ -2858,16 +2768,13 @@ void tr_rpc_request_exec_uri( tr_rpc_response_func callback, void* callback_user_data) { - char const* pch; - tr_variant top; - tr_variant* args; - char* request = tr_strndup(request_uri, request_uri_len); + char* const request = tr_strndup(request_uri, request_uri_len); + auto top = tr_variant{}; tr_variantInitDict(&top, 3); - args = tr_variantDictAddDict(&top, TR_KEY_arguments, 0); - - pch = strchr(request, '?'); + tr_variant* const args = tr_variantDictAddDict(&top, TR_KEY_arguments, 0); + char const* pch = strchr(request, '?'); if (pch == nullptr) { pch = request; diff --git a/libtransmission/torrent.cc b/libtransmission/torrent.cc index 80eb159ff..176c79957 100644 --- a/libtransmission/torrent.cc +++ b/libtransmission/torrent.cc @@ -19,6 +19,7 @@ #include #include #include +#include #ifndef _WIN32 #include /* wait() */ @@ -3504,14 +3505,6 @@ char* tr_torrentBuildPartial(tr_torrent const* tor, tr_file_index_t fileNum) **** ***/ -static int compareTorrentByQueuePosition(void const* va, void const* vb) -{ - tr_torrent const* a = *(tr_torrent const* const*)va; - tr_torrent const* b = *(tr_torrent const* const*)vb; - - return a->queuePosition - b->queuePosition; -} - #ifdef TR_ENABLE_ASSERTS static bool queueIsSequenced(tr_session* session) @@ -3592,56 +3585,52 @@ void tr_torrentSetQueuePosition(tr_torrent* tor, int pos) TR_ASSERT(queueIsSequenced(tor->session)); } -void tr_torrentsQueueMoveTop(tr_torrent** torrents_in, int n) +struct CompareTorrentByQueuePosition { - auto** torrents = static_cast(tr_memdup(torrents_in, sizeof(tr_torrent*) * n)); - qsort(torrents, n, sizeof(tr_torrent*), compareTorrentByQueuePosition); - - for (int i = n - 1; i >= 0; --i) + bool operator()(tr_torrent const* a, tr_torrent const* b) const { - tr_torrentSetQueuePosition(torrents[i], 0); + return a->queuePosition < b->queuePosition; } +}; - tr_free(torrents); +void tr_torrentsQueueMoveTop(tr_torrent* const* torrents_in, size_t n) +{ + auto torrents = std::vector(torrents_in, torrents_in + n); + std::sort(std::rbegin(torrents), std::rend(torrents), CompareTorrentByQueuePosition{}); + for (auto* tor : torrents) + { + tr_torrentSetQueuePosition(tor, 0); + } } -void tr_torrentsQueueMoveUp(tr_torrent** torrents_in, int n) +void tr_torrentsQueueMoveUp(tr_torrent* const* torrents_in, size_t n) { - auto** torrents = static_cast(tr_memdup(torrents_in, sizeof(tr_torrent*) * n)); - qsort(torrents, n, sizeof(tr_torrent*), compareTorrentByQueuePosition); - - for (int i = 0; i < n; ++i) + auto torrents = std::vector(torrents_in, torrents_in + n); + std::sort(std::begin(torrents), std::end(torrents), CompareTorrentByQueuePosition{}); + for (auto* tor : torrents) { - tr_torrentSetQueuePosition(torrents[i], torrents[i]->queuePosition - 1); + tr_torrentSetQueuePosition(tor, tor->queuePosition - 1); } - - tr_free(torrents); } -void tr_torrentsQueueMoveDown(tr_torrent** torrents_in, int n) +void tr_torrentsQueueMoveDown(tr_torrent* const* torrents_in, size_t n) { - auto** torrents = static_cast(tr_memdup(torrents_in, sizeof(tr_torrent*) * n)); - qsort(torrents, n, sizeof(tr_torrent*), compareTorrentByQueuePosition); - - for (int i = n - 1; i >= 0; --i) + auto torrents = std::vector(torrents_in, torrents_in + n); + std::sort(std::rbegin(torrents), std::rend(torrents), CompareTorrentByQueuePosition{}); + for (auto* tor : torrents) { - tr_torrentSetQueuePosition(torrents[i], torrents[i]->queuePosition + 1); + tr_torrentSetQueuePosition(tor, tor->queuePosition + 1); } - - tr_free(torrents); } -void tr_torrentsQueueMoveBottom(tr_torrent** torrents_in, int n) +void tr_torrentsQueueMoveBottom(tr_torrent* const* torrents_in, size_t n) { - auto** torrents = static_cast(tr_memdup(torrents_in, sizeof(tr_torrent*) * n)); - qsort(torrents, n, sizeof(tr_torrent*), compareTorrentByQueuePosition); - - for (int i = 0; i < n; ++i) + auto torrents = std::vector(torrents_in, torrents_in + n); + std::sort(std::begin(torrents), std::end(torrents), CompareTorrentByQueuePosition{}); + for (auto* tor : torrents) { - tr_torrentSetQueuePosition(torrents[i], INT_MAX); + tr_torrentSetQueuePosition(tor, INT_MAX); } - - tr_free(torrents); } static void torrentSetQueued(tr_torrent* tor, bool queued) diff --git a/libtransmission/transmission.h b/libtransmission/transmission.h index 59abb0c37..5aaf182e5 100644 --- a/libtransmission/transmission.h +++ b/libtransmission/transmission.h @@ -635,16 +635,16 @@ void tr_torrentSetQueuePosition(tr_torrent*, int queuePosition); **/ /** @brief Convenience function for moving a batch of torrents to the front of their queue(s) */ -void tr_torrentsQueueMoveTop(tr_torrent** torrents, int torrentCount); +void tr_torrentsQueueMoveTop(tr_torrent* const* torrents, size_t torrentCount); /** @brief Convenience function for moving a batch of torrents ahead one step in their queue(s) */ -void tr_torrentsQueueMoveUp(tr_torrent** torrents, int torrentCount); +void tr_torrentsQueueMoveUp(tr_torrent* const* torrents, size_t torrentCount); /** @brief Convenience function for moving a batch of torrents back one step in their queue(s) */ -void tr_torrentsQueueMoveDown(tr_torrent** torrents, int torrentCount); +void tr_torrentsQueueMoveDown(tr_torrent* const* torrents, size_t torrentCount); /** @brief Convenience function for moving a batch of torrents to the back of their queue(s) */ -void tr_torrentsQueueMoveBottom(tr_torrent** torrents, int torrentCount); +void tr_torrentsQueueMoveBottom(tr_torrent* const* torrents, size_t torrentCount); /** **/