From 5789b02e1b93c0789beea30c588015e9c1cf8abb Mon Sep 17 00:00:00 2001 From: Alden Torres Date: Thu, 18 Aug 2016 11:57:50 -0400 Subject: [PATCH] storage_interface refactor to use span and references (#1007) storage_interface refactor to use span and references --- ChangeLog | 1 + examples/connection_tester.cpp | 2 +- include/libtorrent/storage.hpp | 20 +++++++-------- src/disk_io_thread.cpp | 23 +++++++++-------- src/storage.cpp | 46 +++++++++++++++++----------------- test/make_torrent.cpp | 2 +- test/test_block_cache.cpp | 12 ++++----- test/test_storage.cpp | 26 +++++++++---------- test/test_transfer.cpp | 8 +++--- 9 files changed, 70 insertions(+), 70 deletions(-) diff --git a/ChangeLog b/ChangeLog index da2cf9e6e..dc9fa25d1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,4 @@ + * storage_interface API changed to use span and references * changes in public API to work with std::shared_ptr * extensions API changed to use span and std::shared_ptr * plugin API changed to handle DHT requests using string_view diff --git a/examples/connection_tester.cpp b/examples/connection_tester.cpp index 9b028765e..250272020 100644 --- a/examples/connection_tester.cpp +++ b/examples/connection_tester.cpp @@ -846,7 +846,7 @@ void generate_data(char const* path, torrent_info const& ti) generate_block(piece, i, j, 0x4000); file::iovec_t b = { piece, 0x4000}; storage_error error; - st->writev(&b, 1, i, j, 0, error); + st->writev(b, i, j, 0, error); if (error) std::fprintf(stderr, "storage error: %s\n", error.ec.message().c_str()); } diff --git a/include/libtorrent/storage.hpp b/include/libtorrent/storage.hpp index 840cc4425..301c0fb70 100644 --- a/include/libtorrent/storage.hpp +++ b/include/libtorrent/storage.hpp @@ -45,7 +45,6 @@ POSSIBILITY OF SUCH DAMAGE. #include #include #include -#include #include #include @@ -64,6 +63,7 @@ POSSIBILITY OF SUCH DAMAGE. #include "libtorrent/bdecode.hpp" #include "libtorrent/bitfield.hpp" #include "libtorrent/performance_counters.hpp" +#include "libtorrent/span.hpp" // OVERVIEW // @@ -246,9 +246,9 @@ namespace libtorrent // The number of bytes read or written should be returned, or -1 on // error. If there's an error, the ``storage_error`` must be filled out // to represent the error that occurred. - virtual int readv(file::iovec_t const* bufs, int num_bufs + virtual int readv(span bufs , int piece, int offset, int flags, storage_error& ec) = 0; - virtual int writev(file::iovec_t const* bufs, int num_bufs + virtual int writev(span bufs , int piece, int offset, int flags, storage_error& ec) = 0; // This function is called when first checking (or re-checking) the @@ -294,13 +294,13 @@ namespace libtorrent // on disk. If the resume data seems to be up-to-date, return true. If // not, set ``error`` to a description of what mismatched and return false. // - // If the ``links`` pointer is non-nullptr, it has the same number + // If the ``links`` pointer is non-empty, it has the same number // of elements as there are files. Each element is either empty or contains // the absolute path to a file identical to the corresponding file in this // torrent. The storage must create hard links (or copy) those files. If // any file does not exist or is inaccessible, the disk job must fail. virtual bool verify_resume_data(add_torrent_params const& rd - , std::vector const* links + , std::vector const& links , storage_error& ec) = 0; // This function should release all the file handles that it keeps open @@ -412,18 +412,18 @@ namespace libtorrent virtual int move_storage(std::string const& save_path, int flags , storage_error& ec) override; virtual bool verify_resume_data(add_torrent_params const& rd - , std::vector const* links + , std::vector const& links , storage_error& error) override; virtual bool tick() override; - int readv(file::iovec_t const* bufs, int num_bufs + int readv(span bufs , int piece, int offset, int flags, storage_error& ec) override; - int writev(file::iovec_t const* bufs, int num_bufs + int writev(span bufs , int piece, int offset, int flags, storage_error& ec) override; // if the files in this storage are mapped, returns the mapped // file_storage, otherwise returns the original file_storage object. - file_storage const& files() const { return m_mapped_files?*m_mapped_files:m_files; } + file_storage const& files() const { return m_mapped_files ? *m_mapped_files : m_files; } #ifdef TORRENT_DISK_STATS static bool disk_write_access_log(); @@ -593,7 +593,7 @@ namespace libtorrent // if 'fatal_disk_error' is returned, the error message indicates what // when wrong in the disk access int check_fastresume(add_torrent_params const& rd - , std::vector const* links + , std::vector const& links , storage_error& error); // helper functions for check_fastresume diff --git a/src/disk_io_thread.cpp b/src/disk_io_thread.cpp index f3775fb4d..6c95859e9 100644 --- a/src/disk_io_thread.cpp +++ b/src/disk_io_thread.cpp @@ -616,9 +616,9 @@ namespace libtorrent bool failed = false; for (int i = 1; i <= num_blocks; ++i) { - if (i < num_blocks && flushing[i] == flushing[i-1]+1) continue; - int ret = pe->storage->get_storage_impl()->writev(iov_start - , i - flushing_start + if (i < num_blocks && flushing[i] == flushing[i - 1] + 1) continue; + int ret = pe->storage->get_storage_impl()->writev( + {iov_start, size_t(i - flushing_start)} , piece + flushing[flushing_start] / blocks_in_piece , (flushing[flushing_start] % blocks_in_piece) * block_size , file_flags, error); @@ -1160,7 +1160,7 @@ namespace libtorrent , m_settings.get_bool(settings_pack::coalesce_reads)); file::iovec_t b = { j->buffer.disk_block, size_t(j->d.io.buffer_size) }; - int ret = j->storage->get_storage_impl()->readv(&b, 1 + int ret = j->storage->get_storage_impl()->readv(b , j->piece, j->d.io.offset, file_flags, j->error); TORRENT_ASSERT(ret >= 0 || j->error.ec); @@ -1235,7 +1235,7 @@ namespace libtorrent , m_settings.get_bool(settings_pack::coalesce_reads)); time_point start_time = clock_type::now(); - ret = j->storage->get_storage_impl()->readv(iov, iov_len + ret = j->storage->get_storage_impl()->readv({iov, size_t(iov_len)} , j->piece, adjusted_offset, file_flags, j->error); if (!j->error.ec) @@ -1396,7 +1396,7 @@ namespace libtorrent m_stats_counters.inc_stats_counter(counters::num_writing_threads, 1); // the actual write operation - int ret = j->storage->get_storage_impl()->writev(&b, 1 + int ret = j->storage->get_storage_impl()->writev(b , j->piece, j->d.io.offset, file_flags, j->error); m_stats_counters.inc_stats_counter(counters::num_writing_threads, -1); @@ -2205,7 +2205,7 @@ namespace libtorrent time_point start_time = clock_type::now(); iov.iov_len = (std::min)(block_size, piece_size - offset); - ret = j->storage->get_storage_impl()->readv(&iov, 1, j->piece + ret = j->storage->get_storage_impl()->readv(iov, j->piece , offset, file_flags, j->error); if (ret < 0) break; @@ -2394,7 +2394,7 @@ namespace libtorrent time_point start_time = clock_type::now(); TORRENT_PIECE_ASSERT(ph->offset == i * block_size, pe); - ret = j->storage->get_storage_impl()->readv(&iov, 1, j->piece + ret = j->storage->get_storage_impl()->readv(iov, j->piece , ph->offset, file_flags, j->error); if (ret < 0) @@ -2526,8 +2526,9 @@ namespace libtorrent add_torrent_params tmp; if (rd == nullptr) rd = &tmp; - std::unique_ptr > links(j->d.links); - return j->storage->check_fastresume(*rd, links.get(), j->error); + std::unique_ptr> links(j->d.links); + return j->storage->check_fastresume(*rd + , links ? *links : std::vector(), j->error); } int disk_io_thread::do_rename_file(disk_io_job* j, jobqueue_t& /* completed_jobs */ ) @@ -2631,7 +2632,7 @@ namespace libtorrent time_point start_time = clock_type::now(); - ret = j->storage->get_storage_impl()->readv(&iov, 1, j->piece + ret = j->storage->get_storage_impl()->readv(iov, j->piece , offset, file_flags, j->error); if (ret < 0) diff --git a/src/storage.cpp b/src/storage.cpp index 9a7b4e1ac..d97891d65 100644 --- a/src/storage.cpp +++ b/src/storage.cpp @@ -837,14 +837,15 @@ namespace libtorrent } bool default_storage::verify_resume_data(add_torrent_params const& rd - , std::vector const* links + , std::vector const& links , storage_error& ec) { file_storage const& fs = files(); #ifndef TORRENT_DISABLE_MUTABLE_TORRENTS - if (links) + if (!links.empty()) { + TORRENT_ASSERT(int(links.size()) == fs.num_files()); // if this is a mutable torrent, and we need to pick up some files // from other torrents, do that now. Note that there is an inherent // race condition here. We checked if the files existed on a different @@ -852,15 +853,14 @@ namespace libtorrent // moved. If so, we just fail. The user is responsible to not touch // other torrents until a new mutable torrent has been completely // added. - int idx = 0; - for (std::vector::const_iterator i = links->begin(); - i != links->end(); ++i, ++idx) + for (int idx = 0; idx < fs.num_files(); idx++) { - if (i->empty()) continue; + std::string const& s = links[idx]; + if (s.empty()) continue; error_code err; std::string file_path = fs.file_path(idx, m_save_path); - hard_link(*i, file_path, err); + hard_link(s, file_path, err); // if the file already exists, that's not an error // TODO: 2 is this risky? The upper layer will assume we have the @@ -1099,7 +1099,7 @@ namespace libtorrent return ret; } - int default_storage::readv(file::iovec_t const* bufs, int num_bufs + int default_storage::readv(span bufs , int piece, int offset, int flags, storage_error& ec) { read_fileop op(*this, flags); @@ -1108,14 +1108,14 @@ namespace libtorrent boost::thread::sleep(boost::get_system_time() + boost::posix_time::milliseconds(1000)); #endif - return readwritev(files(), bufs, piece, offset, num_bufs, op, ec); + return readwritev(files(), bufs.data(), piece, offset, int(bufs.size()), op, ec); } - int default_storage::writev(file::iovec_t const* bufs, int num_bufs + int default_storage::writev(span bufs , int piece, int offset, int flags, storage_error& ec) { write_fileop op(*this, flags); - return readwritev(files(), bufs, piece, offset, num_bufs, op, ec); + return readwritev(files(), bufs.data(), piece, offset, int(bufs.size()), op, ec); } // much of what needs to be done when reading and writing is buffer @@ -1374,19 +1374,19 @@ namespace libtorrent void initialize(storage_error&) override {} int move_storage(std::string const&, int, storage_error&) override { return 0; } - int readv(file::iovec_t const* bufs, int num_bufs + int readv(span bufs , int, int, int, storage_error&) override { - return bufs_size(bufs, num_bufs); + return bufs_size(bufs.data(), int(bufs.size())); } - int writev(file::iovec_t const* bufs, int num_bufs + int writev(span bufs , int, int, int, storage_error&) override { - return bufs_size(bufs, num_bufs); + return bufs_size(bufs.data(), int(bufs.size())); } bool verify_resume_data(add_torrent_params const& - , std::vector const* + , std::vector const& , storage_error&) override { return false; } }; } @@ -1407,22 +1407,22 @@ namespace libtorrent { void initialize(storage_error&) override {} - int readv(file::iovec_t const* bufs, int num_bufs + int readv(span bufs , int, int, int, storage_error&) override { int ret = 0; - for (int i = 0; i < num_bufs; ++i) + for (int i = 0; i < int(bufs.size()); ++i) { memset(bufs[i].iov_base, 0, bufs[i].iov_len); ret += int(bufs[i].iov_len); } return 0; } - int writev(file::iovec_t const* bufs, int num_bufs + int writev(span bufs , int, int, int, storage_error&) override { int ret = 0; - for (int i = 0; i < num_bufs; ++i) + for (int i = 0; i < int(bufs.size()); ++i) ret += int(bufs[i].iov_len); return 0; } @@ -1433,12 +1433,12 @@ namespace libtorrent int move_storage(std::string const& /* save_path */ , int /* flags */, storage_error&) override { return 0; } bool verify_resume_data(add_torrent_params const& /* rd */ - , std::vector const* /* links */ + , std::vector const& /* links */ , storage_error&) override { return false; } void release_files(storage_error&) override {} void rename_file(int /* index */ - , std::string const& /* new_filenamem */, storage_error&) override {} + , std::string const& /* new_filename */, storage_error&) override {} void delete_files(int, storage_error&) override {} }; } @@ -1548,7 +1548,7 @@ namespace libtorrent // any file does not exist or is inaccessible, the disk job must fail. int piece_manager::check_fastresume( add_torrent_params const& rd - , std::vector const* links + , std::vector const& links , storage_error& ec) { TORRENT_ASSERT(m_files.piece_length() > 0); diff --git a/test/make_torrent.cpp b/test/make_torrent.cpp index 96067295b..c4a461212 100644 --- a/test/make_torrent.cpp +++ b/test/make_torrent.cpp @@ -190,7 +190,7 @@ void generate_files(libtorrent::torrent_info const& ti, std::string const& path file::iovec_t b = { &buffer[0], size_t(piece_size) }; storage_error ec; - int ret = st.writev(&b, 1, i, 0, 0, ec); + int ret = st.writev(b, i, 0, 0, ec); if (ret != piece_size || ec) { std::fprintf(stderr, "ERROR writing files: (%d expected %d) %s\n" diff --git a/test/test_block_cache.cpp b/test/test_block_cache.cpp index d31ea88e0..09a9f8b00 100644 --- a/test/test_block_cache.cpp +++ b/test/test_block_cache.cpp @@ -49,15 +49,15 @@ struct test_storage_impl : storage_interface { void initialize(storage_error& ec) override {} - int readv(file::iovec_t const* bufs, int num_bufs + int readv(span bufs , int piece, int offset, int flags, storage_error& ec) override { - return bufs_size(bufs, num_bufs); + return bufs_size(bufs.data(), int(bufs.size())); } - int writev(file::iovec_t const* bufs, int num_bufs + int writev(span bufs , int piece, int offset, int flags, storage_error& ec) override { - return bufs_size(bufs, num_bufs); + return bufs_size(bufs.data(), int(bufs.size())); } bool has_any_file(storage_error& ec) override { return false; } @@ -66,10 +66,10 @@ struct test_storage_impl : storage_interface int move_storage(std::string const& save_path, int flags , storage_error& ec) override { return 0; } bool verify_resume_data(add_torrent_params const& rd - , std::vector const* links + , std::vector const& links , storage_error& ec) override { return true; } void release_files(storage_error& ec) override {} - void rename_file(int index, std::string const& new_filenamem + void rename_file(int index, std::string const& new_filename , storage_error& ec) override {} void delete_files(int, storage_error& ec) override {} #ifndef TORRENT_NO_DEPRECATE diff --git a/test/test_storage.cpp b/test/test_storage.cpp index cbbb9b248..7061fd71f 100644 --- a/test/test_storage.cpp +++ b/test/test_storage.cpp @@ -232,25 +232,25 @@ void run_storage_tests(std::shared_ptr info // write piece 1 (in slot 0) file::iovec_t iov = { piece1.data(), half}; - ret = s->writev(&iov, 1, 0, 0, 0, ec); + ret = s->writev(iov, 0, 0, 0, ec); if (ret != half) print_error("writev", ret, ec); iov.iov_base = piece1.data() + half; iov.iov_len = half; - ret = s->writev(&iov, 1, 0, half, 0, ec); + ret = s->writev(iov, 0, half, 0, ec); if (ret != half) print_error("writev", ret, ec); // test unaligned read (where the bytes are aligned) iov.iov_base = piece + 3; iov.iov_len = piece_size - 9; - ret = s->readv(&iov, 1, 0, 3, 0, ec); + ret = s->readv(iov, 0, 3, 0, ec); if (ret != piece_size - 9) print_error("readv",ret, ec); TEST_CHECK(std::equal(piece+3, piece + piece_size-9, piece1.data()+3)); // test unaligned read (where the bytes are not aligned) iov.iov_base = piece; iov.iov_len = piece_size - 9; - ret = s->readv(&iov, 1, 0, 3, 0, ec); + ret = s->readv(iov, 0, 3, 0, ec); TEST_CHECK(ret == piece_size - 9); if (ret != piece_size - 9) print_error("readv", ret, ec); TEST_CHECK(std::equal(piece, piece + piece_size-9, piece1.data()+3)); @@ -258,7 +258,7 @@ void run_storage_tests(std::shared_ptr info // verify piece 1 iov.iov_base = piece; iov.iov_len = piece_size; - ret = s->readv(&iov, 1, 0, 0, 0, ec); + ret = s->readv(iov, 0, 0, 0, ec); TEST_CHECK(ret == piece_size); if (ret != piece_size) print_error("readv", ret, ec); TEST_CHECK(std::equal(piece, piece + piece_size, piece1.data())); @@ -266,24 +266,24 @@ void run_storage_tests(std::shared_ptr info // do the same with piece 0 and 2 (in slot 1 and 2) iov.iov_base = piece0.data(); iov.iov_len = piece_size; - ret = s->writev(&iov, 1, 1, 0, 0, ec); + ret = s->writev(iov, 1, 0, 0, ec); if (ret != piece_size) print_error("writev", ret, ec); iov.iov_base = piece2.data(); iov.iov_len = piece_size; - ret = s->writev(&iov, 1, 2, 0, 0, ec); + ret = s->writev(iov, 2, 0, 0, ec); if (ret != piece_size) print_error("writev", ret, ec); // verify piece 0 and 2 iov.iov_base = piece; iov.iov_len = piece_size; - ret = s->readv(&iov, 1, 1, 0, 0, ec); + ret = s->readv(iov, 1, 0, 0, ec); if (ret != piece_size) print_error("readv", ret, ec); TEST_CHECK(std::equal(piece, piece + piece_size, piece0.data())); iov.iov_base = piece; iov.iov_len = piece_size; - ret = s->readv(&iov, 1, 2, 0, 0, ec); + ret = s->readv(iov, 2, 0, 0, ec); if (ret != piece_size) print_error("readv", ret, ec); TEST_CHECK(std::equal(piece, piece + piece_size, piece2.data())); @@ -335,7 +335,7 @@ void test_remove(std::string const& test_path, bool unbuffered) file::iovec_t b = {&buf[0], 4}; storage_error se; - s->writev(&b, 1, 2, 0, 0, se); + s->writev(b, 2, 0, 0, se); TEST_CHECK(exists(combine_path(test_path, combine_path("temp_storage" , combine_path("folder1", "test2.tmp"))))); @@ -346,7 +346,7 @@ void test_remove(std::string const& test_path, bool unbuffered) , combine_path("folder1", "test2.tmp"))), &st, ec); TEST_EQUAL(st.file_size, 8); - s->writev(&b, 1, 4, 0, 0, se); + s->writev(b, 4, 0, 0, se); TEST_CHECK(exists(combine_path(test_path, combine_path("temp_storage" , combine_path("_folder3", combine_path("subfolder", "test5.tmp")))))); @@ -1289,7 +1289,7 @@ TORRENT_TEST(move_storage_into_self) file::iovec_t const b = {&buf[0], 4}; storage_error se; - s->writev(&b, 1, 2, 0, 0, se); + s->writev(b, 2, 0, 0, se); std::string const test_path = combine_path(save_path, combine_path("temp_storage", "folder1")); s->move_storage(test_path, 0, se); @@ -1335,7 +1335,7 @@ TORRENT_TEST(dont_move_intermingled_files) file::iovec_t b = {&buf[0], 4}; storage_error se; - s->writev(&b, 1, 2, 0, 0, se); + s->writev(b, 2, 0, 0, se); error_code ec; create_directory(combine_path(save_path, combine_path("temp_storage" diff --git a/test/test_transfer.cpp b/test/test_transfer.cpp index f783139ff..19fa70f5e 100644 --- a/test/test_transfer.cpp +++ b/test/test_transfer.cpp @@ -87,8 +87,7 @@ struct test_storage : default_storage } int writev( - file::iovec_t const* bufs - , int num_bufs + span bufs , int piece_index , int offset , int flags @@ -104,10 +103,9 @@ struct test_storage : default_storage return 0; } - for (int i = 0; i < num_bufs; ++i) - m_written += int(bufs[i].iov_len); + for (auto const& b : bufs) m_written += int(b.iov_len); l.unlock(); - return default_storage::writev(bufs, num_bufs, piece_index, offset, flags, se); + return default_storage::writev(bufs, piece_index, offset, flags, se); } ~test_storage() override = default;