From 2e46bad22d9e7c716a5cc3e9cba849356b0b4bca Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Sat, 2 Dec 2023 14:16:36 -0600 Subject: [PATCH] refactor: constify the inout module (#6328) * refactor: move tr_preallocation_mode info tr_open_files * refactor: remove unnecessary error nullptr check * refactor: use snake_case for private method names in inout.cc * refactor: extract-method get_fd() from read_or_write_bytes() * refactor: make function args const where possible * refactor: simplify read_or_write_bytes() * refactor: make buflen a uint64_t in read_or_write_piece() * refactor: move tr_torrentStop() logic from read_or_write_piece() to tr_ioWrite() * refactor: make tr_ioFoo() functions take a const torrent * refactor: make tr_cache::close_torrent_files() take a tor_id instead of a torrent --- libtransmission/cache.cc | 36 ++- libtransmission/cache.h | 12 +- libtransmission/inout.cc | 296 +++++++++++------------ libtransmission/inout.h | 8 +- libtransmission/open-files.cc | 8 +- libtransmission/open-files.h | 9 +- libtransmission/peer-msgs.cc | 4 +- libtransmission/session-settings.h | 3 +- libtransmission/session.cc | 10 +- libtransmission/session.h | 4 +- libtransmission/torrent.cc | 15 +- libtransmission/torrent.h | 2 +- libtransmission/transmission.h | 7 - libtransmission/variant-converters.cc | 21 +- tests/libtransmission/open-files-test.cc | 22 +- tests/libtransmission/settings-test.cc | 9 +- 16 files changed, 226 insertions(+), 240 deletions(-) diff --git a/libtransmission/cache.cc b/libtransmission/cache.cc index c2c18fe70..f9e76374a 100644 --- a/libtransmission/cache.cc +++ b/libtransmission/cache.cc @@ -25,9 +25,9 @@ #include "libtransmission/torrents.h" #include "libtransmission/tr-assert.h" -Cache::Key Cache::make_key(tr_torrent const* torrent, tr_block_info::Location loc) noexcept +Cache::Key Cache::make_key(tr_torrent const& tor, tr_block_info::Location const loc) noexcept { - return std::make_pair(torrent->id(), loc.block); + return std::make_pair(tor.id(), loc.block); } Cache::CIter Cache::find_span_end(CIter span_begin, CIter end) noexcept @@ -104,7 +104,7 @@ int Cache::write_contiguous(CIter const begin, CIter const end) const auto const loc = tor->block_loc(block); - if (auto const err = tr_ioWrite(tor, loc, outlen, out); err != 0) + if (auto const err = tr_ioWrite(*tor, loc, outlen, out); err != 0) { return err; } @@ -130,7 +130,7 @@ Cache::Cache(tr_torrents const& torrents, Memory const max_size) // --- -int Cache::write_block(tr_torrent_id_t tor_id, tr_block_index_t block, std::unique_ptr writeme) +int Cache::write_block(tr_torrent_id_t const tor_id, tr_block_index_t const block, std::unique_ptr writeme) { if (max_blocks_ == 0U) { @@ -140,7 +140,7 @@ int Cache::write_block(tr_torrent_id_t tor_id, tr_block_index_t block, std::uniq // already has a cache layer for the very purpose of this cache // https://github.com/transmission/transmission/pull/5668 auto* const tor = torrents_.get(tor_id); - return tr_ioWrite(tor, tor->block_loc(block), std::size(*writeme), std::data(*writeme)); + return tor == nullptr ? EINVAL : tr_ioWrite(*tor, tor->block_loc(block), std::size(*writeme), std::data(*writeme)); } auto const key = Key{ tor_id, block }; @@ -159,12 +159,12 @@ int Cache::write_block(tr_torrent_id_t tor_id, tr_block_index_t block, std::uniq return cache_trim(); } -Cache::CIter Cache::get_block(tr_torrent const* torrent, tr_block_info::Location const& loc) noexcept +Cache::CIter Cache::get_block(tr_torrent const& tor, tr_block_info::Location const& loc) noexcept { if (auto const [begin, end] = std::equal_range( std::begin(blocks_), std::end(blocks_), - make_key(torrent, loc), + make_key(tor, loc), CompareCacheBlockByKey); begin < end) { @@ -174,25 +174,25 @@ Cache::CIter Cache::get_block(tr_torrent const* torrent, tr_block_info::Location return std::end(blocks_); } -int Cache::read_block(tr_torrent* torrent, tr_block_info::Location const& loc, size_t len, uint8_t* setme) +int Cache::read_block(tr_torrent const& tor, tr_block_info::Location const& loc, size_t len, uint8_t* setme) { - if (auto const iter = get_block(torrent, loc); iter != std::end(blocks_)) + if (auto const iter = get_block(tor, loc); iter != std::end(blocks_)) { std::copy_n(std::begin(*iter->buf), len, setme); return {}; } - return tr_ioRead(torrent, loc, len, setme); + return tr_ioRead(tor, loc, len, setme); } -int Cache::prefetch_block(tr_torrent* torrent, tr_block_info::Location const& loc, size_t len) +int Cache::prefetch_block(tr_torrent const& tor, tr_block_info::Location const& loc, size_t len) { - if (auto const iter = get_block(torrent, loc); iter != std::end(blocks_)) + if (auto const iter = get_block(tor, loc); iter != std::end(blocks_)) { return {}; // already have it } - return tr_ioPrefetch(torrent, loc, len); + return tr_ioPrefetch(tor, loc, len); } // --- @@ -215,20 +215,18 @@ int Cache::flush_span(CIter const begin, CIter const end) return {}; } -int Cache::flush_file(tr_torrent const* const torrent, tr_file_index_t const file) +int Cache::flush_file(tr_torrent const& tor, tr_file_index_t const file) { - auto const tor_id = torrent->id(); - auto const [block_begin, block_end] = torrent->block_span_for_file(file); + auto const tor_id = tor.id(); + auto const [block_begin, block_end] = tor.block_span_for_file(file); return flush_span( std::lower_bound(std::begin(blocks_), std::end(blocks_), std::make_pair(tor_id, block_begin), CompareCacheBlockByKey), std::lower_bound(std::begin(blocks_), std::end(blocks_), std::make_pair(tor_id, block_end), CompareCacheBlockByKey)); } -int Cache::flush_torrent(tr_torrent const* torrent) +int Cache::flush_torrent(tr_torrent_id_t const tor_id) { - auto const tor_id = torrent->id(); - return flush_span( std::lower_bound(std::begin(blocks_), std::end(blocks_), std::make_pair(tor_id, 0), CompareCacheBlockByKey), std::lower_bound(std::begin(blocks_), std::end(blocks_), std::make_pair(tor_id + 1, 0), CompareCacheBlockByKey)); diff --git a/libtransmission/cache.h b/libtransmission/cache.h index 26660464c..99c845fc9 100644 --- a/libtransmission/cache.h +++ b/libtransmission/cache.h @@ -38,10 +38,10 @@ public: // @return any error code from cacheTrim() int write_block(tr_torrent_id_t tor, tr_block_index_t block, std::unique_ptr writeme); - int read_block(tr_torrent* torrent, tr_block_info::Location const& loc, size_t len, uint8_t* setme); - int prefetch_block(tr_torrent* torrent, tr_block_info::Location const& loc, size_t len); - int flush_torrent(tr_torrent const* torrent); - int flush_file(tr_torrent const* torrent, tr_file_index_t file); + int read_block(tr_torrent const& tor, tr_block_info::Location const& loc, size_t len, uint8_t* setme); + int prefetch_block(tr_torrent const& tor, tr_block_info::Location const& loc, size_t len); + int flush_torrent(tr_torrent_id_t tor_id); + int flush_file(tr_torrent const& tor, tr_file_index_t file); private: using Key = std::pair; @@ -55,7 +55,7 @@ private: using Blocks = std::vector; using CIter = Blocks::const_iterator; - [[nodiscard]] static Key make_key(tr_torrent const* torrent, tr_block_info::Location loc) noexcept; + [[nodiscard]] static Key make_key(tr_torrent const& tor, tr_block_info::Location loc) noexcept; [[nodiscard]] static std::pair find_biggest_span(CIter begin, CIter end) noexcept; @@ -78,7 +78,7 @@ private: return max_size.base_quantity() / tr_block_info::BlockSize; } - [[nodiscard]] CIter get_block(tr_torrent const* torrent, tr_block_info::Location const& loc) noexcept; + [[nodiscard]] CIter get_block(tr_torrent const& tor, tr_block_info::Location const& loc) noexcept; tr_torrents const& torrents_; diff --git a/libtransmission/inout.cc b/libtransmission/inout.cc index 5216a8605..337e6ec4d 100644 --- a/libtransmission/inout.cc +++ b/libtransmission/inout.cc @@ -33,13 +33,13 @@ using namespace std::literals; namespace { -bool readEntireBuf(tr_sys_file_t fd, uint64_t file_offset, uint8_t* buf, uint64_t buflen, tr_error* error) +bool read_entire_buf(tr_sys_file_t const fd, uint64_t file_offset, uint8_t* buf, uint64_t buflen, tr_error& error) { while (buflen > 0U) { auto n_read = uint64_t{}; - if (!tr_sys_file_read_at(fd, buf, buflen, file_offset, &n_read, error)) + if (!tr_sys_file_read_at(fd, buf, buflen, file_offset, &n_read, &error)) { return false; } @@ -52,13 +52,13 @@ bool readEntireBuf(tr_sys_file_t fd, uint64_t file_offset, uint8_t* buf, uint64_ return true; } -bool writeEntireBuf(tr_sys_file_t fd, uint64_t file_offset, uint8_t const* buf, uint64_t buflen, tr_error* error) +bool write_entire_buf(tr_sys_file_t const fd, uint64_t file_offset, uint8_t const* buf, uint64_t buflen, tr_error& error) { while (buflen > 0U) { auto n_written = uint64_t{}; - if (!tr_sys_file_write_at(fd, buf, buflen, file_offset, &n_written, error)) + if (!tr_sys_file_write_at(fd, buf, buflen, file_offset, &n_written, &error)) { return false; } @@ -71,6 +71,58 @@ bool writeEntireBuf(tr_sys_file_t fd, uint64_t file_offset, uint8_t const* buf, return true; } +[[nodiscard]] std::optional get_fd( + tr_session& session, + tr_open_files& open_files, + tr_torrent const& tor, + bool const writable, + tr_file_index_t const file_index, + tr_error& error) +{ + auto const tor_id = tor.id(); + + // is the file already open in the fd pool? + if (auto const fd = open_files.get(tor_id, file_index, writable); fd) + { + return fd; + } + + // does the file exist? + auto const file_size = tor.file_size(file_index); + auto const create_if_missing = writable && tor.file_is_wanted(file_index); + auto const prealloc = create_if_missing ? session.preallocationMode() : tr_open_files::Preallocation::None; + if (auto const found = tor.find_file(file_index); found) + { + return open_files.get(tor_id, file_index, writable, found->filename(), prealloc, file_size); + } + + // do we want to create it? + auto err = ENOENT; + if (create_if_missing) + { + auto const base = tor.current_dir(); + auto const suffix = session.isIncompleteFileNamingEnabled() ? tr_torrent_files::PartialFileSuffix : ""sv; + auto const filename = tr_pathbuf{ base, '/', tor.file_subpath(file_index), suffix }; + if (auto const fd = open_files.get(tor_id, file_index, writable, filename, prealloc, file_size); fd) + { + // make a note that we just created a file + session.add_file_created(); + return fd; + } + + err = errno; + } + + error.set( + err, + fmt::format( + _("Couldn't get '{path}': {error} ({error_code})"), + fmt::arg("path", tor.file_subpath(file_index)), + fmt::arg("error", tr_strerror(err)), + fmt::arg("error_code", err))); + return {}; +} + enum class IoMode { Read, @@ -78,191 +130,109 @@ enum class IoMode Write }; -bool getFilename(tr_pathbuf& setme, tr_torrent const* tor, tr_file_index_t file_index, bool do_write) +void read_or_write_bytes( + tr_session& session, + tr_open_files& open_files, + tr_torrent const& tor, + IoMode const io_mode, + tr_file_index_t const file_index, + uint64_t const file_offset, + uint8_t* const buf, + uint64_t const buflen, + tr_error& error) { - if (auto found = tor->find_file(file_index); found) - { - setme.assign(found->filename()); - return true; - } - - if (!do_write) - { - return false; - } - - // We didn't find the file that we want to write to. - // Let's figure out where it goes so that we can create it. - auto const base = tor->current_dir(); - auto const suffix = tor->session->isIncompleteFileNamingEnabled() ? tr_torrent_files::PartialFileSuffix : ""sv; - setme.assign(base, '/', tor->file_subpath(file_index), suffix); - return true; -} - -void readOrWriteBytes( - tr_session* session, - tr_torrent* tor, - IoMode io_mode, - tr_file_index_t file_index, - uint64_t file_offset, - uint8_t* buf, - size_t buflen, - tr_error* error) -{ - TR_ASSERT(file_index < tor->file_count()); - - bool const do_write = io_mode == IoMode::Write; - auto const file_size = tor->file_size(file_index); + TR_ASSERT(file_index < tor.file_count()); + auto const file_size = tor.file_size(file_index); TR_ASSERT(file_size == 0U || file_offset < file_size); TR_ASSERT(file_offset + buflen <= file_size); - if (file_size == 0U) { return; } - auto local_error = tr_error{}; - if (error == nullptr) + auto const writable = io_mode == IoMode::Write; + auto const fd = get_fd(session, open_files, tor, writable, file_index, error); + if (!fd || error) { - error = &local_error; - } - - // --- Find the fd - - auto fd = session->openFiles().get(tor->id(), file_index, do_write); - auto filename = tr_pathbuf{}; - if (!fd && !getFilename(filename, tor, file_index, do_write)) - { - auto const err = ENOENT; - error->set( - err, - fmt::format( - _("Couldn't get '{path}': {error} ({error_code})"), - fmt::arg("path", tor->file_subpath(file_index)), - fmt::arg("error", tr_strerror(err)), - fmt::arg("error_code", err))); - return; - } - - if (!fd) // not in the cache, so open or create it now - { - // open (and maybe create) the file - auto const prealloc = (!do_write || !tor->file_is_wanted(file_index)) ? TR_PREALLOCATE_NONE : - tor->session->preallocationMode(); - fd = session->openFiles().get(tor->id(), file_index, do_write, filename, prealloc, file_size); - if (fd && do_write) - { - // make a note that we just created a file - tor->session->add_file_created(); - } - } - - if (!fd) // couldn't create/open it either - { - auto const errnum = errno; - error->set( - errnum, - fmt::format( - _("Couldn't get '{path}': {error} ({error_code})"), - fmt::arg("path", filename), - fmt::arg("error", tr_strerror(errnum)), - fmt::arg("error_code", errnum))); - tr_logAddErrorTor(tor, std::string{ error->message() }); return; } + auto fmtstr = ""sv; switch (io_mode) { - case IoMode::Read: - if (!readEntireBuf(*fd, file_offset, buf, buflen, error) && *error) - { - tr_logAddErrorTor( - tor, - fmt::format( - _("Couldn't read '{path}': {error} ({error_code})"), - fmt::arg("path", tor->file_subpath(file_index)), - fmt::arg("error", error->message()), - fmt::arg("error_code", error->code()))); - return; - } - break; - - case IoMode::Write: - if (!writeEntireBuf(*fd, file_offset, buf, buflen, error) && *error) - { - tr_logAddErrorTor( - tor, - fmt::format( - _("Couldn't save '{path}': {error} ({error_code})"), - fmt::arg("path", tor->file_subpath(file_index)), - fmt::arg("error", error->message()), - fmt::arg("error_code", error->code()))); - return; - } - break; - case IoMode::Prefetch: tr_sys_file_advise(*fd, file_offset, buflen, TR_SYS_FILE_ADVICE_WILL_NEED); break; + + case IoMode::Write: + fmtstr = _("Couldn't save '{path}': {error} ({error_code})"); + write_entire_buf(*fd, file_offset, buf, buflen, error); + break; + + case IoMode::Read: + fmtstr = _("Couldn't read '{path}': {error} ({error_code})"); + read_entire_buf(*fd, file_offset, buf, buflen, error); + break; + } + + if (error) + { + tr_logAddErrorTor( + &tor, + fmt::format( + fmt::runtime(fmtstr), + fmt::arg("path", tor.file_subpath(file_index)), + fmt::arg("error", error.message()), + fmt::arg("error_code", error.code()))); } } -/* returns 0 on success, or an errno on failure */ -int readOrWritePiece(tr_torrent* tor, IoMode io_mode, tr_block_info::Location loc, uint8_t* buf, size_t buflen) +void read_or_write_piece( + tr_torrent const& tor, + IoMode const io_mode, + tr_block_info::Location const loc, + uint8_t* buf, + uint64_t buflen, + tr_error& error) { - if (loc.piece >= tor->piece_count()) + if (loc.piece >= tor.piece_count()) { - return EINVAL; + error.set_from_errno(EINVAL); + return; } - auto [file_index, file_offset] = tor->file_offset(loc); - - while (buflen != 0U) + auto [file_index, file_offset] = tor.file_offset(loc); + auto& session = *tor.session; + auto& open_files = session.openFiles(); + while (buflen != 0U && !error) { - uint64_t const bytes_this_pass = std::min(uint64_t{ buflen }, uint64_t{ tor->file_size(file_index) - file_offset }); - - auto error = tr_error{}; - readOrWriteBytes(tor->session, tor, io_mode, file_index, file_offset, buf, bytes_this_pass, &error); - if (error) // if IO failed, set torrent's error if not already set - { - if (io_mode == IoMode::Write && tor->error().error_type() != TR_STAT_LOCAL_ERROR) - { - tor->error().set_local_error(error.message()); - tr_torrentStop(tor); - } - - return error.code(); - } - + auto const bytes_this_pass = std::min(buflen, tor.file_size(file_index) - file_offset); + read_or_write_bytes(session, open_files, tor, io_mode, file_index, file_offset, buf, bytes_this_pass, error); if (buf != nullptr) { buf += bytes_this_pass; } buflen -= bytes_this_pass; - ++file_index; file_offset = 0U; } - - return 0; } -std::optional recalculateHash(tr_torrent* tor, tr_piece_index_t piece) +std::optional recalculate_hash(tr_torrent const& tor, tr_piece_index_t const piece) { - TR_ASSERT(tor != nullptr); - TR_ASSERT(piece < tor->piece_count()); + TR_ASSERT(piece < tor.piece_count()); auto sha = tr_sha1::create(); auto buffer = std::array{}; - auto& cache = tor->session->cache; - auto const [begin_byte, end_byte] = tor->block_info().byte_span_for_piece(piece); - auto const [begin_block, end_block] = tor->block_span_for_piece(piece); + auto& cache = tor.session->cache; + auto const [begin_byte, end_byte] = tor.block_info().byte_span_for_piece(piece); + auto const [begin_block, end_block] = tor.block_span_for_piece(piece); [[maybe_unused]] auto n_bytes_checked = size_t{}; for (auto block = begin_block; block < end_block; ++block) { - auto const block_loc = tor->block_loc(block); - auto const block_len = tor->block_size(block); + auto const block_loc = tor.block_loc(block); + auto const block_len = tor.block_size(block); if (auto const success = cache->read_block(tor, block_loc, block_len, std::data(buffer)) == 0; !success) { return {}; @@ -285,29 +255,43 @@ std::optional recalculateHash(tr_torrent* tor, tr_piece_index_ n_bytes_checked += (end - begin); } - TR_ASSERT(tor->piece_size(piece) == n_bytes_checked); + TR_ASSERT(tor.piece_size(piece) == n_bytes_checked); return sha->finish(); } } // namespace -int tr_ioRead(tr_torrent* tor, tr_block_info::Location const& loc, size_t len, uint8_t* setme) +int tr_ioRead(tr_torrent const& tor, tr_block_info::Location const& loc, size_t const len, uint8_t* const setme) { - return readOrWritePiece(tor, IoMode::Read, loc, setme, len); + auto error = tr_error{}; + read_or_write_piece(tor, IoMode::Read, loc, setme, len, error); + return error.code(); } -int tr_ioPrefetch(tr_torrent* tor, tr_block_info::Location const& loc, size_t len) +int tr_ioPrefetch(tr_torrent const& tor, tr_block_info::Location const& loc, size_t const len) { - return readOrWritePiece(tor, IoMode::Prefetch, loc, nullptr, len); + auto error = tr_error{}; + read_or_write_piece(tor, IoMode::Prefetch, loc, nullptr, len, error); + return error.code(); } -int tr_ioWrite(tr_torrent* tor, tr_block_info::Location const& loc, size_t len, uint8_t const* writeme) +int tr_ioWrite(tr_torrent& tor, tr_block_info::Location const& loc, size_t const len, uint8_t const* const writeme) { - return readOrWritePiece(tor, IoMode::Write, loc, const_cast(writeme), len); + auto error = tr_error{}; + read_or_write_piece(tor, IoMode::Write, loc, const_cast(writeme), len, error); + + // if IO failed, set torrent's error if not already set + if (error && tor.error().error_type() != TR_STAT_LOCAL_ERROR) + { + tor.error().set_local_error(error.message()); + tr_torrentStop(&tor); + } + + return error.code(); } -bool tr_ioTestPiece(tr_torrent* tor, tr_piece_index_t piece) +bool tr_ioTestPiece(tr_torrent const& tor, tr_piece_index_t const piece) { - auto const hash = recalculateHash(tor, piece); - return hash && *hash == tor->piece_hash(piece); + auto const hash = recalculate_hash(tor, piece); + return hash && *hash == tor.piece_hash(piece); } diff --git a/libtransmission/inout.h b/libtransmission/inout.h index fd7e93994..3febd4708 100644 --- a/libtransmission/inout.h +++ b/libtransmission/inout.h @@ -27,19 +27,19 @@ struct tr_torrent; * Reads the block specified by the piece index, offset, and length. * @return 0 on success, or an errno value on failure. */ -[[nodiscard]] int tr_ioRead(struct tr_torrent* tor, tr_block_info::Location const& loc, size_t len, uint8_t* setme); +[[nodiscard]] int tr_ioRead(tr_torrent const& tor, tr_block_info::Location const& loc, size_t len, uint8_t* setme); -int tr_ioPrefetch(tr_torrent* tor, tr_block_info::Location const& loc, size_t len); +int tr_ioPrefetch(tr_torrent const& tor, tr_block_info::Location const& loc, size_t len); /** * Writes the block specified by the piece index, offset, and length. * @return 0 on success, or an errno value on failure. */ -[[nodiscard]] int tr_ioWrite(struct tr_torrent* tor, tr_block_info::Location const& loc, size_t len, uint8_t const* writeme); +[[nodiscard]] int tr_ioWrite(tr_torrent& tor, tr_block_info::Location const& loc, size_t len, uint8_t const* writeme); /** * @brief Test to see if the piece matches its metainfo's SHA1 checksum. */ -bool tr_ioTestPiece(tr_torrent* tor, tr_piece_index_t piece); +[[nodiscard]] bool tr_ioTestPiece(tr_torrent const& tor, tr_piece_index_t piece); /* @} */ diff --git a/libtransmission/open-files.cc b/libtransmission/open-files.cc index 53a60428f..e6ec70fb4 100644 --- a/libtransmission/open-files.cc +++ b/libtransmission/open-files.cc @@ -142,7 +142,7 @@ std::optional tr_open_files::get( tr_file_index_t file_num, bool writable, std::string_view filename_in, - tr_preallocation_mode allocation, + Preallocation allocation, uint64_t file_size) { // is there already an entry @@ -196,17 +196,17 @@ std::optional tr_open_files::get( return {}; } - if (writable && !already_existed && allocation != TR_PREALLOCATE_NONE) + if (writable && !already_existed && allocation != Preallocation::None) { bool success = false; char const* type = nullptr; - if (allocation == TR_PREALLOCATE_FULL) + if (allocation == Preallocation::Full) { success = preallocate_file_full(fd, file_size, &error); type = "full"; } - else if (allocation == TR_PREALLOCATE_SPARSE) + else if (allocation == Preallocation::Sparse) { success = preallocate_file_sparse(fd, file_size, &error); type = "sparse"; diff --git a/libtransmission/open-files.h b/libtransmission/open-files.h index 9680561ee..df6e9e1f1 100644 --- a/libtransmission/open-files.h +++ b/libtransmission/open-files.h @@ -24,6 +24,13 @@ class tr_open_files { public: + enum class Preallocation + { + None, + Sparse, + Full + }; + [[nodiscard]] std::optional get(tr_torrent_id_t tor_id, tr_file_index_t file_num, bool writable); [[nodiscard]] std::optional get( @@ -31,7 +38,7 @@ public: tr_file_index_t file_num, bool writable, std::string_view filename, - tr_preallocation_mode allocation, + Preallocation allocation, uint64_t file_size); void close_all(); diff --git a/libtransmission/peer-msgs.cc b/libtransmission/peer-msgs.cc index c21dd7134..502bc0201 100644 --- a/libtransmission/peer-msgs.cc +++ b/libtransmission/peer-msgs.cc @@ -1270,7 +1270,7 @@ void prefetchPieces(tr_peerMsgsImpl* msgs) { if (auto& req = requests[i]; !req.prefetched) { - msgs->session->cache->prefetch_block(msgs->torrent, msgs->torrent->piece_loc(req.index, req.offset), req.length); + msgs->session->cache->prefetch_block(*msgs->torrent, msgs->torrent->piece_loc(req.index, req.offset), req.length); req.prefetched = true; } } @@ -1876,7 +1876,7 @@ namespace peer_pulse_helpers if (ok) { ok = msgs->session->cache - ->read_block(msgs->torrent, msgs->torrent->piece_loc(req.index, req.offset), req.length, std::data(buf)) == 0; + ->read_block(*msgs->torrent, msgs->torrent->piece_loc(req.index, req.offset), req.length, std::data(buf)) == 0; } if (ok) diff --git a/libtransmission/session-settings.h b/libtransmission/session-settings.h index f3ccc2b9f..f9da28358 100644 --- a/libtransmission/session-settings.h +++ b/libtransmission/session-settings.h @@ -12,6 +12,7 @@ #include "libtransmission/log.h" // for tr_log_level #include "libtransmission/net.h" // for tr_port, tr_tos_t +#include "libtransmission/open-files.h" // for tr_open_files::Preallocation #include "libtransmission/peer-io.h" // tr_preferred_transport #include "libtransmission/quark.h" @@ -51,7 +52,7 @@ struct tr_variant; V(TR_KEY_peer_socket_tos, peer_socket_tos, tr_tos_t, 0x04, "") \ V(TR_KEY_pex_enabled, pex_enabled, bool, true, "") \ V(TR_KEY_port_forwarding_enabled, port_forwarding_enabled, bool, true, "") \ - V(TR_KEY_preallocation, preallocation_mode, tr_preallocation_mode, TR_PREALLOCATE_SPARSE, "") \ + V(TR_KEY_preallocation, preallocation_mode, tr_open_files::Preallocation, tr_open_files::Preallocation::Sparse, "") \ V(TR_KEY_prefetch_enabled, is_prefetch_enabled, bool, true, "") \ V(TR_KEY_queue_stalled_enabled, queue_stalled_enabled, bool, true, "") \ V(TR_KEY_queue_stalled_minutes, queue_stalled_minutes, size_t, 30U, "") \ diff --git a/libtransmission/session.cc b/libtransmission/session.cc index c9b9708a1..94dfd6544 100644 --- a/libtransmission/session.cc +++ b/libtransmission/session.cc @@ -2009,16 +2009,16 @@ void tr_session::verify_add(tr_torrent* const tor) // --- -void tr_session::closeTorrentFiles(tr_torrent* tor) noexcept +void tr_session::close_torrent_files(tr_torrent_id_t const tor_id) noexcept { - this->cache->flush_torrent(tor); - openFiles().close_torrent(tor->id()); + this->cache->flush_torrent(tor_id); + openFiles().close_torrent(tor_id); } -void tr_session::closeTorrentFile(tr_torrent* tor, tr_file_index_t file_num) noexcept +void tr_session::close_torrent_file(tr_torrent const& tor, tr_file_index_t file_num) noexcept { this->cache->flush_file(tor, file_num); - openFiles().close_file(tor->id(), file_num); + openFiles().close_file(tor.id(), file_num); } // --- diff --git a/libtransmission/session.h b/libtransmission/session.h index b80716aa9..c6f720237 100644 --- a/libtransmission/session.h +++ b/libtransmission/session.h @@ -557,8 +557,8 @@ public: return open_files_; } - void closeTorrentFiles(tr_torrent* tor) noexcept; - void closeTorrentFile(tr_torrent* tor, tr_file_index_t file_num) noexcept; + void close_torrent_files(tr_torrent_id_t tor_id) noexcept; + void close_torrent_file(tr_torrent const& tor, tr_file_index_t file_num) noexcept; // announce ip diff --git a/libtransmission/torrent.cc b/libtransmission/torrent.cc index 0fdfe2805..764736c95 100644 --- a/libtransmission/torrent.cc +++ b/libtransmission/torrent.cc @@ -609,7 +609,7 @@ void removeTorrentInSessionThread(tr_torrent* tor, bool delete_flag, tr_fileFunc if (delete_flag && tor->has_metainfo()) { // ensure the files are all closed and idle before moving - tor->session->closeTorrentFiles(tor); + tor->session->close_torrent_files(tor->id()); tor->session->verify_remove(tor); if (delete_func == nullptr) @@ -764,7 +764,7 @@ void tr_torrent::stop_now() stopped_.emit(this); session->announcer_->stopTorrent(this); - session->closeTorrentFiles(this); + session->close_torrent_files(id()); if (!is_deleting_) { @@ -1109,7 +1109,7 @@ void tr_torrent::set_location_in_session_thread(std::string_view const path, boo } // ensure the files are all closed and idle before moving - session->closeTorrentFiles(this); + session->close_torrent_files(id()); session->verify_remove(this); auto error = tr_error{}; @@ -1771,7 +1771,7 @@ void tr_torrent::recheck_completeness() } completeness_ = new_completeness; - session->closeTorrentFiles(this); + session->close_torrent_files(id()); if (is_done()) { @@ -1943,10 +1943,9 @@ tr_block_span_t tr_torrent::block_span_for_file(tr_file_index_t const file) cons // --- -// TODO: should be const after tr_ioTestPiece() is const -bool tr_torrent::check_piece(tr_piece_index_t piece) +bool tr_torrent::check_piece(tr_piece_index_t const piece) const { - bool const pass = tr_ioTestPiece(this, piece); + auto const pass = tr_ioTestPiece(*this, piece); tr_logAddTraceTor(this, fmt::format("[LAZY] tr_torrent.checkPiece tested piece {}, pass=={}", piece, pass)); return pass; } @@ -2133,7 +2132,7 @@ std::string_view tr_torrent::primary_mime_type() const void tr_torrent::on_file_completed(tr_file_index_t const file) { /* close the file so that we can reopen in read-only mode as needed */ - session->closeTorrentFile(this, file); + session->close_torrent_file(*this, file); /* now that the file is complete and closed, we can start watching its * mtime timestamp for changes to know if we need to reverify pieces */ diff --git a/libtransmission/torrent.h b/libtransmission/torrent.h index 4a3b7e837..366b2a4d0 100644 --- a/libtransmission/torrent.h +++ b/libtransmission/torrent.h @@ -1098,7 +1098,7 @@ private: return checked_pieces_.test(piece); } - [[nodiscard]] bool check_piece(tr_piece_index_t piece); + [[nodiscard]] bool check_piece(tr_piece_index_t piece) const; [[nodiscard]] constexpr std::optional effective_idle_limit_minutes() const noexcept { diff --git a/libtransmission/transmission.h b/libtransmission/transmission.h index d13843551..bd0adf45c 100644 --- a/libtransmission/transmission.h +++ b/libtransmission/transmission.h @@ -67,13 +67,6 @@ enum tr_verify_added_mode TR_VERIFY_ADDED_FULL = 1 }; -enum tr_preallocation_mode -{ - TR_PREALLOCATE_NONE = 0, - TR_PREALLOCATE_SPARSE = 1, - TR_PREALLOCATE_FULL = 2 -}; - enum tr_encryption_mode { TR_CLEAR_PREFERRED, diff --git a/libtransmission/variant-converters.cc b/libtransmission/variant-converters.cc index ced49fe72..fc73a6106 100644 --- a/libtransmission/variant-converters.cc +++ b/libtransmission/variant-converters.cc @@ -17,6 +17,7 @@ #include "libtransmission/log.h" // for tr_log_level #include "libtransmission/net.h" // for tr_port +#include "libtransmission/open-files.h" // for tr_open_files::Preallocation #include "libtransmission/peer-io.h" // tr_preferred_transport #include "libtransmission/utils.h" // for tr_strv_strip(), tr_strlower() #include "libtransmission/variant.h" @@ -41,12 +42,12 @@ auto constexpr LogKeys = std::array, 7 { "warn", TR_LOG_WARN }, } }; -auto constexpr PreallocationKeys = std::array, 5>{ { - { "off", TR_PREALLOCATE_NONE }, - { "none", TR_PREALLOCATE_NONE }, - { "fast", TR_PREALLOCATE_SPARSE }, - { "sparse", TR_PREALLOCATE_SPARSE }, - { "full", TR_PREALLOCATE_FULL }, +auto constexpr PreallocationKeys = std::array, 5>{ { + { "off", tr_open_files::Preallocation::None }, + { "none", tr_open_files::Preallocation::None }, + { "fast", tr_open_files::Preallocation::Sparse }, + { "sparse", tr_open_files::Preallocation::Sparse }, + { "full", tr_open_files::Preallocation::Full }, } }; auto constexpr VerifyModeKeys = std::array, 2>{ { @@ -228,7 +229,7 @@ tr_variant VariantConverter::save(tr_port const& val) // --- template<> -std::optional VariantConverter::load(tr_variant const& src) +std::optional VariantConverter::load(tr_variant const& src) { static constexpr auto Keys = PreallocationKeys; @@ -249,7 +250,7 @@ std::optional VariantConverter::load(*val)) { return value; } @@ -260,9 +261,9 @@ std::optional VariantConverter::load -tr_variant VariantConverter::save(tr_preallocation_mode const& val) +tr_variant VariantConverter::save(tr_open_files::Preallocation const& val) { - return int64_t{ val }; + return static_cast(val); } // --- diff --git a/tests/libtransmission/open-files-test.cc b/tests/libtransmission/open-files-test.cc index 434e2ddf7..be94973da 100644 --- a/tests/libtransmission/open-files-test.cc +++ b/tests/libtransmission/open-files-test.cc @@ -26,6 +26,8 @@ using namespace std::literals; using OpenFilesTest = libtransmission::test::SessionTest; +static auto constexpr PreallocateFull = tr_open_files::Preallocation::Full; + TEST_F(OpenFilesTest, getCachedFailsIfNotCached) { auto const fd = session_->openFiles().get(0, 0, false); @@ -42,7 +44,7 @@ TEST_F(OpenFilesTest, getOpensIfNotCached) EXPECT_FALSE(session_->openFiles().get(0, 0, false)); // confirm that we can cache the file - auto fd = session_->openFiles().get(0, 0, false, filename, TR_PREALLOCATE_FULL, std::size(Contents)); + auto fd = session_->openFiles().get(0, 0, false, filename, PreallocateFull, std::size(Contents)); EXPECT_TRUE(fd.has_value()); assert(fd.has_value()); EXPECT_NE(TR_BAD_SYS_FILE, *fd); @@ -62,7 +64,7 @@ TEST_F(OpenFilesTest, getCacheSucceedsIfCached) createFileWithContents(filename, Contents); EXPECT_FALSE(session_->openFiles().get(0, 0, false)); - EXPECT_TRUE(session_->openFiles().get(0, 0, false, filename, TR_PREALLOCATE_FULL, std::size(Contents))); + EXPECT_TRUE(session_->openFiles().get(0, 0, false, filename, PreallocateFull, std::size(Contents))); EXPECT_TRUE(session_->openFiles().get(0, 0, false)); } @@ -73,7 +75,7 @@ TEST_F(OpenFilesTest, getCachedReturnsTheSameFd) createFileWithContents(filename, Contents); EXPECT_FALSE(session_->openFiles().get(0, 0, false)); - auto const fd1 = session_->openFiles().get(0, 0, false, filename, TR_PREALLOCATE_FULL, std::size(Contents)); + auto const fd1 = session_->openFiles().get(0, 0, false, filename, PreallocateFull, std::size(Contents)); auto const fd2 = session_->openFiles().get(0, 0, false); EXPECT_TRUE(fd1.has_value()); EXPECT_TRUE(fd2.has_value()); @@ -90,7 +92,7 @@ TEST_F(OpenFilesTest, getCachedFailsIfWrongPermissions) // cache it in ro mode EXPECT_FALSE(session_->openFiles().get(0, 0, false)); - EXPECT_TRUE(session_->openFiles().get(0, 0, false, filename, TR_PREALLOCATE_FULL, std::size(Contents))); + EXPECT_TRUE(session_->openFiles().get(0, 0, false, filename, PreallocateFull, std::size(Contents))); // now try to get it in r/w mode EXPECT_TRUE(session_->openFiles().get(0, 0, false)); @@ -104,7 +106,7 @@ TEST_F(OpenFilesTest, opensInReadOnlyUnlessWritableIsRequested) createFileWithContents(filename, Contents); // cache a file read-only mode - auto fd = session_->openFiles().get(0, 0, false, filename, TR_PREALLOCATE_FULL, std::size(Contents)); + auto fd = session_->openFiles().get(0, 0, false, filename, PreallocateFull, std::size(Contents)); EXPECT_TRUE(fd.has_value()); assert(fd.has_value()); @@ -124,7 +126,7 @@ TEST_F(OpenFilesTest, createsMissingFileIfWriteRequested) EXPECT_FALSE(fd); EXPECT_FALSE(tr_sys_path_exists(filename)); - fd = session_->openFiles().get(0, 0, true, filename, TR_PREALLOCATE_FULL, std::size(Contents)); + fd = session_->openFiles().get(0, 0, true, filename, PreallocateFull, std::size(Contents)); EXPECT_TRUE(fd.has_value()); assert(fd.has_value()); EXPECT_NE(TR_BAD_SYS_FILE, *fd); @@ -138,7 +140,7 @@ TEST_F(OpenFilesTest, closeFileClosesTheFile) createFileWithContents(filename, Contents); // cache a file read-only mode - EXPECT_TRUE(session_->openFiles().get(0, 0, false, filename, TR_PREALLOCATE_FULL, std::size(Contents))); + EXPECT_TRUE(session_->openFiles().get(0, 0, false, filename, PreallocateFull, std::size(Contents))); EXPECT_TRUE(session_->openFiles().get(0, 0, false)); // close the file @@ -155,11 +157,11 @@ TEST_F(OpenFilesTest, closeTorrentClosesTheTorrentFiles) auto filename = tr_pathbuf{ sandboxDir(), "/a.txt" }; createFileWithContents(filename, Contents); - EXPECT_TRUE(session_->openFiles().get(TorId, 1, false, filename, TR_PREALLOCATE_FULL, std::size(Contents))); + EXPECT_TRUE(session_->openFiles().get(TorId, 1, false, filename, PreallocateFull, std::size(Contents))); filename.assign(sandboxDir(), "/b.txt"); createFileWithContents(filename, Contents); - EXPECT_TRUE(session_->openFiles().get(TorId, 3, false, filename, TR_PREALLOCATE_FULL, std::size(Contents))); + EXPECT_TRUE(session_->openFiles().get(TorId, 3, false, filename, PreallocateFull, std::size(Contents))); // confirm that closing a different torrent does not affect these files session_->openFiles().close_torrent(TorId + 1); @@ -184,7 +186,7 @@ TEST_F(OpenFilesTest, closesLeastRecentlyUsedFile) for (int i = 0; i < LargerThanCacheLimit; ++i) { auto filename = tr_pathbuf{ sandboxDir(), fmt::format("/file-{:d}.txt"sv, i) }; - EXPECT_TRUE(session_->openFiles().get(TorId, i, true, filename, TR_PREALLOCATE_FULL, std::size(Contents))); + EXPECT_TRUE(session_->openFiles().get(TorId, i, true, filename, PreallocateFull, std::size(Contents))); } // Do a lookup-only for the files again *in the same order*. By following the diff --git a/tests/libtransmission/settings-test.cc b/tests/libtransmission/settings-test.cc index b5282edee..665b2c741 100644 --- a/tests/libtransmission/settings-test.cc +++ b/tests/libtransmission/settings-test.cc @@ -12,6 +12,7 @@ #include #include +#include #include #include #include @@ -246,12 +247,12 @@ TEST_F(SettingsTest, canLoadPreallocation) auto settings = std::make_unique(); auto const default_value = settings->preallocation_mode; - auto constexpr ExpectedValue = TR_PREALLOCATE_FULL; + auto constexpr ExpectedValue = tr_open_files::Preallocation::Full; ASSERT_NE(ExpectedValue, default_value); auto var = tr_variant{}; tr_variantInitDict(&var, 1); - tr_variantDictAddInt(&var, Key, ExpectedValue); + tr_variantDictAddInt(&var, Key, static_cast(ExpectedValue)); settings->load(var); EXPECT_EQ(ExpectedValue, settings->preallocation_mode); var.clear(); @@ -269,14 +270,14 @@ TEST_F(SettingsTest, canSavePreallocation) auto settings = tr_session_settings{}; auto const default_value = settings.preallocation_mode; - auto constexpr ExpectedValue = TR_PREALLOCATE_FULL; + auto constexpr ExpectedValue = tr_open_files::Preallocation::Full; ASSERT_NE(ExpectedValue, default_value); settings.preallocation_mode = ExpectedValue; auto var = settings.settings(); auto val = int64_t{}; EXPECT_TRUE(tr_variantDictFindInt(&var, Key, &val)); - EXPECT_EQ(ExpectedValue, val); + EXPECT_EQ(static_cast(ExpectedValue), val); } TEST_F(SettingsTest, canLoadSizeT)