From: Timo Sirainen <timo.sirainen@dovecot.fi>
Date: Tue, 5 Jun 2018 10:57:26 +0000 (+0300)
Subject: lib-compression: Use i_stream_nonseekable_try_seek() instead of reimplementing it
X-Git-Tag: 2.3.2.rc1~54
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=6472b5cc0677ea47508d0fde230467da14e6bf09;p=thirdparty%2Fdovecot%2Fcore.git

lib-compression: Use i_stream_nonseekable_try_seek() instead of reimplementing it
---

diff --git a/src/lib-compression/istream-bzlib.c b/src/lib-compression/istream-bzlib.c
index 092fcb5c5b..88e8652a10 100644
--- a/src/lib-compression/istream-bzlib.c
+++ b/src/lib-compression/istream-bzlib.c
@@ -15,7 +15,6 @@ struct bzlib_istream {
 
 	bz_stream zs;
 	uoff_t eof_offset;
-	size_t high_pos;
 	struct stat last_parent_statbuf;
 
 	bool log_errors:1;
@@ -56,28 +55,10 @@ static ssize_t i_stream_bzlib_read(struct istream_private *stream)
 
 	high_offset = stream->istream.v_offset + (stream->pos - stream->skip);
 	if (zstream->eof_offset == high_offset) {
-		i_assert(zstream->high_pos == 0 ||
-			 zstream->high_pos == stream->pos);
 		stream->istream.eof = TRUE;
 		return -1;
 	}
 
-	if (stream->pos < zstream->high_pos) {
-		/* we're here because we seeked back within the read buffer. */
-		ret = zstream->high_pos - stream->pos;
-		stream->pos = zstream->high_pos;
-		zstream->high_pos = 0;
-
-		if (zstream->eof_offset != (uoff_t)-1) {
-			high_offset = stream->istream.v_offset +
-				(stream->pos - stream->skip);
-			i_assert(zstream->eof_offset == high_offset);
-			stream->istream.eof = TRUE;
-		}
-		return ret;
-	}
-	zstream->high_pos = 0;
-
 	if (!zstream->marked) {
 		if (!i_stream_try_alloc(stream, CHUNK_SIZE, &out_size))
 			return -2; /* buffer full */
@@ -183,7 +164,7 @@ static void i_stream_bzlib_reset(struct bzlib_istream *zstream)
 	stream->parent_expected_offset = stream->parent_start_offset;
 	stream->skip = stream->pos = 0;
 	stream->istream.v_offset = 0;
-	zstream->high_pos = 0;
+	stream->high_pos = 0;
 
 	(void)BZ2_bzDecompressEnd(&zstream->zs);
 	i_stream_bzlib_init(zstream);
@@ -193,56 +174,14 @@ static void
 i_stream_bzlib_seek(struct istream_private *stream, uoff_t v_offset, bool mark)
 {
 	struct bzlib_istream *zstream = (struct bzlib_istream *) stream;
-	uoff_t start_offset = stream->istream.v_offset - stream->skip;
-
-	if (v_offset < start_offset) {
-		/* have to seek backwards */
-		i_stream_bzlib_reset(zstream);
-		start_offset = 0;
-	} else if (zstream->high_pos != 0) {
-		stream->pos = zstream->high_pos;
-		zstream->high_pos = 0;
-	}
 
-	if (v_offset <= start_offset + stream->pos) {
-		/* seeking backwards within what's already cached */
-		stream->skip = v_offset - start_offset;
-		stream->istream.v_offset = v_offset;
-		zstream->high_pos = stream->pos;
-		stream->pos = stream->skip;
-	} else {
-		/* read and cache forward */
-		ssize_t ret;
-
-		do {
-			size_t avail = stream->pos - stream->skip;
-
-			if (stream->istream.v_offset + avail >= v_offset) {
-				i_stream_skip(&stream->istream,
-					      v_offset -
-					      stream->istream.v_offset);
-				ret = -1;
-				break;
-			}
-
-			i_stream_skip(&stream->istream, avail);
-		} while ((ret = i_stream_read(&stream->istream)) > 0);
-		i_assert(ret == -1);
-
-		if (stream->istream.v_offset != v_offset) {
-			/* some failure, we've broken it */
-			if (stream->istream.stream_errno != 0) {
-				i_error("bzlib_istream.seek(%s) failed: %s",
-					i_stream_get_name(&stream->istream),
-					strerror(stream->istream.stream_errno));
-				i_stream_close(&stream->istream);
-			} else {
-				/* unexpected EOF. allow it since we may just
-				   want to check if there's anything.. */
-				i_assert(stream->istream.eof);
-			}
-		}
-	}
+	if (i_stream_nonseekable_try_seek(stream, v_offset))
+		return;
+
+	/* have to seek backwards - reset state and retry */
+	i_stream_bzlib_reset(zstream);
+	if (!i_stream_nonseekable_try_seek(stream, v_offset))
+		i_unreached();
 
 	if (mark)
 		zstream->marked = TRUE;
diff --git a/src/lib-compression/istream-lz4.c b/src/lib-compression/istream-lz4.c
index 2a7551b475..6bde36ee7e 100644
--- a/src/lib-compression/istream-lz4.c
+++ b/src/lib-compression/istream-lz4.c
@@ -180,52 +180,14 @@ static void
 i_stream_lz4_seek(struct istream_private *stream, uoff_t v_offset, bool mark)
 {
 	struct lz4_istream *zstream = (struct lz4_istream *) stream;
-	uoff_t start_offset = stream->istream.v_offset - stream->skip;
 
-	if (v_offset < start_offset) {
-		/* have to seek backwards */
-		i_stream_lz4_reset(zstream);
-		start_offset = 0;
-	}
-
-	if (v_offset <= start_offset + stream->pos) {
-		/* seeking backwards within what's already cached */
-		stream->skip = v_offset - start_offset;
-		stream->istream.v_offset = v_offset;
-		stream->pos = stream->skip;
-	} else {
-		/* read and cache forward */
-		ssize_t ret;
-
-		do {
-			size_t avail = stream->pos - stream->skip;
-
-			if (stream->istream.v_offset + avail >= v_offset) {
-				i_stream_skip(&stream->istream,
-					      v_offset -
-					      stream->istream.v_offset);
-				ret = -1;
-				break;
-			}
+	if (i_stream_nonseekable_try_seek(stream, v_offset))
+		return;
 
-			i_stream_skip(&stream->istream, avail);
-		} while ((ret = i_stream_read(&stream->istream)) > 0);
-		i_assert(ret == -1);
-
-		if (stream->istream.v_offset != v_offset) {
-			/* some failure, we've broken it */
-			if (stream->istream.stream_errno != 0) {
-				i_error("lz4_istream.seek(%s) failed: %s",
-					i_stream_get_name(&stream->istream),
-					strerror(stream->istream.stream_errno));
-				i_stream_close(&stream->istream);
-			} else {
-				/* unexpected EOF. allow it since we may just
-				   want to check if there's anything.. */
-				i_assert(stream->istream.eof);
-			}
-		}
-	}
+	/* have to seek backwards - reset state and retry */
+	i_stream_lz4_reset(zstream);
+	if (!i_stream_nonseekable_try_seek(stream, v_offset))
+		i_unreached();
 
 	if (mark)
 		zstream->marked = TRUE;
diff --git a/src/lib-compression/istream-lzma.c b/src/lib-compression/istream-lzma.c
index a47f7b0c7d..3406890206 100644
--- a/src/lib-compression/istream-lzma.c
+++ b/src/lib-compression/istream-lzma.c
@@ -17,7 +17,6 @@ struct lzma_istream {
 
 	lzma_stream strm;
 	uoff_t eof_offset;
-	size_t high_pos;
 	struct stat last_parent_statbuf;
 
 	bool log_errors:1;
@@ -65,28 +64,10 @@ static ssize_t i_stream_lzma_read(struct istream_private *stream)
 
 	high_offset = stream->istream.v_offset + (stream->pos - stream->skip);
 	if (zstream->eof_offset == high_offset) {
-		i_assert(zstream->high_pos == 0 ||
-			 zstream->high_pos == stream->pos);
 		stream->istream.eof = TRUE;
 		return -1;
 	}
 
-	if (stream->pos < zstream->high_pos) {
-		/* we're here because we seeked back within the read buffer. */
-		ret = zstream->high_pos - stream->pos;
-		stream->pos = zstream->high_pos;
-		zstream->high_pos = 0;
-
-		if (zstream->eof_offset != (uoff_t)-1) {
-			high_offset = stream->istream.v_offset +
-				(stream->pos - stream->skip);
-			i_assert(zstream->eof_offset == high_offset);
-			stream->istream.eof = TRUE;
-		}
-		return ret;
-	}
-	zstream->high_pos = 0;
-
 	if (!zstream->marked) {
 		if (!i_stream_try_alloc(stream, CHUNK_SIZE, &out_size))
 			return -2; /* buffer full */
@@ -192,7 +173,6 @@ static void i_stream_lzma_reset(struct lzma_istream *zstream)
 	stream->parent_expected_offset = stream->parent_start_offset;
 	stream->skip = stream->pos = 0;
 	stream->istream.v_offset = 0;
-	zstream->high_pos = 0;
 
 	lzma_end(&zstream->strm);
 	i_stream_lzma_init(zstream);
@@ -202,56 +182,14 @@ static void
 i_stream_lzma_seek(struct istream_private *stream, uoff_t v_offset, bool mark)
 {
 	struct lzma_istream *zstream = (struct lzma_istream *) stream;
-	uoff_t start_offset = stream->istream.v_offset - stream->skip;
-
-	if (v_offset < start_offset) {
-		/* have to seek backwards */
-		i_stream_lzma_reset(zstream);
-		start_offset = 0;
-	} else if (zstream->high_pos != 0) {
-		stream->pos = zstream->high_pos;
-		zstream->high_pos = 0;
-	}
 
-	if (v_offset <= start_offset + stream->pos) {
-		/* seeking backwards within what's already cached */
-		stream->skip = v_offset - start_offset;
-		stream->istream.v_offset = v_offset;
-		zstream->high_pos = stream->pos;
-		stream->pos = stream->skip;
-	} else {
-		/* read and cache forward */
-		ssize_t ret;
-
-		do {
-			size_t avail = stream->pos - stream->skip;
-
-			if (stream->istream.v_offset + avail >= v_offset) {
-				i_stream_skip(&stream->istream,
-					      v_offset -
-					      stream->istream.v_offset);
-				ret = -1;
-				break;
-			}
-
-			i_stream_skip(&stream->istream, avail);
-		} while ((ret = i_stream_read(&stream->istream)) > 0);
-		i_assert(ret == -1);
-
-		if (stream->istream.v_offset != v_offset) {
-			/* some failure, we've broken it */
-			if (stream->istream.stream_errno != 0) {
-				i_error("lzma_istream.seek(%s) failed: %s",
-					i_stream_get_name(&stream->istream),
-					strerror(stream->istream.stream_errno));
-				i_stream_close(&stream->istream);
-			} else {
-				/* unexpected EOF. allow it since we may just
-				   want to check if there's anything.. */
-				i_assert(stream->istream.eof);
-			}
-		}
-	}
+	if (i_stream_nonseekable_try_seek(stream, v_offset))
+		return;
+
+	/* have to seek backwards - reset state and retry */
+	i_stream_lzma_reset(zstream);
+	if (!i_stream_nonseekable_try_seek(stream, v_offset))
+		i_unreached();
 
 	if (mark)
 		zstream->marked = TRUE;
diff --git a/src/lib-compression/istream-zlib.c b/src/lib-compression/istream-zlib.c
index 8adf57e22a..7de770c307 100644
--- a/src/lib-compression/istream-zlib.c
+++ b/src/lib-compression/istream-zlib.c
@@ -26,7 +26,7 @@ struct zlib_istream {
 
 	z_stream zs;
 	uoff_t eof_offset;
-	size_t prev_size, high_pos;
+	size_t prev_size;
 	uint32_t crc32;
 	struct stat last_parent_statbuf;
 
@@ -174,8 +174,6 @@ static ssize_t i_stream_zlib_read(struct istream_private *stream)
 
 	high_offset = stream->istream.v_offset + (stream->pos - stream->skip);
 	if (zstream->eof_offset == high_offset) {
-		i_assert(zstream->high_pos == 0 ||
-			 zstream->high_pos == stream->pos);
 		if (!zstream->trailer_read) {
 			do {
 				ret = i_stream_zlib_read_trailer(zstream);
@@ -226,21 +224,6 @@ static ssize_t i_stream_zlib_read(struct istream_private *stream)
 		zstream->header_read = TRUE;
 	}
 
-	if (stream->pos < zstream->high_pos) {
-		/* we're here because we seeked back within the read buffer. */
-		ret = zstream->high_pos - stream->pos;
-		stream->pos = zstream->high_pos;
-		zstream->high_pos = 0;
-		if (zstream->trailer_read) {
-			high_offset = stream->istream.v_offset +
-				(stream->pos - stream->skip);
-			i_assert(zstream->eof_offset == high_offset);
-			stream->istream.eof = TRUE;
-		}
-		return ret;
-	}
-	zstream->high_pos = 0;
-
 	if (!zstream->marked) {
 		if (!i_stream_try_alloc(stream, CHUNK_SIZE, &out_size))
 			return -2; /* buffer full */
@@ -354,7 +337,7 @@ static void i_stream_zlib_reset(struct zlib_istream *zstream)
 	stream->parent_expected_offset = stream->parent_start_offset;
 	stream->skip = stream->pos = 0;
 	stream->istream.v_offset = 0;
-	zstream->high_pos = 0;
+	stream->high_pos = 0;
 	zstream->prev_size = 0;
 
 	(void)inflateEnd(&zstream->zs);
@@ -365,56 +348,14 @@ static void
 i_stream_zlib_seek(struct istream_private *stream, uoff_t v_offset, bool mark)
 {
 	struct zlib_istream *zstream = (struct zlib_istream *) stream;
-	uoff_t start_offset = stream->istream.v_offset - stream->skip;
-
-	if (v_offset < start_offset) {
-		/* have to seek backwards */
-		i_stream_zlib_reset(zstream);
-		start_offset = 0;
-	} else if (zstream->high_pos != 0) {
-		stream->pos = zstream->high_pos;
-		zstream->high_pos = 0;
-	}
-
-	if (v_offset <= start_offset + stream->pos) {
-		/* seeking backwards within what's already cached */
-		stream->skip = v_offset - start_offset;
-		stream->istream.v_offset = v_offset;
-		zstream->high_pos = stream->pos;
-		stream->pos = stream->skip;
-	} else {
-		/* read and cache forward */
-		ssize_t ret;
 
-		do {
-			size_t avail = stream->pos - stream->skip;
-
-			if (stream->istream.v_offset + avail >= v_offset) {
-				i_stream_skip(&stream->istream,
-					      v_offset -
-					      stream->istream.v_offset);
-				ret = -1;
-				break;
-			}
+	if (i_stream_nonseekable_try_seek(stream, v_offset))
+		return;
 
-			i_stream_skip(&stream->istream, avail);
-		} while ((ret = i_stream_read(&stream->istream)) > 0);
-		i_assert(ret == -1);
-
-		if (stream->istream.v_offset != v_offset) {
-			/* some failure, we've broken it */
-			if (stream->istream.stream_errno != 0) {
-				i_error("zlib_istream.seek(%s) failed: %s",
-					i_stream_get_name(&stream->istream),
-					strerror(stream->istream.stream_errno));
-				i_stream_close(&stream->istream);
-			} else {
-				/* unexpected EOF. allow it since we may just
-				   want to check if there's anything.. */
-				i_assert(stream->istream.eof);
-			}
-		}
-	}
+	/* have to seek backwards - reset state and retry */
+	i_stream_zlib_reset(zstream);
+	if (!i_stream_nonseekable_try_seek(stream, v_offset))
+		i_unreached();
 
 	if (mark)
 		zstream->marked = TRUE;
diff --git a/src/lib-compression/test-compression.c b/src/lib-compression/test-compression.c
index 718d05b817..0f7df3d1fe 100644
--- a/src/lib-compression/test-compression.c
+++ b/src/lib-compression/test-compression.c
@@ -70,7 +70,6 @@ static void test_compression_handler(const struct compression_handler *handler)
 	sha1_result(&sha1, output_sha1);
 
 	/* read and uncompress the data */
-	sha1_init(&sha1);
 	file_input = i_stream_create_fd(fd, IO_BLOCK_SIZE);
 	input = handler->create_istream(file_input, FALSE);
 
@@ -80,17 +79,25 @@ static void test_compression_handler(const struct compression_handler *handler)
 	test_assert(i_stream_get_size(input, TRUE, &size) == 1);
 	test_assert(size == uncompressed_size);
 
-	while ((ret = i_stream_read_more(input, &data, &size)) > 0) {
-		sha1_loop(&sha1, data, size);
-		i_stream_skip(input, size);
+	sha1_init(&sha1);
+	for (bool seeked = FALSE;;) {
+		sha1_init(&sha1);
+		while ((ret = i_stream_read_more(input, &data, &size)) > 0) {
+			sha1_loop(&sha1, data, size);
+			i_stream_skip(input, size);
+		}
+		test_assert(ret == -1);
+		test_assert(input->stream_errno == 0);
+		sha1_result(&sha1, input_sha1);
+		test_assert(memcmp(input_sha1, output_sha1, sizeof(input_sha1)) == 0);
+		if (seeked)
+			break;
+		seeked = TRUE;
+		i_stream_seek(input, 0);
 	}
-	test_assert(ret == -1);
-	test_assert(input->stream_errno == 0);
 	i_stream_destroy(&input);
 	i_stream_destroy(&file_input);
-	sha1_result(&sha1, input_sha1);
 
-	test_assert(memcmp(input_sha1, output_sha1, sizeof(input_sha1)) == 0);
 	i_unlink(path);
 	i_close_fd(&fd);