From 7bdb6f777ba079f2324a96ba8a19ad53ba9cff21 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 9 Feb 2022 12:02:59 -0600 Subject: [PATCH] refactor: remove callback from tr_torrentVerify() public API (#2592) --- cli/cli.cc | 2 +- gtk/OptionsDialog.cc | 2 +- libtransmission/rpcimpl.cc | 2 +- libtransmission/torrent.cc | 142 +++++++++++--------------- libtransmission/transmission.h | 17 +-- libtransmission/verify.cc | 2 +- libtransmission/verify.h | 5 + macosx/Torrent.mm | 2 +- tests/libtransmission/test-fixtures.h | 16 +-- 9 files changed, 71 insertions(+), 119 deletions(-) diff --git a/cli/cli.cc b/cli/cli.cc index 73b2940a1..0643966e3 100644 --- a/cli/cli.cc +++ b/cli/cli.cc @@ -320,7 +320,7 @@ int tr_main(int argc, char* argv[]) if (verify) { verify = false; - tr_torrentVerify(tor, nullptr, nullptr); + tr_torrentVerify(tor); } for (;;) diff --git a/gtk/OptionsDialog.cc b/gtk/OptionsDialog.cc index 24a408b96..dcb708ff6 100644 --- a/gtk/OptionsDialog.cc +++ b/gtk/OptionsDialog.cc @@ -166,7 +166,7 @@ void OptionsDialog::Impl::updateTorrent() tr_torrentSetDownloadDir(tor_, downloadDir_.c_str()); file_list_->set_sensitive(tr_torrentHasMetadata(tor_)); file_list_->set_torrent(tr_torrentId(tor_)); - tr_torrentVerify(tor_, nullptr, nullptr); + tr_torrentVerify(tor_); } } diff --git a/libtransmission/rpcimpl.cc b/libtransmission/rpcimpl.cc index 43ef8b488..3293abef2 100644 --- a/libtransmission/rpcimpl.cc +++ b/libtransmission/rpcimpl.cc @@ -354,7 +354,7 @@ static char const* torrentVerify( { for (auto* tor : getTorrents(session, args_in)) { - tr_torrentVerify(tor, nullptr, nullptr); + tr_torrentVerify(tor); notify(session, TR_RPC_TORRENT_CHANGED, tor); } diff --git a/libtransmission/torrent.cc b/libtransmission/torrent.cc index bf3493a53..da600bae2 100644 --- a/libtransmission/torrent.cc +++ b/libtransmission/torrent.cc @@ -750,7 +750,7 @@ static void torrentInit(tr_torrent* tor, tr_ctor const* ctor) else { tor->startAfterVerify = doStart; - tr_torrentVerify(tor, nullptr, nullptr); + tr_torrentVerify(tor); } } else if (doStart) @@ -1369,99 +1369,70 @@ void tr_torrentStartNow(tr_torrent* tor) } } -struct verify_data +static void onVerifyDoneThreadFunc(void* vtor) { - bool aborted; - tr_torrent* tor; - tr_verify_done_func callback_func; - void* callback_data; -}; - -static void onVerifyDoneThreadFunc(void* vdata) -{ - auto* data = static_cast(vdata); - - if (auto* const tor = data->tor; !tor->isDeleting) - { - if (!data->aborted) - { - tor->recheckCompleteness(); - } - - if (data->callback_func != nullptr) - { - (*data->callback_func)(tor, data->aborted, data->callback_data); - } - - if (!data->aborted && tor->startAfterVerify) - { - tor->startAfterVerify = false; - torrentStart(tor, false); - } - } - - tr_free(data); -} - -static void onVerifyDone(tr_torrent* tor, bool aborted, void* vdata) -{ - auto* data = static_cast(vdata); - - TR_ASSERT(data->tor == tor); + auto* const tor = static_cast(vtor); + TR_ASSERT(tr_amInEventThread(tor->session)); if (tor->isDeleting) { - tr_free(data); return; } - data->aborted = aborted; - tr_runInEventThread(tor->session, onVerifyDoneThreadFunc, data); + tor->recheckCompleteness(); + + if (tor->startAfterVerify) + { + tor->startAfterVerify = false; + torrentStart(tor, false); + } } -static void verifyTorrent(void* vdata) +static void onVerifyDone(tr_torrent* tor, bool aborted, void* /*unused*/) { - auto* data = static_cast(vdata); - tr_torrent* tor = data->tor; + if (aborted || tor->isDeleting) + { + return; + } + + tr_runInEventThread(tor->session, onVerifyDoneThreadFunc, tor); +} + +static void verifyTorrent(void* vtor) +{ + auto* tor = static_cast(vtor); + TR_ASSERT(tr_amInEventThread(tor->session)); auto const lock = tor->unique_lock(); if (tor->isDeleting) { - tr_free(data); + return; + } + + /* if the torrent's already being verified, stop it */ + tr_verifyRemove(tor); + + bool const startAfter = (tor->isRunning || tor->startAfterVerify) && !tor->isStopping; + + if (tor->isRunning) + { + tr_torrentStop(tor); + } + + if (setLocalErrorIfFilesDisappeared(tor)) + { + tor->startAfterVerify = false; } else { - /* if the torrent's already being verified, stop it */ - tr_verifyRemove(tor); - - bool const startAfter = (tor->isRunning || tor->startAfterVerify) && !tor->isStopping; - - if (tor->isRunning) - { - tr_torrentStop(tor); - } - tor->startAfterVerify = startAfter; - - if (setLocalErrorIfFilesDisappeared(tor)) - { - tor->startAfterVerify = false; - } - else - { - tr_verifyAdd(tor, onVerifyDone, data); - } + tr_verifyAdd(tor, onVerifyDone, nullptr); } } -void tr_torrentVerify(tr_torrent* tor, tr_verify_done_func callback_func, void* callback_data) +void tr_torrentVerify(tr_torrent* tor) { - auto* const data = tr_new(struct verify_data, 1); - data->tor = tor; - data->aborted = false; - data->callback_func = callback_func; - data->callback_data = callback_data; - tr_runInEventThread(tor->session, verifyTorrent, data); + tr_runInEventThread(tor->session, verifyTorrent, tor); } void tr_torrentSave(tr_torrent* tor) @@ -1479,6 +1450,7 @@ static void stopTorrent(void* vtor) { auto* tor = static_cast(vtor); TR_ASSERT(tr_isTorrent(tor)); + TR_ASSERT(tr_amInEventThread(tor->session)); auto const lock = tor->unique_lock(); tr_logAddTorInfo(tor, "%s", "Pausing"); @@ -1502,7 +1474,7 @@ static void stopTorrent(void* vtor) tor->magnetVerify = false; tr_logAddTorInfo(tor, "%s", "Magnet Verify"); refreshCurrentDir(tor); - tr_torrentVerify(tor, nullptr, nullptr); + tr_torrentVerify(tor); callScriptIfEnabled(tor, TR_SCRIPT_ON_TORRENT_ADDED); } @@ -1510,25 +1482,25 @@ static void stopTorrent(void* vtor) void tr_torrentStop(tr_torrent* tor) { - TR_ASSERT(tr_isTorrent(tor)); - - if (tr_isTorrent(tor)) + if (!tr_isTorrent(tor)) { - auto const lock = tor->unique_lock(); - - tor->isRunning = false; - tor->isStopping = false; - tor->prefetchMagnetMetadata = false; - tor->setDirty(); - tr_runInEventThread(tor->session, stopTorrent, tor); + return; } + + auto const lock = tor->unique_lock(); + + tor->isRunning = false; + tor->isStopping = false; + tor->prefetchMagnetMetadata = false; + tor->setDirty(); + tr_runInEventThread(tor->session, stopTorrent, tor); } static void closeTorrent(void* vtor) { - auto* tor = static_cast(vtor); - + auto* const tor = static_cast(vtor); TR_ASSERT(tr_isTorrent(tor)); + TR_ASSERT(tr_amInEventThread(tor->session)); tor->session->removed_torrents.emplace_back(tor->uniqueId, tr_time()); diff --git a/libtransmission/transmission.h b/libtransmission/transmission.h index 9c79239be..3b599e7b0 100644 --- a/libtransmission/transmission.h +++ b/libtransmission/transmission.h @@ -1482,25 +1482,10 @@ void tr_torrentAvailability(tr_torrent const* torrent, int8_t* tab, int size); void tr_torrentAmountFinished(tr_torrent const* torrent, float* tab, int size); -/** - * Callback function invoked when a torrent finishes being verified. - * - * @param torrent the torrent that was verified - * @param aborted true if the verify ended prematurely for some reason, - * such as tr_torrentStop() or tr_torrentSetLocation() - * being called during verification. - * @param user_data the user-defined pointer from tr_torrentVerify() - */ -using tr_verify_done_func = void (*)(tr_torrent* torrent, bool aborted, void* user_data); - /** * Queue a torrent for verification. - * - * If callback_func is non-nullptr, it will be called from the libtransmission - * thread after the torrent's completness state is updated after the - * file verification pass. */ -void tr_torrentVerify(tr_torrent* torrent, tr_verify_done_func callback_func_or_nullptr, void* callback_data_or_nullptr); +void tr_torrentVerify(tr_torrent* torrent); bool tr_torrentHasMetadata(tr_torrent const* tor); diff --git a/libtransmission/verify.cc b/libtransmission/verify.cc index 9064e64d1..27eeffb76 100644 --- a/libtransmission/verify.cc +++ b/libtransmission/verify.cc @@ -59,7 +59,7 @@ static bool verifyTorrent(tr_torrent* tor, bool const* stopFlag) /* if we're starting a new file... */ if (filePos == 0 && fd == TR_BAD_SYS_FILE && fileIndex != prevFileIndex) { - char* filename = tr_torrentFindFile(tor, fileIndex); + char* const filename = tr_torrentFindFile(tor, fileIndex); fd = filename == nullptr ? TR_BAD_SYS_FILE : tr_sys_file_open(filename, TR_SYS_FILE_READ | TR_SYS_FILE_SEQUENTIAL, 0, nullptr); tr_free(filename); diff --git a/libtransmission/verify.h b/libtransmission/verify.h index e24cfac1e..8a261a464 100644 --- a/libtransmission/verify.h +++ b/libtransmission/verify.h @@ -9,11 +9,16 @@ #error only libtransmission should #include this header. #endif +struct tr_session; +struct tr_torrent; + /** * @addtogroup file_io File IO * @{ */ +using tr_verify_done_func = void (*)(tr_torrent*, bool aborted, void* user_data); + void tr_verifyAdd(tr_torrent* tor, tr_verify_done_func callback_func, void* callback_user_data); void tr_verifyRemove(tr_torrent* tor); diff --git a/macosx/Torrent.mm b/macosx/Torrent.mm index 9ab64671f..42d5c4681 100644 --- a/macosx/Torrent.mm +++ b/macosx/Torrent.mm @@ -415,7 +415,7 @@ bool trashDataFile(char const* filename, tr_error** error) - (void)resetCache { - tr_torrentVerify(fHandle, NULL, NULL); + tr_torrentVerify(fHandle); [self update]; } diff --git a/tests/libtransmission/test-fixtures.h b/tests/libtransmission/test-fixtures.h index 56964ad7a..5ceb090e9 100644 --- a/tests/libtransmission/test-fixtures.h +++ b/tests/libtransmission/test-fixtures.h @@ -435,19 +435,9 @@ protected: { EXPECT_NE(nullptr, tor->session); EXPECT_FALSE(tr_amInEventThread(tor->session)); - - auto constexpr onVerifyDone = [](tr_torrent*, bool, void* done) noexcept - { - *static_cast(done) = true; - }; - - bool done = false; - tr_torrentVerify(tor, onVerifyDone, &done); - auto test = [&done]() - { - return done; - }; - EXPECT_TRUE(waitFor(test, 2000)); + tr_torrentVerify(tor); + tr_wait_msec(100); + EXPECT_TRUE(waitFor([tor]() { return tor->verifyState == TR_VERIFY_NONE; }, 4000)); } tr_session* session_ = nullptr;