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()
2026-05-08 09:39:08 +01:00 · 2023-11-27 23:04:04 -06:00
parent 5d64c860ea
commit 1ec6b2b232
5 changed files with 134 additions and 149 deletions
@@ -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);
    }

@@ -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<uint16_t>(i);
+        tor->set_peer_limit(static_cast<uint16_t>(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);

@@ -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<time_t> const& tr_torrent::ResumeHelper::file_mtimes() const noexcept
 {
    return tor_.file_mtimes_;
@@ -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<bool> 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;
 };

 // ---
@@ -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);