use std::function instead of the fileop interface for readwritev()

include/libtorrent/aux_/storage_utils.hpp

@@ -58,23 +58,16 @@ namespace libtorrent {
 		, int bytes, span<iovec_t> target);
 	TORRENT_EXTRA_EXPORT span<iovec_t> advance_bufs(span<iovec_t> bufs, int bytes);
 
-	// this identifies a read or write operation so that readwritev() knows
+	// this is a read or write operation so that readwritev() knows
 	// what to do when it's actually touching the file
-	struct fileop
+	using fileop = std::function<int(file_index_t, std::int64_t, span<iovec_t const>, storage_error&)>;
-	{
-		virtual int file_op(file_index_t const file_index, std::int64_t const file_offset
-			, span<iovec_t const> bufs, storage_error& ec) = 0;
-
-	protected:
-		~fileop() {}
-	};
 
 	// this function is responsible for turning read and write operations in the
 	// torrent space (pieces) into read and write operations in the filesystem
 	// space (files on disk).
 	TORRENT_EXTRA_EXPORT int readwritev(file_storage const& files
 		, span<iovec_t const> bufs, piece_index_t piece, int offset
-		, fileop& op, storage_error& ec);
+		, storage_error& ec, fileop op);
 
 	// moves the files in file_storage f from ``save_path`` to
 	// ``destination_save_path`` according to the rules defined by ``flags``.

src/disk_io_thread.cpp

@@ -2162,7 +2162,8 @@ namespace libtorrent {
 		std::uint32_t const file_flags = file_flags_for_job(j
 			, m_settings.get_bool(settings_pack::coalesce_reads));
 
-		iovec_t iov = { m_disk_cache.allocate_buffer("hashing"), 0x4000 };
+		iovec_t iov = { m_disk_cache.allocate_buffer("hashing")
+			, static_cast<std::size_t>(block_size) };
 		hasher h;
 		int ret = 0;
 		int offset = 0;

src/storage.cpp

@@ -83,164 +83,6 @@ namespace libtorrent {
 			std::memset(buf.data(), 0, buf.size());
 	}
 
-	struct write_fileop final : aux::fileop
-	{
-		write_fileop(default_storage& st, std::uint32_t const flags)
-			: m_storage(st)
-			, m_flags(flags)
-		{}
-
-		int file_op(file_index_t const file_index
-			, std::int64_t const file_offset
-			, span<iovec_t const> bufs, storage_error& ec)
-			final
-		{
-			if (m_storage.files().pad_file_at(file_index))
-			{
-				// writing to a pad-file is a no-op
-				return bufs_size(bufs);
-			}
-
-			if (file_index < m_storage.m_file_priority.end_index()
-				&& m_storage.m_file_priority[file_index] == 0)
-			{
-				m_storage.need_partfile();
-
-				error_code e;
-				peer_request map = m_storage.files().map_file(file_index
-					, file_offset, 0);
-				int ret = m_storage.m_part_file->writev(bufs
-					, map.piece, map.start, e);
-
-				if (e)
-				{
-					ec.ec = e;
-					ec.file(file_index);
-					ec.operation = storage_error::partfile_write;
-					return -1;
-				}
-				return ret;
-			}
-
-			// invalidate our stat cache for this file, since
-			// we're writing to it
-			m_storage.m_stat_cache.set_dirty(file_index);
-
-			file_handle handle = m_storage.open_file(file_index
-				, file::read_write, ec);
-			if (ec) return -1;
-
-			// please ignore the adjusted_offset. It's just file_offset.
-			std::int64_t adjusted_offset =
-#ifndef TORRENT_NO_DEPRECATE
-				m_storage.files().file_base_deprecated(file_index) +
-#endif
-				file_offset;
-
-			error_code e;
-			int const ret = int(handle->writev(adjusted_offset
-				, bufs, e, m_flags));
-
-			// set this unconditionally in case the upper layer would like to treat
-			// short reads as errors
-			ec.operation = storage_error::write;
-
-				// we either get an error or 0 or more bytes read
-			TORRENT_ASSERT(e || ret >= 0);
-			TORRENT_ASSERT(ret <= bufs_size(bufs));
-
-			if (e)
-			{
-				ec.ec = e;
-				ec.file(file_index);
-				return -1;
-			}
-
-			return ret;
-		}
-	private:
-		default_storage& m_storage;
-		std::uint32_t const m_flags;
-	};
-
-	struct read_fileop final : aux::fileop
-	{
-		read_fileop(default_storage& st, std::uint32_t const flags)
-			: m_storage(st)
-			, m_flags(flags)
-		{}
-
-		int file_op(file_index_t const file_index
-			, std::int64_t const file_offset
-			, span<iovec_t const> bufs, storage_error& ec)
-			final
-		{
-			if (m_storage.files().pad_file_at(file_index))
-			{
-				// reading from a pad file yields zeroes
-				clear_bufs(bufs);
-				return bufs_size(bufs);
-			}
-
-			if (file_index < m_storage.m_file_priority.end_index()
-				&& m_storage.m_file_priority[file_index] == 0)
-			{
-				m_storage.need_partfile();
-
-				error_code e;
-				peer_request map = m_storage.files().map_file(file_index
-					, file_offset, 0);
-				int ret = m_storage.m_part_file->readv(bufs
-					, map.piece, map.start, e);
-
-				if (e)
-				{
-					ec.ec = e;
-					ec.file(file_index);
-					ec.operation = storage_error::partfile_read;
-					return -1;
-				}
-				return ret;
-			}
-
-			file_handle handle = m_storage.open_file(file_index
-				, file::read_only | m_flags, ec);
-			if (ec) return -1;
-
-			// please ignore the adjusted_offset. It's just file_offset.
-			std::int64_t adjusted_offset =
-#ifndef TORRENT_NO_DEPRECATE
-				m_storage.files().file_base_deprecated(file_index) +
-#endif
-				file_offset;
-
-			error_code e;
-			int const ret = int(handle->readv(adjusted_offset
-				, bufs, e, m_flags));
-
-			// set this unconditionally in case the upper layer would like to treat
-			// short reads as errors
-			ec.operation = storage_error::read;
-
-				// we either get an error or 0 or more bytes read
-			TORRENT_ASSERT(e || ret >= 0);
-			TORRENT_ASSERT(ret <= bufs_size(bufs));
-
-			if (e)
-			{
-				ec.ec = e;
-				ec.file(file_index);
-				return -1;
-			}
-
-			return ret;
-		}
-
-	private:
-		default_storage& m_storage;
-		std::uint32_t const m_flags;
-	};
-
 	default_storage::default_storage(storage_params const& params
 		, file_pool& pool)
 		: storage_interface(*params.files)
@@ -622,22 +464,150 @@ namespace libtorrent {
 
 	int default_storage::readv(span<iovec_t const> bufs
 		, piece_index_t const piece, int const offset
-		, std::uint32_t const flags, storage_error& ec)
+		, std::uint32_t const flags, storage_error& error)
 	{
-		read_fileop op(*this, flags);
-
 #ifdef TORRENT_SIMULATE_SLOW_READ
 		std::this_thread::sleep_for(seconds(1));
 #endif
-		return readwritev(files(), bufs, piece, offset, op, ec);
+		return readwritev(files(), bufs, piece, offset, error
+			, [this, flags](file_index_t const file_index
+				, std::int64_t const file_offset
+				, span<iovec_t const> vec, storage_error& ec)
+		{
+			if (files().pad_file_at(file_index))
+			{
+				// reading from a pad file yields zeroes
+				clear_bufs(vec);
+				return bufs_size(vec);
+			}
+
+			if (file_index < m_file_priority.end_index()
+				&& m_file_priority[file_index] == 0)
+			{
+				need_partfile();
+
+				error_code e;
+				peer_request map = files().map_file(file_index
+					, file_offset, 0);
+				int const ret = m_part_file->readv(vec
+					, map.piece, map.start, e);
+
+				if (e)
+				{
+					ec.ec = e;
+					ec.file(file_index);
+					ec.operation = storage_error::partfile_read;
+					return -1;
+				}
+				return ret;
+			}
+
+			file_handle handle = open_file(file_index
+				, file::read_only | flags, ec);
+			if (ec) return -1;
+
+			// please ignore the adjusted_offset. It's just file_offset.
+			std::int64_t const adjusted_offset =
+#ifndef TORRENT_NO_DEPRECATE
+				files().file_base_deprecated(file_index) +
+#endif
+				file_offset;
+
+			error_code e;
+			int const ret = int(handle->readv(adjusted_offset
+				, vec, e, flags));
+
+			// set this unconditionally in case the upper layer would like to treat
+			// short reads as errors
+			ec.operation = storage_error::read;
+
+				// we either get an error or 0 or more bytes read
+			TORRENT_ASSERT(e || ret >= 0);
+			TORRENT_ASSERT(ret <= bufs_size(vec));
+
+			if (e)
+			{
+				ec.ec = e;
+				ec.file(file_index);
+				return -1;
+			}
+
+			return ret;
+		});
 	}
 
 	int default_storage::writev(span<iovec_t const> bufs
 		, piece_index_t const piece, int const offset
-		, std::uint32_t const flags, storage_error& ec)
+		, std::uint32_t const flags, storage_error& error)
 	{
-		write_fileop op(*this, flags);
+		return readwritev(files(), bufs, piece, offset, error
-		return readwritev(files(), bufs, piece, offset, op, ec);
+			, [this, flags](file_index_t const file_index
+				, std::int64_t const file_offset
+				, span<iovec_t const> vec, storage_error& ec)
+		{
+			if (files().pad_file_at(file_index))
+			{
+				// writing to a pad-file is a no-op
+				return bufs_size(vec);
+			}
+
+			if (file_index < m_file_priority.end_index()
+				&& m_file_priority[file_index] == 0)
+			{
+				need_partfile();
+
+				error_code e;
+				peer_request map = files().map_file(file_index
+					, file_offset, 0);
+				int const ret = m_part_file->writev(vec
+					, map.piece, map.start, e);
+
+				if (e)
+				{
+					ec.ec = e;
+					ec.file(file_index);
+					ec.operation = storage_error::partfile_write;
+					return -1;
+				}
+				return ret;
+			}
+
+			// invalidate our stat cache for this file, since
+			// we're writing to it
+			m_stat_cache.set_dirty(file_index);
+
+			file_handle handle = open_file(file_index
+				, file::read_write, ec);
+			if (ec) return -1;
+
+			// please ignore the adjusted_offset. It's just file_offset.
+			std::int64_t const adjusted_offset =
+#ifndef TORRENT_NO_DEPRECATE
+				files().file_base_deprecated(file_index) +
+#endif
+				file_offset;
+
+			error_code e;
+			int const ret = int(handle->writev(adjusted_offset
+				, vec, e, flags));
+
+			// set this unconditionally in case the upper layer would like to treat
+			// short reads as errors
+			ec.operation = storage_error::write;
+
+				// we either get an error or 0 or more bytes read
+			TORRENT_ASSERT(e || ret >= 0);
+			TORRENT_ASSERT(ret <= bufs_size(vec));
+
+			if (e)
+			{
+				ec.ec = e;
+				ec.file(file_index);
+				return -1;
+			}
+
+			return ret;
+		});
 	}
 
 	file_handle default_storage::open_file(file_index_t const file

src/storage_utils.cpp

@@ -104,8 +104,8 @@ namespace libtorrent { namespace aux {
 	// and writing. This function is a template, and the fileop decides what to
 	// do with the file and the buffers.
 	int readwritev(file_storage const& files, span<iovec_t const> const bufs
-		, piece_index_t const piece, const int offset, fileop& op
+		, piece_index_t const piece, const int offset
-		, storage_error& ec)
+		, storage_error& ec, fileop op)
 	{
 		TORRENT_ASSERT(piece >= piece_index_t(0));
 		TORRENT_ASSERT(piece < files.end_piece());
@@ -169,7 +169,7 @@ namespace libtorrent { namespace aux {
 			// file_bytes_left bytes, i.e. just this one operation
 			int tmp_bufs_used = copy_bufs(current_buf, file_bytes_left, tmp_buf);
 
-			int bytes_transferred = op.file_op(file_index, file_offset
+			int bytes_transferred = op(file_index, file_offset
 				, tmp_buf.first(tmp_bufs_used), ec);
 			if (ec) return -1;
 

test/test_storage.cpp

@@ -1079,12 +1079,12 @@ file_storage make_fs()
 	return fs;
 }
 
-struct test_fileop : aux::fileop
+struct test_fileop
 {
 	explicit test_fileop(int stripe_size) : m_stripe_size(stripe_size) {}
 
-	int file_op(file_index_t const file_index, std::int64_t const file_offset
+	int operator()(file_index_t const file_index, std::int64_t const file_offset
-		, span<iovec_t const> bufs, storage_error& ec) override
+		, span<iovec_t const> bufs, storage_error& ec)
 	{
 		size_t offset = size_t(file_offset);
 		if (file_index >= m_file_data.end_index())
@@ -1117,13 +1117,13 @@ struct test_fileop : aux::fileop
 	aux::vector<std::vector<char>, file_index_t> m_file_data;
 };
 
-struct test_read_fileop : aux::fileop
+struct test_read_fileop
 {
 	// EOF after size bytes read
 	explicit test_read_fileop(int size) : m_size(size), m_counter(0) {}
 
-	int file_op(file_index_t const file_index, std::int64_t const file_offset
+	int operator()(file_index_t const file_index, std::int64_t const file_offset
-		, span<iovec_t const> bufs, storage_error& ec) override
+		, span<iovec_t const> bufs, storage_error& ec)
 	{
 		int local_size = std::min(m_size, bufs_size(bufs));
 		const int read = local_size;
@@ -1147,14 +1147,14 @@ struct test_read_fileop : aux::fileop
 	int m_counter;
 };
 
-struct test_error_fileop : aux::fileop
+struct test_error_fileop
 {
 	// EOF after size bytes read
 	explicit test_error_fileop(file_index_t error_file)
 		: m_error_file(error_file) {}
 
-	int file_op(file_index_t const file_index, std::int64_t const file_offset
+	int operator()(file_index_t const file_index, std::int64_t const file_offset
-		, span<iovec_t const> bufs, storage_error& ec) override
+		, span<iovec_t const> bufs, storage_error& ec)
 	{
 		if (m_error_file == file_index)
 		{
@@ -1201,7 +1201,8 @@ TORRENT_TEST(readwritev_stripe_1)
 	int num_bufs2 = count_bufs(iov2, int(fs.total_size()));
 	TEST_CHECK(num_bufs2 <= num_bufs);
 
-	int ret = readwritev(fs, {iov2, size_t(num_bufs2)}, piece_index_t(0), 0, fop, ec);
+	int ret = readwritev(fs, {iov2, size_t(num_bufs2)}, piece_index_t(0), 0, ec
+		, std::ref(fop));
 
 	TEST_EQUAL(ret, fs.total_size());
 	TEST_EQUAL(fop.m_file_data.size(), 4);
@@ -1228,7 +1229,7 @@ TORRENT_TEST(readwritev_single_buffer)
 	iovec_t iov = { &buf[0], buf.size() };
 	fill_pattern(&iov, 1);
 
-	int ret = readwritev(fs, iov, piece_index_t(0), 0, fop, ec);
+	int ret = readwritev(fs, iov, piece_index_t(0), 0, ec, std::ref(fop));
 
 	TEST_EQUAL(ret, fs.total_size());
 	TEST_EQUAL(fop.m_file_data.size(), 4);
@@ -1253,7 +1254,7 @@ TORRENT_TEST(readwritev_read)
 	iovec_t iov = { &buf[0], buf.size() };
 
 	// read everything
-	int ret = readwritev(fs, iov, piece_index_t(0), 0, fop, ec);
+	int ret = readwritev(fs, iov, piece_index_t(0), 0, ec, std::ref(fop));
 
 	TEST_EQUAL(ret, fs.total_size());
 	TEST_CHECK(check_pattern(buf, 0));
@@ -1270,7 +1271,7 @@ TORRENT_TEST(readwritev_read_short)
 		, static_cast<size_t>(fs.total_size()) };
 
 	// read everything
-	int ret = readwritev(fs, iov, piece_index_t(0), 0, fop, ec);
+	int ret = readwritev(fs, iov, piece_index_t(0), 0, ec, std::ref(fop));
 
 	TEST_EQUAL(static_cast<int>(ec.file()), 3);
 
@@ -1290,7 +1291,7 @@ TORRENT_TEST(readwritev_error)
 		, static_cast<size_t>(fs.total_size()) };
 
 	// read everything
-	int ret = readwritev(fs, iov, piece_index_t(0), 0, fop, ec);
+	int ret = readwritev(fs, iov, piece_index_t(0), 0, ec, std::ref(fop));
 
 	TEST_EQUAL(ret, -1);
 	TEST_EQUAL(static_cast<int>(ec.file()), 2);
@@ -1317,7 +1318,7 @@ TORRENT_TEST(readwritev_zero_size_files)
 		, static_cast<size_t>(fs.total_size()) };
 
 	// read everything
-	int ret = readwritev(fs, iov, piece_index_t(0), 0, fop, ec);
+	int ret = readwritev(fs, iov, piece_index_t(0), 0, ec, std::ref(fop));
 
 	TEST_EQUAL(ret, fs.total_size());
 	TEST_CHECK(check_pattern(buf, 0));