improve const correctness of disk I/O code by making readwrite() a te…

…mplate that can work with both span<char> and span<char const>.

CMakeLists.txt

@@ -231,6 +231,7 @@ set(libtorrent_aux_include_files
 	puff.hpp
 	random.hpp
 	range.hpp
+	readwrite.hpp
 	receive_buffer.hpp
 	request_blocks.hpp
 	resolve_links.hpp

Makefile

@@ -631,6 +631,7 @@ HEADERS = \
   aux_/puff.hpp                     \
   aux_/random.hpp                   \
   aux_/range.hpp                    \
+  aux_/readwrite.hpp                \
   aux_/receive_buffer.hpp           \
   aux_/request_blocks.hpp           \
   aux_/resolve_links.hpp            \

include/libtorrent/aux_/mmap_storage.hpp

@@ -83,7 +83,7 @@ namespace libtorrent::aux {
 			, piece_index_t piece, int offset, aux::open_mode_t mode
 			, disk_job_flags_t flags
 			, storage_error&);
-		int write(settings_interface const&, span<char> buffer
+		int write(settings_interface const&, span<char const> buffer
 			, piece_index_t piece, int offset, aux::open_mode_t mode
 			, disk_job_flags_t flags
 			, storage_error&);

include/libtorrent/aux_/part_file.hpp

@@ -26,7 +26,6 @@ see LICENSE file.
 #include "libtorrent/units.hpp"
 #include "libtorrent/hasher.hpp"
 #include "libtorrent/aux_/open_mode.hpp"
-#include "libtorrent/aux_/storage_utils.hpp" // for iovec_t
 
 namespace libtorrent::aux {
 
@@ -39,7 +38,7 @@ namespace libtorrent::aux {
 		part_file(std::string path, std::string name, int num_pieces, int piece_size);
 		~part_file();
 
-		int write(span<char> buf, piece_index_t piece, int offset, error_code& ec);
+		int write(span<char const> buf, piece_index_t piece, int offset, error_code& ec);
 		int read(span<char> buf, piece_index_t piece, int offset, error_code& ec);
 		int hash(hasher& ph, std::ptrdiff_t len, piece_index_t piece, int offset, error_code& ec);
 		int hash2(hasher256& ph, std::ptrdiff_t len, piece_index_t piece, int offset, error_code& ec);

include/libtorrent/aux_/posix_part_file.hpp

@@ -23,7 +23,6 @@ see LICENSE file.
 #include "libtorrent/units.hpp"
 #include "libtorrent/hasher.hpp"
 #include "libtorrent/aux_/open_mode.hpp"
-#include "libtorrent/aux_/storage_utils.hpp" // for iovec_t
 #include "libtorrent/aux_/file_pointer.hpp"
 
 namespace libtorrent {
@@ -38,7 +37,7 @@ namespace aux {
 		posix_part_file(std::string path, std::string name, int num_pieces, int piece_size);
 		~posix_part_file();
 
-		int write(span<char> bufs, piece_index_t piece, int offset, error_code& ec);
+		int write(span<char const> bufs, piece_index_t piece, int offset, error_code& ec);
 		int read(span<char> buf, piece_index_t piece, int offset, error_code& ec);
 		int hash(hasher& ph, std::ptrdiff_t len, piece_index_t piece, int offset, error_code& ec);
 		int hash2(hasher256& ph, std::ptrdiff_t len, piece_index_t piece, int offset, error_code& ec);

include/libtorrent/aux_/posix_storage.hpp

@@ -16,7 +16,6 @@ see LICENSE file.
 #include "libtorrent/aux_/stat_cache.hpp"
 #include "libtorrent/file_storage.hpp"
 #include "libtorrent/storage_defs.hpp"
-#include "libtorrent/aux_/storage_utils.hpp" // for iovec_t
 #include "libtorrent/hex.hpp" // to_hex
 #include "libtorrent/aux_/open_mode.hpp" // for aux::open_mode_t
 #include "libtorrent/aux_/file_pointer.hpp"
@@ -41,7 +40,7 @@ namespace aux {
 			, storage_error& error);
 
 		int write(settings_interface const& sett
-			, span<char> buffer
+			, span<char const> buffer
 			, piece_index_t const piece, int const offset
 			, storage_error& error);
 

include/libtorrent/aux_/readwrite.hpp

@@ -0,0 +1,137 @@
+/*
+
+Copyright (c) 2017-2020, 2022, Arvid Norberg
+Copyright (c) 2018, 2020, Alden Torres
+All rights reserved.
+
+You may use, distribute and modify this code under the terms of the BSD license,
+see LICENSE file.
+*/
+
+#ifndef TORRENT_READWRITE_HPP_INCLUDE
+#define TORRENT_READWRITE_HPP_INCLUDE
+
+#include <cstdint>
+#include <functional>
+
+#include "libtorrent/units.hpp"
+#include "libtorrent/file_storage.hpp"
+#include "libtorrent/span.hpp"
+#include "libtorrent/assert.hpp"
+#include "libtorrent/socket.hpp"
+
+namespace libtorrent::aux {
+
+// 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).
+//
+// Much of what needs to be done when reading and writing is buffer
+// management and piece to file mapping. Most of that is the same for reading
+// and writing. This function is a template, and the fileop decides what to
+// do with the file and the buffers.
+//
+// op is a read or write operation so that readwrite() knows
+// what to do when it's actually touching the file
+
+template <typename Fun>
+int readwrite(file_storage const& files, span<char> buf
+	, piece_index_t const piece, const int offset
+	, storage_error& ec, Fun op)
+{
+	return readwrite_impl(files, buf, piece, offset, ec, op);
+}
+
+template <typename Fun>
+int readwrite(file_storage const& files, span<char const> buf
+	, piece_index_t const piece, const int offset
+	, storage_error& ec, Fun op)
+{
+	return readwrite_impl(files, buf, piece, offset, ec, op);
+}
+
+template <typename Char, typename Fun>
+int readwrite_impl(file_storage const& files, span<Char> buf
+	, piece_index_t const piece, const int offset
+	, storage_error& ec, Fun op)
+{
+	TORRENT_ASSERT(piece >= piece_index_t(0));
+	TORRENT_ASSERT(piece < files.end_piece());
+	TORRENT_ASSERT(offset >= 0);
+	TORRENT_ASSERT(buf.size() > 0);
+
+	TORRENT_ASSERT(buf.size() > 0);
+	TORRENT_ASSERT(static_cast<int>(piece) * static_cast<std::int64_t>(files.piece_length())
+		+ offset + buf.size() <= files.total_size());
+
+	// find the file iterator and file offset
+	std::int64_t const torrent_offset = static_cast<int>(piece) * std::int64_t(files.piece_length()) + offset;
+	file_index_t file_index = files.file_index_at_offset(torrent_offset);
+	TORRENT_ASSERT(torrent_offset >= files.file_offset(file_index));
+	TORRENT_ASSERT(torrent_offset < files.file_offset(file_index) + files.file_size(file_index));
+	std::int64_t file_offset = torrent_offset - files.file_offset(file_index);
+
+	int ret = 0;
+
+	while (buf.size() > 0)
+	{
+		// the number of bytes left to read in the current file (specified by
+		// file_index). This is the minimum of (file_size - file_offset) and
+		// buf.size().
+		int file_bytes_left = int(buf.size());
+		if (file_offset + file_bytes_left > files.file_size(file_index))
+			file_bytes_left = std::max(static_cast<int>(files.file_size(file_index) - file_offset), 0);
+
+		// there are no bytes left in this file, move to the next one
+		// this loop skips over empty files
+		if (file_bytes_left == 0)
+		{
+			do
+			{
+				++file_index;
+				file_offset = 0;
+				TORRENT_ASSERT(file_index < files.end_file());
+
+				// this should not happen. buf.size() should be clamped by the total
+				// size of the torrent, so we should never run off the end of it
+				if (file_index >= files.end_file()) return ret;
+
+				// skip empty files
+			}
+			while (files.file_size(file_index) == 0);
+
+			file_bytes_left = int(buf.size());
+			if (file_offset + file_bytes_left > files.file_size(file_index))
+				file_bytes_left = std::max(static_cast<int>(files.file_size(file_index) - file_offset), 0);
+			TORRENT_ASSERT(file_bytes_left > 0);
+		}
+
+		int const bytes_transferred = op(file_index, file_offset
+			, buf.first(file_bytes_left), ec);
+		TORRENT_ASSERT(bytes_transferred <= file_bytes_left);
+		if (ec)
+		{
+			ec.file(file_index);
+			return ret;
+		}
+
+		buf = buf.subspan(bytes_transferred);
+		file_offset += bytes_transferred;
+		ret += bytes_transferred;
+
+		// if the file operation returned 0, we've hit end-of-file. We're done
+		if (bytes_transferred == 0 && file_bytes_left > 0 )
+		{
+			// fill in this information in case the caller wants to treat
+			// a short-read as an error
+			ec.operation = operation_t::file_read;
+			ec.ec = boost::asio::error::eof;
+			ec.file(file_index);
+		}
+	}
+	return ret;
+}
+
+}
+
+#endif

include/libtorrent/aux_/storage_utils.hpp

@@ -18,33 +18,15 @@ see LICENSE file.
 #include "libtorrent/config.hpp"
 #include "libtorrent/fwd.hpp"
 #include "libtorrent/span.hpp"
-#include "libtorrent/span.hpp"
 #include "libtorrent/units.hpp"
 #include "libtorrent/disk_interface.hpp" // for status_t
 #include "libtorrent/session_types.hpp"
 #include "libtorrent/error_code.hpp"
 
-namespace libtorrent {
-
-	// TODO: 3 remove this typedef, and use span<char const> for disk write
-	// operations
-	using iovec_t = span<char>;
-
-namespace aux {
+namespace libtorrent::aux {
 
 	struct stat_cache;
 
-	// this is a read or write operation so that readwrite() knows
-	// what to do when it's actually touching the file
-	using fileop = std::function<int(file_index_t, std::int64_t, span<char>, storage_error&)>;
-
-	// 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 readwrite(file_storage const& files
-		, span<char> buf, piece_index_t piece, int offset
-		, 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``.
 	// returns the status code and the new save_path.
@@ -102,6 +84,6 @@ namespace aux {
 
 	TORRENT_EXTRA_EXPORT void copy_file(std::string const& f
 		, std::string const& newf, storage_error& se);
-}}
+}
 
 #endif

include/libtorrent/aux_/utp_stream.hpp

@@ -23,7 +23,6 @@ see LICENSE file.
 #include "libtorrent/aux_/sliding_average.hpp"
 #include "libtorrent/address.hpp"
 #include "libtorrent/aux_/invariant_check.hpp"
-#include "libtorrent/aux_/storage_utils.hpp" // for iovec_t
 
 #include <functional>
 
@@ -35,6 +34,8 @@ see LICENSE file.
 
 namespace libtorrent::aux {
 
+	using iovec_t = span<char>;
+
 #ifndef TORRENT_UTP_LOG_ENABLE
 	#define TORRENT_UTP_LOG 0
 	#define TORRENT_VERBOSE_UTP_LOG 0

src/mmap_storage.cpp

@@ -39,6 +39,7 @@ see LICENSE file.
 #include "libtorrent/aux_/file_view_pool.hpp"
 #include "libtorrent/aux_/drive_info.hpp"
 #include "libtorrent/aux_/stat_cache.hpp"
+#include "libtorrent/aux_/readwrite.hpp"
 #include "libtorrent/hex.hpp" // to_hex
 
 #if TORRENT_HAVE_MMAP || TORRENT_HAVE_MAP_VIEW_OF_FILE
@@ -582,7 +583,7 @@ error_code translate_error(std::error_code const& err, bool const write)
 	}
 
 	int mmap_storage::write(settings_interface const& sett
-		, span<char> buffer
+		, span<char const> buffer
 		, piece_index_t const piece, int const offset
 		, aux::open_mode_t const mode
 		, disk_job_flags_t const flags
@@ -594,7 +595,7 @@ error_code translate_error(std::error_code const& err, bool const write)
 		return readwrite(files(), buffer, piece, offset, error
 			, [this, mode, flags, &sett](file_index_t const file_index
 				, std::int64_t const file_offset
-				, span<char> buf, storage_error& ec)
+				, span<char const> buf, storage_error& ec)
 		{
 			if (files().pad_file_at(file_index))
 			{
@@ -682,10 +683,10 @@ error_code translate_error(std::error_code const& err, bool const write)
 		char dummy;
 		std::vector<char> scratch;
 
-		return readwrite(files(), {&dummy, len}, piece, offset, error
+		return readwrite(files(), span<char const>{&dummy, len}, piece, offset, error
 			, [this, mode, flags, &ph, &sett, &scratch](file_index_t const file_index
 				, std::int64_t const file_offset
-				, span<char> const buf, storage_error& ec)
+				, span<char const> const buf, storage_error& ec)
 		{
 			if (files().pad_file_at(file_index))
 				return aux::hash_zeroes(ph, buf.size());

src/part_file.cpp

@@ -46,7 +46,6 @@ see LICENSE file.
 #include "libtorrent/assert.hpp"
 #include "libtorrent/aux_/vector.hpp"
 #include "libtorrent/aux_/path.hpp"
-#include "libtorrent/aux_/storage_utils.hpp" // for iovec_t
 
 #include <functional> // for std::function
 #include <cstdint>
@@ -149,7 +148,7 @@ namespace libtorrent::aux {
 		return slot;
 	}
 
-	int part_file::write(span<char> buf, piece_index_t const piece
+	int part_file::write(span<char const> buf, piece_index_t const piece
 		, int const offset, error_code& ec)
 	{
 		TORRENT_ASSERT(offset >= 0);

src/posix_part_file.cpp

@@ -47,7 +47,7 @@ see LICENSE file.
 #include "libtorrent/assert.hpp"
 #include "libtorrent/aux_/vector.hpp"
 #include "libtorrent/aux_/path.hpp"
-#include "libtorrent/aux_/storage_utils.hpp" // for iovec_t
+#include "libtorrent/aux_/storage_utils.hpp" // for copy_file
 
 #include <functional> // for std::function
 #include <cstdint>
@@ -153,7 +153,7 @@ namespace aux {
 		return slot;
 	}
 
-	int posix_part_file::write(span<char> buf, piece_index_t const piece
+	int posix_part_file::write(span<char const> buf, piece_index_t const piece
 		, int const offset, error_code& ec)
 	{
 		TORRENT_ASSERT(offset >= 0);

src/posix_storage.cpp

@@ -17,6 +17,8 @@ see LICENSE file.
 #include "libtorrent/aux_/open_mode.hpp"
 #include "libtorrent/aux_/file_pointer.hpp"
 #include "libtorrent/torrent_status.hpp"
+#include "libtorrent/aux_/storage_utils.hpp" // for read_zeroes, move_storage
+#include "libtorrent/aux_/readwrite.hpp"
 
 using namespace libtorrent::flags; // for flag operators
 
@@ -197,7 +199,7 @@ namespace aux {
 	}
 
 	int posix_storage::write(settings_interface const&
-		, span<char> buffer
+		, span<char const> buffer
 		, piece_index_t const piece, int const offset
 		, storage_error& error)
 	{
@@ -207,7 +209,7 @@ namespace aux {
 		return readwrite(files(), buffer, piece, offset, error
 			, [this](file_index_t const file_index
 				, std::int64_t const file_offset
-				, span<char> buf, storage_error& ec)
+				, span<char const> buf, storage_error& ec)
 		{
 			if (files().pad_file_at(file_index))
 			{