From 577ae383e2a97f447a997d1692999a2943c1ae0a Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Fri, 23 Jan 2026 09:58:10 -0600 Subject: [PATCH] refactor: tr_torrentSetLocation() takes a std::function callback --- gtk/FileList.cc | 21 +------ libtransmission/rpcimpl.cc | 42 ++++++-------- libtransmission/torrent.cc | 24 +++----- libtransmission/torrent.h | 9 +-- libtransmission/transmission.h | 6 +- macosx/FileRenameSheetController.mm | 12 +++- macosx/Torrent.h | 6 +- macosx/Torrent.mm | 83 ++++++++++++++++++---------- tests/libtransmission/rename-test.cc | 18 ++---- 9 files changed, 103 insertions(+), 118 deletions(-) diff --git a/gtk/FileList.cc b/gtk/FileList.cc index 03144c0d9..b24beb618 100644 --- a/gtk/FileList.cc +++ b/gtk/FileList.cc @@ -812,13 +812,6 @@ bool FileList::Impl::onViewButtonPressed(guint button, TrGdkModifierType state, return handled; } -struct rename_data -{ - Glib::ustring newname; - Glib::ustring path_string; - gpointer impl = nullptr; -}; - void FileList::Impl::on_rename_done(Glib::ustring const& path_string, Glib::ustring const& newname, int error) { rename_done_tags_.push( @@ -901,22 +894,12 @@ void FileList::Impl::cell_edited_callback(Glib::ustring const& path_string, Glib oldpath.insert(0, 1, G_DIR_SEPARATOR); } - /* do the renaming */ - auto rename_data = std::make_unique(); - rename_data->newname = newname; - rename_data->impl = this; - rename_data->path_string = path_string; + // do the renaming tr_torrentRenamePath( tor, oldpath.raw(), newname.raw(), - static_cast( - [](tr_torrent* /*tor*/, char const* /*oldpath*/, char const* /*newname*/, int error, gpointer data) - { - auto const data_grave = std::unique_ptr(static_cast(data)); - static_cast(data_grave->impl)->on_rename_done(data_grave->path_string, data_grave->newname, error); - }), - rename_data.release()); + [this, newname, path_string](int const error) { on_rename_done(path_string, newname, error); }); } FileList::FileList( diff --git a/libtransmission/rpcimpl.cc b/libtransmission/rpcimpl.cc index 6974e6796..a1cdd5edb 100644 --- a/libtransmission/rpcimpl.cc +++ b/libtransmission/rpcimpl.cc @@ -1431,26 +1431,6 @@ namespace make_torrent_field_helpers // --- -void torrentRenamePathDone(tr_torrent* tor, char const* oldpath, char const* newname, int error, void* user_data) -{ - using namespace JsonRpc; - - auto* const data = static_cast(user_data); - - data->args_out.try_emplace(TR_KEY_id, tor->id()); - data->args_out.try_emplace(TR_KEY_path, oldpath); - data->args_out.try_emplace(TR_KEY_name, newname); - - if (error == 0) - { - tr_rpc_idle_done(data, Error::SUCCESS, {}); - } - else - { - tr_rpc_idle_done(data, Error::SYSTEM_ERROR, tr_strerror(error)); - } -} - void torrentRenamePath(tr_session* session, tr_variant::Map const& args_in, struct tr_rpc_idle_data* idle_data) { using namespace JsonRpc; @@ -1462,14 +1442,28 @@ void torrentRenamePath(tr_session* session, tr_variant::Map const& args_in, stru return; } + auto* const tor = torrents[0]; + idle_data->args_out.try_emplace(TR_KEY_id, tor->id()); + auto const oldpath = args_in.value_if(TR_KEY_path).value_or(""sv); auto const newname = args_in.value_if(TR_KEY_name).value_or(""sv); - torrents[0]->rename_path( + idle_data->args_out.try_emplace(TR_KEY_path, oldpath); + idle_data->args_out.try_emplace(TR_KEY_name, newname); + + tor->rename_path( oldpath, newname, - [](tr_torrent* tor, char const* old_path, char const* new_name, int error, void* user_data) - { torrentRenamePathDone(tor, old_path, new_name, error, user_data); }, - idle_data); + [idle_data](int const error) + { + if (error == 0) + { + tr_rpc_idle_done(idle_data, JsonRpc::Error::SUCCESS, {}); + } + else + { + tr_rpc_idle_done(idle_data, JsonRpc::Error::SYSTEM_ERROR, tr_strerror(error)); + } + }); } // --- diff --git a/libtransmission/torrent.cc b/libtransmission/torrent.cc index ff29d223c..b42499d00 100644 --- a/libtransmission/torrent.cc +++ b/libtransmission/torrent.cc @@ -2471,8 +2471,7 @@ void renameTorrentFileString(tr_torrent* tor, std::string_view oldpath, std::str void tr_torrent::rename_path_in_session_thread( std::string_view const oldpath, std::string_view const newname, - tr_torrent_rename_done_func const& callback, - void* const callback_user_data) + std::function const& on_rename_done) { using namespace rename_helpers; @@ -2511,33 +2510,26 @@ void tr_torrent::rename_path_in_session_thread( mark_changed(); - if (callback != nullptr) + if (on_rename_done) { - auto const szold = tr_pathbuf{ oldpath }; - auto const sznew = tr_pathbuf{ newname }; - callback(this, szold.c_str(), sznew.c_str(), error, callback_user_data); + on_rename_done(error); } } -void tr_torrent::rename_path( - std::string_view oldpath, - std::string_view newname, - tr_torrent_rename_done_func&& callback, - void* callback_user_data) +void tr_torrent::rename_path(std::string_view oldpath, std::string_view newname, std::function on_rename_done) { this->session->run_in_session_thread( - [this, oldpath = std::string(oldpath), newname = std::string(newname), cb = std::move(callback), callback_user_data]() - { rename_path_in_session_thread(oldpath, newname, std::move(cb), callback_user_data); }); + [this, oldpath = std::string(oldpath), newname = std::string(newname), cb = std::move(on_rename_done)]() + { rename_path_in_session_thread(oldpath, newname, std::move(cb)); }); } void tr_torrentRenamePath( tr_torrent* tor, std::string_view const oldpath, std::string_view const newname, - tr_torrent_rename_done_func callback, - void* callback_user_data) + std::function on_rename_done) { - tor->rename_path(oldpath, newname, std::move(callback), callback_user_data); + tor->rename_path(oldpath, newname, std::move(on_rename_done)); } // --- diff --git a/libtransmission/torrent.h b/libtransmission/torrent.h index fa4b2e5a8..e7b58ffbc 100644 --- a/libtransmission/torrent.h +++ b/libtransmission/torrent.h @@ -180,11 +180,7 @@ struct tr_torrent void set_location(std::string_view location, bool move_from_old_path, int volatile* setme_state); - void rename_path( - std::string_view oldpath, - std::string_view newname, - tr_torrent_rename_done_func&& callback, - void* callback_user_data); + void rename_path(std::string_view oldpath, std::string_view newname, std::function on_rename_done); // these functions should become private when possible, // but more refactoring is needed before that can happen @@ -1332,8 +1328,7 @@ private: void rename_path_in_session_thread( std::string_view oldpath, std::string_view newname, - tr_torrent_rename_done_func const& callback, - void* callback_user_data); + std::function const& on_rename_done); void start_in_session_thread(); diff --git a/libtransmission/transmission.h b/libtransmission/transmission.h index 5523ae3ff..46254a2f5 100644 --- a/libtransmission/transmission.h +++ b/libtransmission/transmission.h @@ -828,9 +828,6 @@ void tr_torrentStart(tr_torrent* torrent); /** @brief Stop (pause) a torrent */ void tr_torrentStop(tr_torrent* torrent); -using tr_torrent_rename_done_func = std::function< - void(tr_torrent* torrent, char const* oldpath, char const* newname, int error, void* user_data)>; - /** * @brief Rename a file or directory in a torrent. * @@ -877,8 +874,7 @@ void tr_torrentRenamePath( tr_torrent* tor, std::string_view oldpath, std::string_view newname, - tr_torrent_rename_done_func callback, - void* callback_user_data); + std::function on_rename_done = {}); enum : uint8_t { diff --git a/macosx/FileRenameSheetController.mm b/macosx/FileRenameSheetController.mm index 0d59d2e1b..a3dcda314 100644 --- a/macosx/FileRenameSheetController.mm +++ b/macosx/FileRenameSheetController.mm @@ -106,10 +106,18 @@ typedef void (^CompletionBlock)(BOOL); - (IBAction)rename:(id)sender { - void (^completionHandler)(BOOL) = ^(BOOL didRename) { + __weak FileRenameSheetController* weakSelf = self; + auto completionHandler = [weakSelf](bool didRename) + { + FileRenameSheetController* strongSelf = weakSelf; + if (strongSelf == nil) + { + return; + } + if (didRename) { - [NSApp endSheet:self.window returnCode:NSModalResponseOK]; + [NSApp endSheet:strongSelf.window returnCode:NSModalResponseOK]; } else { diff --git a/macosx/Torrent.h b/macosx/Torrent.h index d524ac254..8bc6c12f5 100644 --- a/macosx/Torrent.h +++ b/macosx/Torrent.h @@ -5,6 +5,8 @@ #import #import +#include + #include @class FileListNode; @@ -117,10 +119,10 @@ extern NSString* const kTorrentDidChangeGroupNotification; @property(nonatomic, readonly) NSString* lastKnownDataLocation; - (NSString*)fileLocation:(FileListNode*)node; -- (void)renameTorrent:(NSString*)newName completionHandler:(void (^)(BOOL didRename))completionHandler; +- (void)renameTorrent:(NSString*)newName completionHandler:(std::function)completionHandler; - (void)renameFileNode:(FileListNode*)node withName:(NSString*)newName - completionHandler:(void (^)(BOOL didRename))completionHandler; + completionHandler:(std::function)completionHandler; @property(nonatomic, readonly) time_t eta; @property(nonatomic, readonly) CGFloat progress; diff --git a/macosx/Torrent.mm b/macosx/Torrent.mm index 79e861278..4bdff2e61 100644 --- a/macosx/Torrent.mm +++ b/macosx/Torrent.mm @@ -2,7 +2,9 @@ // It may be used under the MIT (SPDX: MIT) license. // License text can be found in the licenses/ folder. +#include #include +#include #include #include @@ -50,33 +52,20 @@ static dispatch_queue_t timeMachineExcludeQueue; - (void)renameFinished:(BOOL)success nodes:(NSArray*)nodes - completionHandler:(void (^)(BOOL))completionHandler + completionHandler:(std::function)completionHandler oldPath:(NSString*)oldPath newName:(NSString*)newName; +- (void)renamePath:(NSString*)oldPath + withName:(NSString*)newName + nodes:(NSArray*)nodes + completionHandler:(std::function)completionHandler; + @property(nonatomic, readonly) BOOL shouldShowEta; @property(nonatomic, readonly) NSString* etaString; @end -void renameCallback(tr_torrent* /*torrent*/, char const* oldPathCharString, char const* newNameCharString, int error, void* contextInfo) -{ - @autoreleasepool - { - NSString* oldPath = @(oldPathCharString); - NSString* newName = @(newNameCharString); - - dispatch_async(dispatch_get_main_queue(), ^{ - NSDictionary* contextDict = (__bridge_transfer NSDictionary*)contextInfo; - Torrent* torrentObject = contextDict[@"Torrent"]; - [torrentObject renameFinished:error == 0 nodes:contextDict[@"Nodes"] - completionHandler:contextDict[@"CompletionHandler"] - oldPath:oldPath - newName:newName]; - }); - } -} - bool trashDataFile(char const* filename, void* /*user_data*/, tr_error* error) { if (filename == NULL) @@ -854,28 +843,64 @@ bool trashDataFile(char const* filename, void* /*user_data*/, tr_error* error) } } -- (void)renameTorrent:(NSString*)newName completionHandler:(void (^)(BOOL didRename))completionHandler +- (void)renameTorrent:(NSString*)newName completionHandler:(std::function)completionHandler { NSParameterAssert(newName != nil); NSParameterAssert(![newName isEqualToString:@""]); - NSDictionary* contextInfo = @{ @"Torrent" : self, @"CompletionHandler" : [completionHandler copy] }; - - tr_torrentRenamePath(self.fHandle, tr_torrentName(self.fHandle), newName.UTF8String, renameCallback, (__bridge_retained void*)(contextInfo)); + NSString* oldPath = tr_strv_to_utf8_nsstring(tr_torrentName(self.fHandle)); + [self renamePath:oldPath withName:newName nodes:nil completionHandler:std::move(completionHandler)]; } - (void)renameFileNode:(FileListNode*)node withName:(NSString*)newName - completionHandler:(void (^)(BOOL didRename))completionHandler + completionHandler:(std::function)completionHandler { NSParameterAssert(node.torrent == self); NSParameterAssert(newName != nil); NSParameterAssert(![newName isEqualToString:@""]); - NSDictionary* contextInfo = @{ @"Torrent" : self, @"Nodes" : @[ node ], @"CompletionHandler" : [completionHandler copy] }; - NSString* oldPath = [node.path stringByAppendingPathComponent:node.name]; - tr_torrentRenamePath(self.fHandle, oldPath.UTF8String, newName.UTF8String, renameCallback, (__bridge_retained void*)(contextInfo)); + [self renamePath:oldPath withName:newName nodes:@[ node ] completionHandler:std::move(completionHandler)]; +} + +- (void)renamePath:(NSString*)oldPath + withName:(NSString*)newName + nodes:(NSArray*)nodes + completionHandler:(std::function)completionHandler +{ + struct RenameContext + { + __strong Torrent* torrent = nil; + __strong NSArray* nodes = nil; + __strong NSString* oldPath = nil; + __strong NSString* newName = nil; + std::function completionHandler; + }; + + auto context = std::make_shared(); + context->torrent = self; + context->nodes = nodes; + context->oldPath = oldPath; + context->newName = newName; + context->completionHandler = std::move(completionHandler); + + tr_torrentRenamePath( + self.fHandle, + oldPath.UTF8String, + newName.UTF8String, + [context](int error) + { + @autoreleasepool + { + dispatch_async(dispatch_get_main_queue(), ^{ + [context->torrent renameFinished:error == 0 nodes:context->nodes + completionHandler:context->completionHandler + oldPath:context->oldPath + newName:context->newName]; + }); + } + }); } - (time_t)eta @@ -2098,11 +2123,11 @@ bool trashDataFile(char const* filename, void* /*user_data*/, tr_error* error) - (void)renameFinished:(BOOL)success nodes:(NSArray*)nodes - completionHandler:(void (^)(BOOL))completionHandler + completionHandler:(std::function)completionHandler oldPath:(NSString*)oldPath newName:(NSString*)newName { - NSParameterAssert(completionHandler != nil); + NSParameterAssert(static_cast(completionHandler)); NSParameterAssert(oldPath != nil); NSParameterAssert(newName != nil); diff --git a/tests/libtransmission/rename-test.cc b/tests/libtransmission/rename-test.cc index 03e48db04..0317e484b 100644 --- a/tests/libtransmission/rename-test.cc +++ b/tests/libtransmission/rename-test.cc @@ -111,20 +111,10 @@ protected: static int torrentRenameAndWait(tr_torrent* tor, std::string_view const oldpath, std::string_view const newname) { - auto const on_rename_done = - [](tr_torrent* /*tor*/, char const* /*oldpath*/, char const* /*newname*/, int error, void* user_data) noexcept - { - *static_cast(user_data) = error; - }; - - int error = -1; - tr_torrentRenamePath(tor, oldpath, newname, on_rename_done, &error); - auto test = [&error]() - { - return error != -1; - }; - EXPECT_TRUE(waitFor(test, MaxWaitMsec)); - return error; + auto error = std::optional{}; + tr_torrentRenamePath(tor, oldpath, newname, [&error](int const err) { error = err; }); + EXPECT_TRUE(waitFor([&error]() { return error.has_value(); }, MaxWaitMsec)); + return error.value_or(ETIME); } };