dovecot-2.1: zlib: zlib/bzlib ostreams no longer assert-crash if...

dovecot at dovecot.org dovecot at dovecot.org
Sun Jan 29 01:37:50 EET 2012


details:   http://hg.dovecot.org/dovecot-2.1/rev/3c0bd1fd035b
changeset: 14032:3c0bd1fd035b
user:      Timo Sirainen <tss at iki.fi>
date:      Sun Jan 29 01:37:44 2012 +0200
description:
zlib: zlib/bzlib ostreams no longer assert-crash if parent stream becomes full.
This fixes assert-crashes with IMAP COMPRESS extension.

diffstat:

 src/plugins/zlib/ostream-bzlib.c |  84 +++++++++++++++++++++++++++++---------
 src/plugins/zlib/ostream-zlib.c  |  86 ++++++++++++++++++++++++++++++---------
 2 files changed, 129 insertions(+), 41 deletions(-)

diffs (truncated from 306 to 300 lines):

diff -r 397224940894 -r 3c0bd1fd035b src/plugins/zlib/ostream-bzlib.c
--- a/src/plugins/zlib/ostream-bzlib.c	Sun Jan 29 00:48:43 2012 +0200
+++ b/src/plugins/zlib/ostream-bzlib.c	Sun Jan 29 01:37:44 2012 +0200
@@ -15,7 +15,7 @@
 	bz_stream zs;
 
 	char outbuf[CHUNK_SIZE];
-	struct ostream *output;
+	unsigned int outbuf_offset, outbuf_used;
 
 	unsigned int flushed:1;
 };
@@ -28,26 +28,55 @@
 	(void)BZ2_bzCompressEnd(&zstream->zs);
 }
 
+static int o_stream_zlib_send_outbuf(struct bzlib_ostream *zstream)
+{
+	ssize_t ret;
+	size_t size;
+
+	if (zstream->outbuf_used == 0)
+		return 1;
+
+	size = zstream->outbuf_used - zstream->outbuf_offset;
+	i_assert(size > 0);
+	ret = o_stream_send(zstream->ostream.parent,
+			    zstream->outbuf + zstream->outbuf_offset, size);
+	if (ret < 0) {
+		o_stream_copy_error_from_parent(&zstream->ostream);
+		return -1;
+	}
+	if ((size_t)ret != size) {
+		zstream->outbuf_offset += ret;
+		return 0;
+	}
+	zstream->outbuf_offset = 0;
+	zstream->outbuf_used = 0;
+	return 1;
+}
+
 static ssize_t
 o_stream_bzlib_send_chunk(struct bzlib_ostream *zstream,
 			  const void *data, size_t size)
 {
 	bz_stream *zs = &zstream->zs;
-	ssize_t ret;
+	int ret;
+
+	i_assert(zstream->outbuf_used == 0);
 
 	zs->next_in = (void *)data;
 	zs->avail_in = size;
 	while (zs->avail_in > 0) {
 		if (zs->avail_out == 0) {
+			/* previous block was compressed. send it and start
+			   compression for a new block. */
 			zs->next_out = zstream->outbuf;
 			zs->avail_out = sizeof(zstream->outbuf);
 
-			ret = o_stream_send(zstream->ostream.parent,
-					    zstream->outbuf,
-					    sizeof(zstream->outbuf));
-			if (ret != (ssize_t)sizeof(zstream->outbuf)) {
-				o_stream_copy_error_from_parent(&zstream->ostream);
+			zstream->outbuf_used = sizeof(zstream->outbuf);
+			if ((ret = o_stream_zlib_send_outbuf(zstream)) < 0)
 				return -1;
+			if (ret == 0) {
+				/* parent stream's buffer full */
+				break;
 			}
 		}
 
@@ -58,8 +87,10 @@
 			i_unreached();
 		}
 	}
+	size -= zs->avail_in;
+
 	zstream->flushed = FALSE;
-	return 0;
+	return size;
 }
 
 static int o_stream_bzlib_send_flush(struct bzlib_ostream *zstream)
@@ -79,18 +110,19 @@
 	if (zstream->flushed)
 		return 0;
 
+	if ((ret = o_stream_zlib_send_outbuf(zstream)) <= 0)
+		return ret;
+
+	i_assert(zstream->outbuf_used == 0);
 	do {
 		len = sizeof(zstream->outbuf) - zs->avail_out;
 		if (len != 0) {
 			zs->next_out = zstream->outbuf;
 			zs->avail_out = sizeof(zstream->outbuf);
 
-			ret = o_stream_send(zstream->ostream.parent,
-					    zstream->outbuf, len);
-			if (ret != (int)len) {
-				o_stream_copy_error_from_parent(&zstream->ostream);
-				return -1;
-			}
+			zstream->outbuf_used = len;
+			if ((ret = o_stream_zlib_send_outbuf(zstream)) <= 0)
+				return ret;
 			if (done)
 				break;
 		}
@@ -130,17 +162,29 @@
 		    const struct const_iovec *iov, unsigned int iov_count)
 {
 	struct bzlib_ostream *zstream = (struct bzlib_ostream *)stream;
-	ssize_t bytes = 0;
+	ssize_t ret, bytes = 0;
 	unsigned int i;
 
-	for (i = 0; i < iov_count; i++) {
-		if (o_stream_bzlib_send_chunk(zstream, iov[i].iov_base,
-					      iov[i].iov_len) < 0)
-			return -1;
-		bytes += iov[i].iov_len;
+	if ((ret = o_stream_zlib_send_outbuf(zstream)) <= 0) {
+		/* error / we still couldn't flush existing data to
+		   parent stream. */
+		return ret;
 	}
 
+	for (i = 0; i < iov_count; i++) {
+		ret = o_stream_bzlib_send_chunk(zstream, iov[i].iov_base,
+						iov[i].iov_len);
+		if (ret < 0)
+			return -1;
+		bytes += ret;
+		if ((size_t)ret != iov[i].iov_len)
+			break;
+	}
 	stream->ostream.offset += bytes;
+
+	/* avail_in!=0 check is used to detect errors. if it's non-zero here
+	   it simply means we didn't send all the data */
+	zstream->zs.avail_in = 0;
 	return bytes;
 }
 
diff -r 397224940894 -r 3c0bd1fd035b src/plugins/zlib/ostream-zlib.c
--- a/src/plugins/zlib/ostream-zlib.c	Sun Jan 29 00:48:43 2012 +0200
+++ b/src/plugins/zlib/ostream-zlib.c	Sun Jan 29 01:37:44 2012 +0200
@@ -18,6 +18,7 @@
 
 	unsigned char gz_header[10];
 	unsigned char outbuf[CHUNK_SIZE];
+	unsigned int outbuf_offset, outbuf_used;
 
 	uint32_t crc, bytes32;
 
@@ -77,13 +78,39 @@
 	return 0;
 }
 
-static int
+static int o_stream_zlib_send_outbuf(struct zlib_ostream *zstream)
+{
+	ssize_t ret;
+	size_t size;
+
+	if (zstream->outbuf_used == 0)
+		return 1;
+
+	size = zstream->outbuf_used - zstream->outbuf_offset;
+	i_assert(size > 0);
+	ret = o_stream_send(zstream->ostream.parent,
+			    zstream->outbuf + zstream->outbuf_offset, size);
+	if (ret < 0) {
+		o_stream_copy_error_from_parent(&zstream->ostream);
+		return -1;
+	}
+	if ((size_t)ret != size) {
+		zstream->outbuf_offset += ret;
+		return 0;
+	}
+	zstream->outbuf_offset = 0;
+	zstream->outbuf_used = 0;
+	return 1;
+}
+
+static ssize_t
 o_stream_zlib_send_chunk(struct zlib_ostream *zstream,
 			 const void *data, size_t size)
 {
 	z_stream *zs = &zstream->zs;
-	ssize_t ret;
-	int flush;
+	int ret, flush;
+
+	i_assert(zstream->outbuf_used == 0);
 
 	flush = zstream->ostream.corked || zstream->gz ?
 		Z_NO_FLUSH : Z_SYNC_FLUSH;
@@ -95,15 +122,17 @@
 	zs->avail_in = size;
 	while (zs->avail_in > 0) {
 		if (zs->avail_out == 0) {
+			/* previous block was compressed. send it and start
+			   compression for a new block. */
 			zs->next_out = zstream->outbuf;
 			zs->avail_out = sizeof(zstream->outbuf);
 
-			ret = o_stream_send(zstream->ostream.parent,
-					    zstream->outbuf,
-					    sizeof(zstream->outbuf));
-			if (ret != (ssize_t)sizeof(zstream->outbuf)) {
-				o_stream_copy_error_from_parent(&zstream->ostream);
+			zstream->outbuf_used = sizeof(zstream->outbuf);
+			if ((ret = o_stream_zlib_send_outbuf(zstream)) < 0)
 				return -1;
+			if (ret == 0) {
+				/* parent stream's buffer full */
+				break;
 			}
 		}
 
@@ -115,11 +144,13 @@
 			i_unreached();
 		}
 	}
+	size -= zs->avail_in;
+
 	zstream->crc = crc32_data_more(zstream->crc, data, size);
 	zstream->bytes32 += size;
-	zstream->flushed = flush == Z_SYNC_FLUSH &&
+	zstream->flushed = flush == Z_SYNC_FLUSH && zs->avail_in == 0 &&
 		zs->avail_out == sizeof(zstream->outbuf);
-	return 0;
+	return size;
 }
 
 static int o_stream_zlib_send_flush(struct zlib_ostream *zstream)
@@ -141,18 +172,19 @@
 	if (!zstream->header_sent)
 		o_stream_zlib_send_gz_header(zstream);
 
+	if ((ret = o_stream_zlib_send_outbuf(zstream)) <= 0)
+		return ret;
+
+	i_assert(zstream->outbuf_used == 0);
 	do {
 		len = sizeof(zstream->outbuf) - zs->avail_out;
 		if (len != 0) {
 			zs->next_out = zstream->outbuf;
 			zs->avail_out = sizeof(zstream->outbuf);
 
-			ret = o_stream_send(zstream->ostream.parent,
-					    zstream->outbuf, len);
-			if (ret != (int)len) {
-				o_stream_copy_error_from_parent(&zstream->ostream);
-				return -1;
-			}
+			zstream->outbuf_used = len;
+			if ((ret = o_stream_zlib_send_outbuf(zstream)) <= 0)
+				return ret;
 			if (done)
 				break;
 		}
@@ -194,21 +226,33 @@
 		    const struct const_iovec *iov, unsigned int iov_count)
 {
 	struct zlib_ostream *zstream = (struct zlib_ostream *)stream;
-	ssize_t bytes = 0;
+	ssize_t ret, bytes = 0;
 	unsigned int i;
 
+	if ((ret = o_stream_zlib_send_outbuf(zstream)) <= 0) {
+		/* error / we still couldn't flush existing data to
+		   parent stream. */
+		return ret;
+	}
+
 	for (i = 0; i < iov_count; i++) {
-		if (o_stream_zlib_send_chunk(zstream, iov[i].iov_base,
-					     iov[i].iov_len) < 0)
+		ret = o_stream_zlib_send_chunk(zstream, iov[i].iov_base,
+					       iov[i].iov_len);
+		if (ret < 0)
 			return -1;
-		bytes += iov[i].iov_len;
+		bytes += ret;
+		if ((size_t)ret != iov[i].iov_len)
+			break;
 	}
 	stream->ostream.offset += bytes;
 
-	if (!zstream->ostream.corked) {
+	if (!zstream->ostream.corked && i == iov_count) {
 		if (o_stream_zlib_send_flush(zstream) < 0)
 			return -1;
 	}


More information about the dovecot-cvs mailing list