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))
  {