From 7ccc4ddf0de992462de03b10a2450ae7c2cdb588 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max.kellermann@gmail.com>
Date: Thu, 5 Oct 2023 10:25:16 +0200
Subject: [PATCH] io/Reader: use std::span

---
 src/decoder/Reader.cxx                       |  6 +++---
 src/decoder/Reader.hxx                       |  2 +-
 src/decoder/plugins/SidplayDecoderPlugin.cxx |  2 +-
 src/input/Reader.cxx                         |  6 +++---
 src/input/Reader.hxx                         |  2 +-
 src/input/plugins/FileInputPlugin.cxx        |  2 +-
 src/io/BufferedReader.cxx                    |  6 +++---
 src/io/BufferedReader.hxx                    |  2 +-
 src/io/FileReader.cxx                        |  8 ++++----
 src/io/FileReader.hxx                        |  2 +-
 src/io/PeekReader.cxx                        | 21 ++++++++++----------
 src/io/PeekReader.hxx                        |  4 ++--
 src/io/Reader.hxx                            |  6 +++---
 src/lib/xiph/OggSyncState.cxx                |  4 ++--
 src/lib/zlib/AutoGunzipReader.cxx            |  6 +++---
 src/lib/zlib/AutoGunzipReader.hxx            |  2 +-
 src/lib/zlib/GunzipReader.cxx                | 16 +++++++--------
 src/lib/zlib/GunzipReader.hxx                |  4 ++--
 test/run_gunzip.cxx                          |  2 +-
 19 files changed, 52 insertions(+), 51 deletions(-)

diff --git a/src/decoder/Reader.cxx b/src/decoder/Reader.cxx
index 0a91fb180..4e2b50604 100644
--- a/src/decoder/Reader.cxx
+++ b/src/decoder/Reader.cxx
@@ -4,8 +4,8 @@
 #include "Reader.hxx"
 #include "DecoderAPI.hxx"
 
-size_t
-DecoderReader::Read(void *data, size_t size)
+std::size_t
+DecoderReader::Read(std::span<std::byte> dest)
 {
-	return decoder_read(client, is, data, size);
+	return decoder_read(client, is, dest.data(), dest.size());
 }
diff --git a/src/decoder/Reader.hxx b/src/decoder/Reader.hxx
index 8cc996369..aa50a7f6e 100644
--- a/src/decoder/Reader.hxx
+++ b/src/decoder/Reader.hxx
@@ -30,7 +30,7 @@ public:
 	}
 
 	/* virtual methods from class Reader */
-	size_t Read(void *data, size_t size) override;
+	std::size_t Read(std::span<std::byte> dest) override;
 };
 
 #endif
diff --git a/src/decoder/plugins/SidplayDecoderPlugin.cxx b/src/decoder/plugins/SidplayDecoderPlugin.cxx
index 08dd97439..6c671e490 100644
--- a/src/decoder/plugins/SidplayDecoderPlugin.cxx
+++ b/src/decoder/plugins/SidplayDecoderPlugin.cxx
@@ -60,7 +60,7 @@ static constexpr unsigned rom_size = 8192;
 static void loadRom(const Path rom_path, uint8_t *dump)
 {
 	FileReader romDump(rom_path);
-	if (romDump.Read(dump, rom_size) != rom_size)
+	if (romDump.Read(std::as_writable_bytes(std::span{dump, rom_size})) != rom_size)
 		throw FmtRuntimeError("Could not load rom dump '{}'", rom_path);
 }
 
diff --git a/src/input/Reader.cxx b/src/input/Reader.cxx
index d8d345b1d..be2b2b5a1 100644
--- a/src/input/Reader.cxx
+++ b/src/input/Reader.cxx
@@ -4,10 +4,10 @@
 #include "Reader.hxx"
 #include "InputStream.hxx"
 
-size_t
-InputStreamReader::Read(void *data, size_t size)
+std::size_t
+InputStreamReader::Read(std::span<std::byte> dest)
 {
-	size_t nbytes = is.LockRead(data, size);
+	size_t nbytes = is.LockRead(dest.data(), dest.size());
 	assert(nbytes > 0 || is.IsEOF());
 
 	return nbytes;
diff --git a/src/input/Reader.hxx b/src/input/Reader.hxx
index 25bac3a1d..f61a1b7f9 100644
--- a/src/input/Reader.hxx
+++ b/src/input/Reader.hxx
@@ -20,7 +20,7 @@ public:
 		:is(_is) {}
 
 	/* virtual methods from class Reader */
-	size_t Read(void *data, size_t size) override;
+	std::size_t Read(std::span<std::byte> dest) override;
 };
 
 #endif
diff --git a/src/input/plugins/FileInputPlugin.cxx b/src/input/plugins/FileInputPlugin.cxx
index b079270ca..0954956c8 100644
--- a/src/input/plugins/FileInputPlugin.cxx
+++ b/src/input/plugins/FileInputPlugin.cxx
@@ -78,7 +78,7 @@ FileInputStream::Read(std::unique_lock<Mutex> &,
 
 	{
 		const ScopeUnlock unlock(mutex);
-		nbytes = reader.Read(ptr, read_size);
+		nbytes = reader.Read({static_cast<std::byte *>(ptr), read_size});
 	}
 
 	if (nbytes == 0 && !IsEOF())
diff --git a/src/io/BufferedReader.cxx b/src/io/BufferedReader.cxx
index 7bd3dd6f8..d3ab85809 100644
--- a/src/io/BufferedReader.cxx
+++ b/src/io/BufferedReader.cxx
@@ -26,7 +26,7 @@ BufferedReader::Fill(bool need_more)
 		assert(!w.empty());
 	}
 
-	std::size_t nbytes = reader.Read(w.data(), w.size());
+	std::size_t nbytes = reader.Read(w);
 	if (nbytes == 0) {
 		eof = true;
 		return !need_more;
@@ -95,9 +95,9 @@ BufferedReader::ReadLine()
 	}
 
 	/* terminate the last line */
-	w[0] = 0;
+	w[0] = {};
 
-	char *line = buffer.Read().data();
+	char *line = reinterpret_cast<char *>(buffer.Read().data());
 	buffer.Clear();
 	++line_number;
 	return line;
diff --git a/src/io/BufferedReader.hxx b/src/io/BufferedReader.hxx
index aee2bc770..d4f761e52 100644
--- a/src/io/BufferedReader.hxx
+++ b/src/io/BufferedReader.hxx
@@ -15,7 +15,7 @@ class BufferedReader {
 
 	Reader &reader;
 
-	DynamicFifoBuffer<char> buffer;
+	DynamicFifoBuffer<std::byte> buffer;
 
 	bool eof = false;
 
diff --git a/src/io/FileReader.cxx b/src/io/FileReader.cxx
index b7d8f597e..8fb11c853 100644
--- a/src/io/FileReader.cxx
+++ b/src/io/FileReader.cxx
@@ -30,12 +30,12 @@ FileReader::GetFileInfo() const
 }
 
 std::size_t
-FileReader::Read(void *data, std::size_t size)
+FileReader::Read(std::span<std::byte> dest)
 {
 	assert(IsDefined());
 
 	DWORD nbytes;
-	if (!ReadFile(handle, data, size, &nbytes, nullptr))
+	if (!ReadFile(handle, dest.data(), dest.size(), &nbytes, nullptr))
 		throw FmtLastError("Failed to read from {}", path);
 
 	return nbytes;
@@ -82,11 +82,11 @@ FileReader::GetFileInfo() const
 }
 
 std::size_t
-FileReader::Read(void *data, std::size_t size)
+FileReader::Read(std::span<std::byte> dest)
 {
 	assert(IsDefined());
 
-	ssize_t nbytes = fd.Read(data, size);
+	ssize_t nbytes = fd.Read(dest);
 	if (nbytes < 0)
 		throw FmtErrno("Failed to read from {}", path);
 
diff --git a/src/io/FileReader.hxx b/src/io/FileReader.hxx
index 671101bc5..476ccea7d 100644
--- a/src/io/FileReader.hxx
+++ b/src/io/FileReader.hxx
@@ -100,5 +100,5 @@ public:
 	void Skip(off_t offset);
 
 	/* virtual methods from class Reader */
-	std::size_t Read(void *data, std::size_t size) override;
+	std::size_t Read(std::span<std::byte> dest) override;
 };
diff --git a/src/io/PeekReader.cxx b/src/io/PeekReader.cxx
index f46af55f3..fddce2f09 100644
--- a/src/io/PeekReader.cxx
+++ b/src/io/PeekReader.cxx
@@ -17,8 +17,7 @@ PeekReader::Peek(size_t size)
 	assert(buffer_position == 0);
 
 	do {
-		size_t nbytes = next.Read(buffer + buffer_size,
-					  size - buffer_size);
+		size_t nbytes = next.Read(std::span{buffer}.first(buffer_size).subspan(size));
 		if (nbytes == 0)
 			return nullptr;
 
@@ -29,15 +28,17 @@ PeekReader::Peek(size_t size)
 }
 
 size_t
-PeekReader::Read(void *data, size_t size)
+PeekReader::Read(std::span<std::byte> dest)
 {
-	size_t buffer_remaining = buffer_size - buffer_position;
-	if (buffer_remaining > 0) {
-		size_t nbytes = std::min(buffer_remaining, size);
-		memcpy(data, buffer + buffer_position, nbytes);
-		buffer_position += nbytes;
-		return nbytes;
+	auto src = std::span{buffer}.first(buffer_size).subspan(buffer_position);
+	if (!src.empty()) {
+		if (dest.size() < src.size())
+			src = src.first(dest.size());
+
+		std::copy(src.begin(), src.end(), dest.begin());
+		buffer_position += src.size();
+		return src.size();
 	}
 
-	return next.Read(data, size);
+	return next.Read(dest);
 }
diff --git a/src/io/PeekReader.hxx b/src/io/PeekReader.hxx
index b7f5649c9..3d67b298d 100644
--- a/src/io/PeekReader.hxx
+++ b/src/io/PeekReader.hxx
@@ -18,7 +18,7 @@ class PeekReader final : public Reader {
 
 	size_t buffer_size = 0, buffer_position = 0;
 
-	uint8_t buffer[64];
+	std::byte buffer[64];
 
 public:
 	explicit PeekReader(Reader &_next)
@@ -27,7 +27,7 @@ public:
 	const void *Peek(size_t size);
 
 	/* virtual methods from class Reader */
-	size_t Read(void *data, size_t size) override;
+	size_t Read(std::span<std::byte> dest) override;
 };
 
 #endif
diff --git a/src/io/Reader.hxx b/src/io/Reader.hxx
index 970706548..7babefc49 100644
--- a/src/io/Reader.hxx
+++ b/src/io/Reader.hxx
@@ -5,6 +5,7 @@
 #define READER_HXX
 
 #include <cstddef>
+#include <span>
 #include <type_traits>
 
 /**
@@ -25,14 +26,13 @@ public:
 	 * @return the number of bytes read into the given buffer or 0
 	 * on end-of-stream
 	 */
-	[[gnu::nonnull]]
-	virtual std::size_t Read(void *data, std::size_t size) = 0;
+	virtual std::size_t Read(std::span<std::byte> dest) = 0;
 
 	template<typename T>
 	requires std::is_standard_layout_v<T> && std::is_trivially_copyable_v<T>
 	void ReadT(T &dest) {
 		// TODO check return value
-		Read(&dest, sizeof(dest));
+		Read(std::as_writable_bytes(std::span{&dest, 1}));
 	}
 };
 
diff --git a/src/lib/xiph/OggSyncState.cxx b/src/lib/xiph/OggSyncState.cxx
index 35eb2ac8e..08d8e98ea 100644
--- a/src/lib/xiph/OggSyncState.cxx
+++ b/src/lib/xiph/OggSyncState.cxx
@@ -7,11 +7,11 @@
 bool
 OggSyncState::Feed(size_t size)
 {
-	char *buffer = ogg_sync_buffer(&oy, size);
+	std::byte *buffer = reinterpret_cast<std::byte *>(ogg_sync_buffer(&oy, size));
 	if (buffer == nullptr)
 		return false;
 
-	size_t nbytes = reader.Read(buffer, size);
+	size_t nbytes = reader.Read({buffer, size});
 	if (nbytes == 0)
 		return false;
 
diff --git a/src/lib/zlib/AutoGunzipReader.cxx b/src/lib/zlib/AutoGunzipReader.cxx
index 606759f63..8c2735b07 100644
--- a/src/lib/zlib/AutoGunzipReader.cxx
+++ b/src/lib/zlib/AutoGunzipReader.cxx
@@ -32,12 +32,12 @@ AutoGunzipReader::Detect()
 		next = &peek;
 }
 
-size_t
-AutoGunzipReader::Read(void *data, size_t size)
+std::size_t
+AutoGunzipReader::Read(std::span<std::byte> dest)
 {
 	if (next == nullptr)
 		Detect();
 
 	assert(next != nullptr);
-	return next->Read(data, size);
+	return next->Read(dest);
 }
diff --git a/src/lib/zlib/AutoGunzipReader.hxx b/src/lib/zlib/AutoGunzipReader.hxx
index ef5ce5a0a..bb695e67f 100644
--- a/src/lib/zlib/AutoGunzipReader.hxx
+++ b/src/lib/zlib/AutoGunzipReader.hxx
@@ -24,7 +24,7 @@ public:
 	~AutoGunzipReader() noexcept;
 
 	/* virtual methods from class Reader */
-	size_t Read(void *data, size_t size) override;
+	std::size_t Read(std::span<std::byte> dest) override;
 
 private:
 	void Detect();
diff --git a/src/lib/zlib/GunzipReader.cxx b/src/lib/zlib/GunzipReader.cxx
index a39bbe50d..75bb38805 100644
--- a/src/lib/zlib/GunzipReader.cxx
+++ b/src/lib/zlib/GunzipReader.cxx
@@ -24,7 +24,7 @@ GunzipReader::FillBuffer()
 	auto w = buffer.Write();
 	assert(!w.empty());
 
-	std::size_t nbytes = next.Read(w.data(), w.size());
+	std::size_t nbytes = next.Read(w);
 	if (nbytes == 0)
 		return false;
 
@@ -33,13 +33,13 @@ GunzipReader::FillBuffer()
 }
 
 std::size_t
-GunzipReader::Read(void *data, std::size_t size)
+GunzipReader::Read(std::span<std::byte> dest)
 {
 	if (eof)
 		return 0;
 
-	z.next_out = (Bytef *)data;
-	z.avail_out = size;
+	z.next_out = (Bytef *)dest.data();
+	z.avail_out = dest.size();
 
 	while (true) {
 		int flush = Z_NO_FLUSH;
@@ -52,19 +52,19 @@ GunzipReader::Read(void *data, std::size_t size)
 				flush = Z_FINISH;
 		}
 
-		z.next_in = r.data();
+		z.next_in = reinterpret_cast<Bytef *>(r.data());
 		z.avail_in = r.size();
 
 		int result = inflate(&z, flush);
 		if (result == Z_STREAM_END) {
 			eof = true;
-			return size - z.avail_out;
+			return dest.size() - z.avail_out;
 		} else if (result != Z_OK)
 			throw ZlibError(result);
 
 		buffer.Consume(r.size() - z.avail_in);
 
-		if (z.avail_out < size)
-			return size - z.avail_out;
+		if (z.avail_out < dest.size())
+			return dest.size() - z.avail_out;
 	}
 }
diff --git a/src/lib/zlib/GunzipReader.hxx b/src/lib/zlib/GunzipReader.hxx
index 706ab489d..5cc6bf915 100644
--- a/src/lib/zlib/GunzipReader.hxx
+++ b/src/lib/zlib/GunzipReader.hxx
@@ -19,7 +19,7 @@ class GunzipReader final : public Reader {
 
 	z_stream z;
 
-	StaticFifoBuffer<Bytef, 65536> buffer;
+	StaticFifoBuffer<std::byte, 65536> buffer;
 
 public:
 	/**
@@ -34,7 +34,7 @@ public:
 	}
 
 	/* virtual methods from class Reader */
-	std::size_t Read(void *data, std::size_t size) override;
+	std::size_t Read(std::span<std::byte> dest) override;
 
 private:
 	bool FillBuffer();
diff --git a/test/run_gunzip.cxx b/test/run_gunzip.cxx
index 5cc5bb969..b8e97952b 100644
--- a/test/run_gunzip.cxx
+++ b/test/run_gunzip.cxx
@@ -16,7 +16,7 @@ Copy(OutputStream &dest, Reader &src)
 {
 	while (true) {
 		std::byte buffer[4096];
-		size_t nbytes = src.Read(buffer, sizeof(buffer));
+		std::size_t nbytes = src.Read(buffer);
 		if (nbytes == 0)
 			break;