Merge branch 'v0.21.x' into master

NEWS

@@ -13,8 +13,6 @@ ver 0.22 (not yet released)
   - ffmpeg: allow partial reads
   - io_uring: new plugin for local files on Linux (using liburing)
   - smbclient: close unused SMB/CIFS connections
-* archive
-  - iso9660: support seeking
 * database
   - upnp: drop support for libupnp versions older than 1.8
 * playlist
@@ -48,6 +46,8 @@ ver 0.21.26 (not yet released)
 * archive
   - bzip2: fix crash on corrupt bzip2 file
   - bzip2: flush output at end of input file
+  - iso9660: fix unaligned reads
+  - iso9660: support seeking
   - zzip: fix crash on corrupt ZIP file
 * decoder
   - sndfile: fix lost samples at end of file

src/archive/plugins/Iso9660ArchivePlugin.cxx

@@ -29,9 +29,12 @@
 #include "fs/Path.hxx"
 #include "util/RuntimeError.hxx"
 #include "util/StringCompare.hxx"
+#include "util/WritableBuffer.hxx"
 
 #include <cdio/iso9660.h>
 
+#include <array>
+
 #include <stdlib.h>
 #include <string.h>
 
@@ -150,6 +153,47 @@ class Iso9660InputStream final : public InputStream {
 
 	const lsn_t lsn;
 
+	/**
+	 * libiso9660 can only read whole sectors at a time, and this
+	 * buffer is used to store one whole sector and allow Read()
+	 * to handle partial sector reads.
+	 */
+	class BlockBuffer {
+		size_t position = 0, fill = 0;
+
+		std::array<uint8_t, ISO_BLOCKSIZE> data;
+
+	public:
+		ConstBuffer<uint8_t> Read() const noexcept {
+			assert(fill <= data.size());
+			assert(position <= fill);
+
+			return {&data[position], &data[fill]};
+		}
+
+		void Consume(size_t nbytes) noexcept {
+			assert(nbytes <= Read().size);
+
+			position += nbytes;
+		}
+
+		WritableBuffer<uint8_t> Write() noexcept {
+			assert(Read().empty());
+
+			return {data.data(), data.size()};
+		}
+
+		void Append(size_t nbytes) noexcept {
+			assert(Read().empty());
+			assert(nbytes <= data.size());
+
+			fill = nbytes;
+			position = 0;
+		}
+	};
+
+	BlockBuffer buffer;
+
 public:
 	Iso9660InputStream(std::shared_ptr<Iso9660> _iso,
 			   const char *_uri,
@@ -170,6 +214,9 @@ public:
 		    void *ptr, size_t size) override;
 
 	void Seek(std::unique_lock<Mutex> &, offset_type new_offset) override {
+		if (new_offset > size)
+			throw std::runtime_error("Invalid seek offset");
+
 		offset = new_offset;
 	}
 };
@@ -195,35 +242,56 @@ size_t
 Iso9660InputStream::Read(std::unique_lock<Mutex> &,
 			 void *ptr, size_t read_size)
 {
-	const ScopeUnlock unlock(mutex);
-
-	int readed = 0;
-	int no_blocks, cur_block;
-	size_t left_bytes = size - offset;
-
-	if (left_bytes < read_size) {
-		no_blocks = CEILING(left_bytes, ISO_BLOCKSIZE);
-	} else {
-		no_blocks = read_size / ISO_BLOCKSIZE;
-	}
-
-	if (no_blocks == 0)
+	const offset_type remaining = size - offset;
+	if (remaining == 0)
 		return 0;
 
-	cur_block = offset / ISO_BLOCKSIZE;
+	if (offset_type(read_size) > remaining)
+		read_size = remaining;
 
-	readed = iso->SeekRead(ptr, lsn + cur_block, no_blocks);
+	auto r = buffer.Read();
 
-	if (readed != no_blocks * ISO_BLOCKSIZE)
-		throw FormatRuntimeError("error reading ISO file at lsn %lu",
-					 (unsigned long)cur_block);
+	if (r.empty()) {
+		/* the buffer is empty - read more data from the ISO file */
 
-	if (left_bytes < read_size) {
-		readed = left_bytes;
+		assert(offset % ISO_BLOCKSIZE == 0);
+
+		const ScopeUnlock unlock(mutex);
+
+		const lsn_t read_lsn = lsn + offset / ISO_BLOCKSIZE;
+
+		if (read_size >= ISO_BLOCKSIZE) {
+			/* big read - read right into the caller's buffer */
+
+			auto nbytes = iso->SeekRead(ptr, read_lsn,
+						    read_size / ISO_BLOCKSIZE);
+			if (nbytes <= 0)
+				throw std::runtime_error("Failed to read ISO9660 file");
+
+			offset += nbytes;
+			return nbytes;
+		}
+
+		/* fill the buffer */
+
+		auto w = buffer.Write();
+		auto nbytes = iso->SeekRead(w.data, read_lsn,
+					    w.size / ISO_BLOCKSIZE);
+		if (nbytes <= 0)
+			throw std::runtime_error("Failed to read ISO9660 file");
+
+		buffer.Append(nbytes);
+
+		r = buffer.Read();
 	}
 
-	offset += readed;
-	return readed;
+	assert(!r.empty());
+
+	size_t nbytes = std::min(read_size, r.size);
+	memcpy(ptr, r.data, nbytes);
+	buffer.Consume(nbytes);
+	offset += nbytes;
+	return nbytes;
 }
 
 bool

test/run_input.cxx

@@ -49,11 +49,15 @@
 #include <unistd.h>
 #include <stdlib.h>
 
+static constexpr std::size_t MAX_CHUNK_SIZE = 16384;
+
 struct CommandLine {
 	const char *uri = nullptr;
 
 	FromNarrowPath config_path;
 
+	std::size_t chunk_size = MAX_CHUNK_SIZE;
+
 	bool verbose = false;
 
 	bool scan = false;
@@ -63,14 +67,27 @@ enum Option {
 	OPTION_CONFIG,
 	OPTION_VERBOSE,
 	OPTION_SCAN,
+	OPTION_CHUNK_SIZE,
 };
 
 static constexpr OptionDef option_defs[] = {
 	{"config", 0, true, "Load a MPD configuration file"},
 	{"verbose", 'v', false, "Verbose logging"},
 	{"scan", 0, false, "Scan tags instead of reading raw data"},
+	{"chunk-size", 0, true, "Read this number of bytes at a time"},
 };
 
+static std::size_t
+ParseSize(const char *s)
+{
+	char *endptr;
+	std::size_t value = std::strtoul(s, &endptr, 10);
+	if (endptr == s)
+		throw std::runtime_error("Failed to parse integer");
+
+	return value;
+}
+
 static CommandLine
 ParseCommandLine(int argc, char **argv)
 {
@@ -90,6 +107,12 @@ ParseCommandLine(int argc, char **argv)
 		case OPTION_SCAN:
 			c.scan = true;
 			break;
+
+		case OPTION_CHUNK_SIZE:
+			c.chunk_size = ParseSize(o.value);
+			if (c.chunk_size <= 0 || c.chunk_size > MAX_CHUNK_SIZE)
+				throw std::runtime_error("Invalid chunk size");
+			break;
 		}
 	}
 
@@ -130,7 +153,7 @@ tag_save(FILE *file, const Tag &tag)
 }
 
 static int
-dump_input_stream(InputStream &is, FileDescriptor out)
+dump_input_stream(InputStream &is, FileDescriptor out, size_t chunk_size)
 {
 	std::unique_lock<Mutex> lock(is.mutex);
 
@@ -150,8 +173,9 @@ dump_input_stream(InputStream &is, FileDescriptor out)
 			}
 		}
 
-		char buffer[4096];
-		size_t num_read = is.Read(lock, buffer, sizeof(buffer));
+		char buffer[MAX_CHUNK_SIZE];
+		assert(chunk_size <= sizeof(buffer));
+		size_t num_read = is.Read(lock, buffer, chunk_size);
 		if (num_read == 0)
 			break;
 
@@ -231,7 +255,8 @@ try {
 
 	Mutex mutex;
 	auto is = InputStream::OpenReady(c.uri, mutex);
-	return dump_input_stream(*is, FileDescriptor(STDOUT_FILENO));
+	return dump_input_stream(*is, FileDescriptor(STDOUT_FILENO),
+				 c.chunk_size);
 } catch (...) {
 	PrintException(std::current_exception());
 	return EXIT_FAILURE;

test/test_archive_iso9660.sh

@@ -7,4 +7,7 @@ DST="$(pwd)/test/tmp/${SRC_BASE}.iso"
 mkdir -p test/tmp
 rm -f "$DST"
 mkisofs -quiet -l -o "$DST" "$SRC"
-./test/run_input "$DST/${SRC_BASE}" |diff "$SRC" -
+
+# Using an odd chunk size to check whether the plugin can cope with
+# partial sectors
+./test/run_input --chunk-size=1337 "$DST/${SRC_BASE}" |diff "$SRC" -