From 1ec6b2b2328c114075171917ef417d13c35cc495 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 27 Nov 2023 23:04:04 -0600 Subject: [PATCH] refactor: make more tr_torrent fields private (#6305) * refactor: make tr_torrent::max_connected_peers_ private * refactor: make tr_torrent::completeness_ private * refactor: make tr_torrent::mark_edited() private * refactor: make tr_torrent::mark_changed() private * refactor: make tr_torrent::start_when_stable_ private * refactor: make tr_torrent::is_dirty() private * refactor: make tr_torrent::byte_span() private * refactor: remove unused non-const version of tr_torrent::metainfo() * refactor: make tr_torrent::set_dirty() private * refactor: remove unused tr_torrentStartMagnet() --- libtransmission/peer-mgr.cc | 2 - libtransmission/resume.cc | 22 ++--- libtransmission/torrent.cc | 171 +++++++++++++++------------------ libtransmission/torrent.h | 85 +++++++++------- libtransmission/transmission.h | 3 - 5 files changed, 134 insertions(+), 149 deletions(-) diff --git a/libtransmission/peer-mgr.cc b/libtransmission/peer-mgr.cc index d18dd6822..91728263f 100644 --- a/libtransmission/peer-mgr.cc +++ b/libtransmission/peer-mgr.cc @@ -487,7 +487,6 @@ public: tor->bytes_uploaded_ += event.length; tr_announcerAddBytes(tor, TR_ANN_UP, event.length); tor->set_date_active(now); - tor->set_dirty(); tor->session->add_uploaded(event.length); msgs->peer_info->set_latest_piece_data_time(now); @@ -800,7 +799,6 @@ private: { tor->bytes_downloaded_ += sent_length; tor->set_date_active(now); - tor->set_dirty(); tor->session->add_downloaded(sent_length); } diff --git a/libtransmission/resume.cc b/libtransmission/resume.cc index 8624fe58d..2bc5ca19e 100644 --- a/libtransmission/resume.cc +++ b/libtransmission/resume.cc @@ -468,7 +468,7 @@ void saveProgress(tr_variant* dict, tr_torrent const* tor, tr_torrent::ResumeHel bitfieldToRaw(helper.checked_pieces(), tr_variantDictAdd(prog, TR_KEY_pieces)); /* add the progress */ - if (tor->completeness == TR_SEED) + if (tor->is_seed()) { tr_variantDictAddStrView(prog, TR_KEY_have, "all"sv); } @@ -627,10 +627,9 @@ auto loadProgress(tr_variant* dict, tr_torrent* tor, tr_torrent::ResumeHelper& h // --- -tr_resume::fields_t loadFromFile(tr_torrent* tor, tr_torrent::ResumeHelper& helper, tr_resume::fields_t fields_to_load) +tr_resume::fields_t load_from_file(tr_torrent* tor, tr_torrent::ResumeHelper& helper, tr_resume::fields_t fields_to_load) { TR_ASSERT(tr_isTorrent(tor)); - auto const was_dirty = tor->is_dirty(); tr_torrent_metainfo::migrate_file(tor->session->resumeDir(), tor->name(), tor->info_hash_string(), ".resume"sv); @@ -688,13 +687,13 @@ tr_resume::fields_t loadFromFile(tr_torrent* tor, tr_torrent::ResumeHelper& help if ((fields_to_load & tr_resume::MaxPeers) != 0 && tr_variantDictFindInt(&top, TR_KEY_max_peers, &i)) { - tor->max_connected_peers_ = static_cast(i); + tor->set_peer_limit(static_cast(i)); fields_loaded |= tr_resume::MaxPeers; } if (auto val = bool{}; (fields_to_load & tr_resume::Run) != 0 && tr_variantDictFindBool(&top, TR_KEY_paused, &val)) { - tor->start_when_stable = !val; + helper.load_start_when_stable(!val); fields_loaded |= tr_resume::Run; } @@ -796,11 +795,6 @@ tr_resume::fields_t loadFromFile(tr_torrent* tor, tr_torrent::ResumeHelper& help fields_loaded |= loadGroup(&top, tor); } - /* loading the resume file triggers of a lot of changes, - * but none of them needs to trigger a re-saving of the - * same resume information... */ - tor->set_dirty(was_dirty); - return fields_loaded; } @@ -817,7 +811,7 @@ auto set_from_ctor( { if (auto const val = ctor.paused(mode); val) { - tor->start_when_stable = !*val; + helper.load_start_when_stable(!*val); ret |= tr_resume::Run; } } @@ -826,7 +820,7 @@ auto set_from_ctor( { if (auto const val = ctor.peer_limit(mode); val) { - tor->max_connected_peers_ = *val; + tor->set_peer_limit(*val); ret |= tr_resume::MaxPeers; } } @@ -870,7 +864,7 @@ fields_t load(tr_torrent* tor, tr_torrent::ResumeHelper& helper, fields_t fields ret |= use_mandatory_fields(tor, helper, fields_to_load, ctor); fields_to_load &= ~ret; - ret |= loadFromFile(tor, helper, fields_to_load); + ret |= load_from_file(tor, helper, fields_to_load); fields_to_load &= ~ret; ret |= use_fallback_fields(tor, helper, fields_to_load, ctor); @@ -904,7 +898,7 @@ void save(tr_torrent* const tor, tr_torrent::ResumeHelper const& helper) tr_variantDictAddInt(&top, TR_KEY_uploaded, tor->bytes_uploaded_.ever()); tr_variantDictAddInt(&top, TR_KEY_max_peers, tor->peer_limit()); tr_variantDictAddInt(&top, TR_KEY_bandwidth_priority, tor->get_priority()); - tr_variantDictAddBool(&top, TR_KEY_paused, !tor->start_when_stable); + tr_variantDictAddBool(&top, TR_KEY_paused, !helper.start_when_stable()); tr_variantDictAddBool(&top, TR_KEY_sequentialDownload, tor->is_sequential_download()); savePeers(&top, tor); diff --git a/libtransmission/torrent.cc b/libtransmission/torrent.cc index 151fe12c8..d5c81c3fd 100644 --- a/libtransmission/torrent.cc +++ b/libtransmission/torrent.cc @@ -597,17 +597,6 @@ bool torrentShouldQueue(tr_torrent const* const tor) return tor->session->count_queue_free_slots(dir) == 0; } -void torrentResetTransferStats(tr_torrent* tor) -{ - auto const lock = tor->unique_lock(); - - tor->bytes_uploaded_.start_new_session(); - tor->bytes_downloaded_.start_new_session(); - tor->bytes_corrupt_.start_new_session(); - - tor->set_dirty(); -} - bool removeTorrentFile(char const* filename, void* /*user_data*/, tr_error* error) { return tr_sys_path_remove(filename, error); @@ -737,13 +726,17 @@ void tr_torrent::start_in_session_thread() time_t const now = tr_time(); is_running_ = true; - completeness = completion_.status(); + completeness_ = completion_.status(); date_started_ = now; mark_changed(); error().clear(); finished_seeding_by_idle_ = false; - torrentResetTransferStats(this); + bytes_uploaded_.start_new_session(); + bytes_downloaded_.start_new_session(); + bytes_corrupt_.start_new_session(); + set_dirty(); + session->announcer_->startTorrent(this); lpdAnnounceAt = now; started_.emit(this); @@ -786,7 +779,7 @@ void tr_torrentStop(tr_torrent* tor) auto const lock = tor->unique_lock(); - tor->start_when_stable = false; + tor->start_when_stable_ = false; tor->set_dirty(); tor->session->runInSessionThread([tor]() { tor->stop_now(); }); } @@ -897,7 +890,7 @@ void tr_torrent::on_metainfo_completed() date_done_ = date_added_; recheck_completeness(); - if (start_when_stable) + if (start_when_stable_) { start(false, {}); } @@ -963,7 +956,7 @@ void tr_torrent::init(tr_ctor const& ctor) tr_torrent_metainfo::migrate_file(session->torrentDir(), name(), info_hash_string(), ".torrent"sv); } - completeness = completion_.status(); + completeness_ = completion_.status(); ctor.init_torrent_priorities(*this); ctor.init_torrent_wanted(*this); @@ -1038,7 +1031,7 @@ void tr_torrent::init(tr_ctor const& ctor) { on_metainfo_completed(); } - else if (start_when_stable) + else if (start_when_stable_) { auto const bypass_queue = !has_metainfo; // to fetch metainfo from peers start(bypass_queue, has_any_local_data); @@ -1057,8 +1050,8 @@ void tr_torrent::set_metainfo(tr_torrent_metainfo tm) got_metainfo_.emit(this); session->onMetadataCompleted(this); - this->set_dirty(); - this->mark_edited(); + set_dirty(); + mark_edited(); on_metainfo_completed(); this->on_announce_list_changed(); @@ -1417,7 +1410,7 @@ tr_file_view tr_torrentFile(tr_torrent const* tor, tr_file_index_t file) auto const length = tor->file_size(file); auto const [begin, end] = tor->pieces_in_file(file); - if (tor->completeness == TR_SEED || length == 0) + if (tor->is_seed() || length == 0) { return { subpath.c_str(), length, length, 1.0, begin, end, priority, wanted }; } @@ -1516,33 +1509,22 @@ void tr_torrentAmountFinished(tr_torrent const* tor, float* tabs, int n_tabs) // --- Start/Stop Callback -namespace -{ -void tr_torrentStartImpl(tr_torrent* tor, bool bypass_queue) -{ - if (!tr_isTorrent(tor)) - { - return; - } - - tor->start_when_stable = true; - tor->start(bypass_queue, {}); -} -} // namespace - void tr_torrentStart(tr_torrent* tor) { - tr_torrentStartImpl(tor, false); + if (tr_isTorrent(tor)) + { + tor->start_when_stable_ = true; + tor->start(false /*bypass_queue*/, {}); + } } void tr_torrentStartNow(tr_torrent* tor) { - tr_torrentStartImpl(tor, true); -} - -void tr_torrentStartMagnet(tr_torrent* tor) -{ - tr_torrentStart(tor); + if (tr_isTorrent(tor)) + { + tor->start_when_stable_ = true; + tor->start(true /*bypass_queue*/, {}); + } } // --- @@ -1577,7 +1559,7 @@ void tr_torrentVerify(tr_torrent* tor) tor->error().set_local_error( _("Paused torrent as no data was found! Ensure your drives are connected or use \"Set Location\", " "then use \"Verify Local Data\" again. To re-download, start the torrent.")); - tor->start_when_stable = false; + tor->start_when_stable_ = false; } tor->session->verify_add(tor); @@ -1671,7 +1653,7 @@ void tr_torrent::VerifyMediator::on_verify_done(bool const aborted) tor->verify_done_callback_(tor); } - if (tor->start_when_stable) + if (tor->start_when_stable_) { tor->start(false, !tor->checked_pieces_.has_none()); } @@ -1729,9 +1711,7 @@ void tr_torrent::recheck_completeness() needs_completeness_check_ = false; - auto const new_completeness = completion_.status(); - - if (new_completeness != completeness) + if (auto const new_completeness = completion_.status(); completeness_ != new_completeness) { bool const recent_change = bytes_downloaded_.during_this_session() != 0U; bool const was_running = is_running(); @@ -1742,35 +1722,35 @@ void tr_torrent::recheck_completeness() this, fmt::format( "State changed from {} to {}", - get_completion_string(completeness), + get_completion_string(completeness_), get_completion_string(new_completeness))); } - this->completeness = new_completeness; - this->session->closeTorrentFiles(this); + completeness_ = new_completeness; + session->closeTorrentFiles(this); - if (this->is_done()) + if (is_done()) { if (recent_change) { tr_announcerTorrentCompleted(this); - this->mark_changed(); + mark_changed(); date_done_ = tr_time(); } - if (this->current_dir() == this->incomplete_dir()) + if (current_dir() == incomplete_dir()) { - this->set_location(this->download_dir(), true, nullptr); + set_location(download_dir(), true, nullptr); } done_.emit(this, recent_change); } - this->session->onTorrentCompletenessChanged(this, completeness, was_running); + session->onTorrentCompletenessChanged(this, completeness_, was_running); - this->set_dirty(); + set_dirty(); - if (this->is_done()) + if (is_done()) { save_resume_file(); callScriptIfEnabled(this, TR_SCRIPT_ON_TORRENT_DONE); @@ -1836,7 +1816,7 @@ tr_priority_t tr_torrentGetPriority(tr_torrent const* tor) return tor->get_priority(); } -void tr_torrentSetPriority(tr_torrent* tor, tr_priority_t priority) +void tr_torrentSetPriority(tr_torrent* const tor, tr_priority_t const priority) { TR_ASSERT(tr_isTorrent(tor)); TR_ASSERT(tr_isPriority(priority)); @@ -1855,12 +1835,7 @@ void tr_torrentSetPeerLimit(tr_torrent* tor, uint16_t max_connected_peers) { TR_ASSERT(tr_isTorrent(tor)); - if (tor->max_connected_peers_ != max_connected_peers) - { - tor->max_connected_peers_ = max_connected_peers; - - tor->set_dirty(); - } + tor->set_peer_limit(max_connected_peers); } uint16_t tr_torrentGetPeerLimit(tr_torrent const* tor) @@ -2413,77 +2388,69 @@ void renameTorrentFileString(tr_torrent* tor, std::string_view oldpath, std::str } } -void torrentRenamePath( - tr_torrent* const tor, - std::string const& oldpath, // NOLINT performance-unnecessary-value-param - std::string const& newname, // NOLINT performance-unnecessary-value-param - tr_torrent_rename_done_func callback, +} // namespace rename_helpers +} // namespace + +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) { - TR_ASSERT(tr_isTorrent(tor)); + using namespace rename_helpers; - int error = 0; + auto error = int{ 0 }; - if (!renameArgsAreValid(tor, oldpath, newname)) + if (!renameArgsAreValid(this, oldpath, newname)) { error = EINVAL; } - else if (auto const file_indices = renameFindAffectedFiles(tor, oldpath); std::empty(file_indices)) + else if (auto const file_indices = renameFindAffectedFiles(this, oldpath); std::empty(file_indices)) { error = EINVAL; } else { - error = renamePath(tor, oldpath, newname); + error = renamePath(this, oldpath, newname); if (error == 0) { /* update tr_info.files */ for (auto const& file_index : file_indices) { - renameTorrentFileString(tor, oldpath, newname, file_index); + renameTorrentFileString(this, oldpath, newname, file_index); } /* update tr_info.name if user changed the toplevel */ - if (std::size(file_indices) == tor->file_count() && !tr_strv_contains(oldpath, '/')) + if (std::size(file_indices) == file_count() && !tr_strv_contains(oldpath, '/')) { - tor->set_name(newname); + set_name(newname); } - tor->mark_edited(); - tor->set_dirty(); + mark_edited(); + set_dirty(); } } - /// + mark_changed(); - tor->mark_changed(); - - /* callback */ if (callback != nullptr) { - (*callback)(tor, oldpath.c_str(), newname.c_str(), error, callback_user_data); + auto const szold = tr_pathbuf{ oldpath }; + auto const sznew = tr_pathbuf{ newname }; + (*callback)(this, szold.c_str(), sznew.c_str(), error, callback_user_data); } } -} // namespace rename_helpers -} // namespace - void tr_torrent::rename_path( std::string_view oldpath, std::string_view newname, tr_torrent_rename_done_func callback, void* callback_user_data) { - using namespace rename_helpers; - this->session->runInSessionThread( - torrentRenamePath, - this, - std::string{ oldpath }, - std::string{ newname }, - callback, - callback_user_data); + [this, oldpath = std::string(oldpath), newname = std::string(newname), callback, callback_user_data]() + { rename_path_in_session_thread(oldpath, newname, callback, callback_user_data); }); } void tr_torrentRenamePath( @@ -2535,8 +2502,8 @@ void tr_torrent::mark_changed() } bool const checked = check_piece(piece); - this->mark_changed(); - this->set_dirty(); + mark_changed(); + set_dirty(); checked_pieces_.set(piece, checked); return checked; @@ -2664,6 +2631,18 @@ void tr_torrent::ResumeHelper::load_incomplete_dir(std::string_view const dir) n // --- +void tr_torrent::ResumeHelper::load_start_when_stable(bool const val) noexcept +{ + tor_.start_when_stable_ = val; +} + +bool tr_torrent::ResumeHelper::start_when_stable() const noexcept +{ + return tor_.start_when_stable_; +} + +// --- + std::vector const& tr_torrent::ResumeHelper::file_mtimes() const noexcept { return tor_.file_mtimes_; diff --git a/libtransmission/torrent.h b/libtransmission/torrent.h index 1044e4ddc..072832929 100644 --- a/libtransmission/torrent.h +++ b/libtransmission/torrent.h @@ -83,6 +83,7 @@ struct tr_torrent final : public tr_completion::torrent_view void load_incomplete_dir(std::string_view dir) noexcept; void load_seconds_downloading_before_current_start(time_t when) noexcept; void load_seconds_seeding_before_current_start(time_t when) noexcept; + void load_start_when_stable(bool val) noexcept; [[nodiscard]] tr_bitfield const& blocks() const noexcept; [[nodiscard]] tr_bitfield const& checked_pieces() const noexcept; @@ -92,6 +93,7 @@ struct tr_torrent final : public tr_completion::torrent_view [[nodiscard]] time_t date_done() const noexcept; [[nodiscard]] time_t seconds_downloading(time_t now) const noexcept; [[nodiscard]] time_t seconds_seeding(time_t now) const noexcept; + [[nodiscard]] bool start_when_stable() const noexcept; private: friend class libtransmission::test::RenameTest_multifileTorrent_Test; @@ -361,17 +363,17 @@ public: [[nodiscard]] constexpr bool is_done() const noexcept { - return completeness != TR_LEECH; + return completeness_ != TR_LEECH; } [[nodiscard]] constexpr bool is_seed() const noexcept { - return completeness == TR_SEED; + return completeness_ == TR_SEED; } [[nodiscard]] constexpr bool is_partial_seed() const noexcept { - return completeness == TR_PARTIAL_SEED; + return completeness_ == TR_PARTIAL_SEED; } void amount_done_bins(float* tab, int n_tabs) const @@ -391,11 +393,6 @@ public: return fpm_.file_offset(loc.byte, include_empty_files); } - [[nodiscard]] auto byte_span(tr_file_index_t file) const - { - return fpm_.byte_span(file); - } - /// WANTED [[nodiscard]] bool piece_is_wanted(tr_piece_index_t piece) const final @@ -456,11 +453,6 @@ public: /// METAINFO - [[nodiscard]] constexpr auto& metainfo() noexcept - { - return metainfo_; - } - [[nodiscard]] constexpr auto const& metainfo() const noexcept { return metainfo_; @@ -681,6 +673,7 @@ public: this->date_active_ = when; bump_date_changed(when); + set_dirty(); } [[nodiscard]] constexpr auto activity() const noexcept @@ -751,19 +744,6 @@ public: void start(bool bypass_queue, std::optional has_any_local_data); - [[nodiscard]] constexpr auto is_dirty() const noexcept - { - return is_dirty_; - } - - constexpr void set_dirty(bool dirty = true) noexcept - { - is_dirty_ = dirty; - } - - void mark_edited(); - void mark_changed(); - [[nodiscard]] constexpr auto has_changed_since(time_t when) const noexcept { return date_changed_ > when; @@ -786,6 +766,15 @@ public: return max_connected_peers_; } + constexpr void set_peer_limit(uint16_t val) noexcept + { + if (max_connected_peers_ != val) + { + max_connected_peers_ = val; + set_dirty(); + } + } + // --- idleness void set_idle_limit_mode(tr_idlelimit mode) noexcept @@ -973,14 +962,6 @@ public: time_t lpdAnnounceAt = 0; - tr_completeness completeness = TR_LEECH; - - uint16_t max_connected_peers_ = TR_DEFAULT_PEER_LIMIT_TORRENT; - - // start the torrent after all the startup scaffolding is done, - // e.g. fetching metadata from peers and/or verifying the torrent - bool start_when_stable = false; - private: friend tr_file_view tr_torrentFile(tr_torrent const* tor, tr_file_index_t file); friend tr_stat const* tr_torrentStat(tr_torrent* tor); @@ -989,7 +970,11 @@ private: friend void tr_torrentFreeInSessionThread(tr_torrent* tor); friend void tr_torrentRemove(tr_torrent* tor, bool delete_flag, tr_fileFunc delete_func, void* user_data); friend void tr_torrentSetDownloadDir(tr_torrent* tor, char const* path); + friend void tr_torrentSetPriority(tr_torrent* tor, tr_priority_t priority); + friend void tr_torrentStart(tr_torrent* tor); + friend void tr_torrentStartNow(tr_torrent* tor); friend void tr_torrentStop(tr_torrent* tor); + friend void tr_torrentUseSessionLimits(tr_torrent* tor, bool enabled); friend void tr_torrentVerify(tr_torrent* tor); enum class VerifyState : uint8_t @@ -1210,6 +1195,11 @@ private: // must be called after the torrent's announce list changes. void on_announce_list_changed(); + [[nodiscard]] auto byte_span(tr_file_index_t file) const + { + return fpm_.byte_span(file); + } + // --- void set_has_piece(tr_piece_index_t piece, bool has) @@ -1217,6 +1207,19 @@ private: completion_.set_has_piece(piece, has); } + void mark_changed(); + void mark_edited(); + + constexpr void set_dirty(bool dirty = true) noexcept + { + is_dirty_ = dirty; + } + + [[nodiscard]] constexpr auto is_dirty() const noexcept + { + return is_dirty_; + } + void init(tr_ctor const& ctor); void on_metainfo_updated(); @@ -1230,6 +1233,12 @@ private: void set_location_in_session_thread(std::string_view path, bool move_from_old_path, int volatile* setme_state); + void rename_path_in_session_thread( + std::string_view oldpath, + std::string_view newname, + tr_torrent_rename_done_func callback, + void* const callback_user_data); + void start_in_session_thread(); void stop_now(); @@ -1314,8 +1323,12 @@ private: VerifyState verify_state_ = VerifyState::None; + tr_completeness completeness_ = TR_LEECH; + uint16_t idle_limit_minutes_ = 0; + uint16_t max_connected_peers_ = TR_DEFAULT_PEER_LIMIT_TORRENT; + bool is_deleting_ = false; bool is_dirty_ = false; bool is_queued_ = false; @@ -1327,6 +1340,10 @@ private: bool needs_completeness_check_ = true; bool sequential_download_ = false; + + // start the torrent after all the startup scaffolding is done, + // e.g. fetching metadata from peers and/or verifying the torrent + bool start_when_stable_ = false; }; // --- diff --git a/libtransmission/transmission.h b/libtransmission/transmission.h index 22048db6e..12e0e5656 100644 --- a/libtransmission/transmission.h +++ b/libtransmission/transmission.h @@ -589,9 +589,6 @@ void tr_sessionSetAntiBruteForceEnabled(tr_session* session, bool enabled); /** @brief Like `tr_torrentStart()`, but resumes right away regardless of the queues. */ void tr_torrentStartNow(tr_torrent* tor); -/** @brief DEPRECATED. Equivalent to `tr_torrentStart()`. Use that instead. */ -void tr_torrentStartMagnet(tr_torrent* tor); - /** @brief Return the queued torrent's position in the queue it's in. [0...n) */ size_t tr_torrentGetQueuePosition(tr_torrent const* tor);