Mercurial > dovecot > original-hg > dovecot-1.2

changeset 8196:40a07553606c HEAD

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Input streams: Improved error handling and added more asserts.
author Timo Sirainen <tss@iki.fi>
date Sat, 13 Sep 2008 13:06:06 +0300
parents 413c70b0ae1d
children 264f493b1575
files src/lib-mail/istream-header-filter.c src/lib-storage/index/mbox/istream-raw-mbox.c src/lib/istream-concat.c src/lib/istream-crlf.c src/lib/istream-file.c src/lib/istream-limit.c src/lib/istream-mmap.c src/lib/istream-seekable.c src/lib/istream-tee.c src/lib/istream.c src/plugins/zlib/istream-zlib.c
diffstat 11 files changed, 62 insertions(+), 35 deletions(-) [+]
line wrap: on
line diff
--- a/src/lib-mail/istream-header-filter.c	Sat Sep 13 13:02:13 2008 +0300
+++ b/src/lib-mail/istream-header-filter.c	Sat Sep 13 13:06:06 2008 +0300
@@ -92,6 +92,7 @@
 
 	mstream->istream.buffer = buffer_get_data(mstream->hdr_buf, &pos);
 	ret = (ssize_t)(pos - mstream->istream.pos - mstream->istream.skip);
+	i_assert(ret > 0);
 	mstream->istream.pos = pos;
 	return ret;
 }
@@ -244,8 +245,10 @@
 	ret = (ssize_t)(pos - mstream->istream.pos - mstream->istream.skip);
 	mstream->istream.pos = pos;
 
-	if (hdr_ret == 0)
+	if (hdr_ret == 0) {
+		i_assert(ret > 0);
 		return ret;
+	}
 
 	if (hdr == NULL) {
 		/* finished */
--- a/src/lib-storage/index/mbox/istream-raw-mbox.c	Sat Sep 13 13:02:13 2008 +0300
+++ b/src/lib-storage/index/mbox/istream-raw-mbox.c	Sat Sep 13 13:06:06 2008 +0300
@@ -61,7 +61,11 @@
 
 	while ((p = memchr(buf+skip, '\n', pos-skip)) == NULL) {
 		if (i_stream_read(rstream->istream.parent) < 0) {
-			/* EOF - shouldn't happen */
+			/* EOF shouldn't happen */
+			rstream->istream.istream.eof =
+				rstream->istream.parent->eof;
+			rstream->istream.istream.stream_errno =
+				rstream->istream.parent->stream_errno;
 			return -1;
 		}
 		buf = i_stream_get_data(rstream->istream.parent, &pos);
@@ -79,6 +83,7 @@
 	    mbox_from_parse(buf+5, pos-5, &received_time, &tz, &sender) < 0) {
 		/* broken From - should happen only at beginning of
 		   file if this isn't a mbox.. */
+		rstream->istream.istream.stream_errno = EBADMSG;
 		return -1;
 	}
 
@@ -302,6 +307,7 @@
 			rstream->hdr_offset + rstream->mail_size);
 		rstream->eof = TRUE;
 		rstream->corrupted = TRUE;
+		rstream->istream.istream.stream_errno = EBADMSG;
 		return -1;
 	}
 
--- a/src/lib/istream-concat.c	Sat Sep 13 13:02:13 2008 +0300
+++ b/src/lib/istream-concat.c	Sat Sep 13 13:06:06 2008 +0300
@@ -138,6 +138,7 @@
 		}
 
 		stream->istream.eof = cstream->cur_input->eof && last_stream;
+		i_assert(ret != -1 || stream->istream.eof);
 		data = i_stream_get_data(cstream->cur_input, &pos);
 	}
 
@@ -198,7 +199,6 @@
 {
 	struct concat_istream *cstream = (struct concat_istream *)stream;
 
-	stream->istream.stream_errno = 0;
 	stream->istream.v_offset = v_offset;
 	stream->skip = stream->pos = 0;
 	cstream->prev_size = 0;
--- a/src/lib/istream-crlf.c	Sat Sep 13 13:02:13 2008 +0300
+++ b/src/lib/istream-crlf.c	Sat Sep 13 13:06:06 2008 +0300
@@ -87,7 +87,7 @@
 	i_stream_skip(stream->parent, i);
 
 	ret = dest - stream->pos;
-	i_assert(ret != 0);
+	i_assert(ret > 0);
 	stream->pos = dest;
 	return ret;
 }
@@ -147,6 +147,7 @@
 		i_assert(cstream->pending_cr && size == 1);
 		return i_stream_crlf_read_lf(stream);
 	}
+	i_assert(ret > 0);
 	stream->pos = dest;
 	return ret;
 }
--- a/src/lib/istream-file.c	Sat Sep 13 13:02:13 2008 +0300
+++ b/src/lib/istream-file.c	Sat Sep 13 13:06:06 2008 +0300
@@ -46,7 +46,6 @@
 	size_t size;
 	ssize_t ret;
 
-	stream->istream.stream_errno = 0;
 	if (!i_stream_get_buffer_space(stream, 1, &size))
 		return -2;
 
@@ -78,7 +77,7 @@
 			i_assert(!stream->istream.blocking);
 			ret = 0;
 		} else {
-			stream->istream.eof = TRUE;
+			i_assert(errno != 0);
 			stream->istream.stream_errno = errno;
 			return -1;
 		}
@@ -101,6 +100,7 @@
 
 	stream->pos += ret;
 	i_assert(ret != 0 || !fstream->file);
+	i_assert(ret != -1);
 	return ret;
 }
 
@@ -117,7 +117,6 @@
 		fstream->skip_left += v_offset - stream->istream.v_offset;
 	}
 
-	stream->istream.stream_errno = 0;
 	stream->istream.v_offset = v_offset;
 	stream->skip = stream->pos = 0;
 	fstream->seen_eof = FALSE;
--- a/src/lib/istream-limit.c	Sat Sep 13 13:02:13 2008 +0300
+++ b/src/lib/istream-limit.c	Sat Sep 13 13:06:06 2008 +0300
@@ -80,6 +80,8 @@
 	ret = pos > stream->pos ? (ssize_t)(pos - stream->pos) :
 		(ret == 0 ? 0 : -1);
 	stream->pos = pos;
+	i_assert(ret != -1 || stream->istream.eof ||
+		 stream->istream.stream_errno != 0);
 	return ret;
 }
 
@@ -90,7 +92,6 @@
 
 	i_assert(v_offset <= lstream->v_size);
 
-	stream->istream.stream_errno = 0;
 	stream->istream.v_offset = v_offset;
 	stream->skip = stream->pos = 0;
 }
--- a/src/lib/istream-mmap.c	Sat Sep 13 13:02:13 2008 +0300
+++ b/src/lib/istream-mmap.c	Sat Sep 13 13:06:06 2008 +0300
@@ -66,8 +66,6 @@
 	size_t aligned_skip;
 	uoff_t top;
 
-	stream->istream.stream_errno = 0;
-
 	if (stream->pos < stream->buffer_size) {
 		/* more bytes available without needing to mmap() */
 		stream->pos = stream->buffer_size;
@@ -108,6 +106,7 @@
 			mmap(NULL, stream->buffer_size, PROT_READ, MAP_PRIVATE,
 			     stream->fd, mstream->mmap_offset);
 		if (mstream->mmap_base == MAP_FAILED) {
+			i_assert(errno != 0);
 			stream->istream.stream_errno = errno;
 			mstream->mmap_base = NULL;
 			stream->buffer = NULL;
@@ -126,7 +125,7 @@
 	}
 
 	stream->pos = stream->buffer_size;
-	i_assert(stream->pos - stream->skip != 0);
+	i_assert(stream->pos - stream->skip > 0);
 	return stream->pos - stream->skip;
 }
 
--- a/src/lib/istream-seekable.c	Sat Sep 13 13:02:13 2008 +0300
+++ b/src/lib/istream-seekable.c	Sat Sep 13 13:06:06 2008 +0300
@@ -2,6 +2,7 @@
 
 #include "lib.h"
 #include "buffer.h"
+#include "close-keep-errno.h"
 #include "hex-binary.h"
 #include "randgen.h"
 #include "write-full.h"
@@ -107,14 +108,14 @@
 	if (unlink(path) < 0) {
 		/* shouldn't happen.. */
 		i_error("unlink(%s) failed: %m", path);
-		(void)close(fd);
+		close_keep_errno(fd);
 		return -1;
 	}
 
 	/* copy our currently read buffer to it */
 	if (write_full(fd, sstream->buffer->data, sstream->buffer->used) < 0) {
 		i_error("write_full(%s) failed: %m", path);
-		(void)close(fd);
+		close_keep_errno(fd);
 		return -1;
 	}
 	sstream->write_peak = sstream->buffer->used;
@@ -139,10 +140,10 @@
 
 	while ((ret = i_stream_read(sstream->cur_input)) < 0) {
 		if (!sstream->cur_input->eof) {
-			/* error */
+			/* full / error */
 			sstream->istream.istream.stream_errno =
 				sstream->cur_input->stream_errno;
-			return -1;
+			return ret;
 		}
 
 		/* go to next stream */
@@ -161,7 +162,7 @@
 	return ret;
 }
 
-static bool read_from_buffer(struct seekable_istream *sstream, ssize_t *ret)
+static bool read_from_buffer(struct seekable_istream *sstream, ssize_t *ret_r)
 {
 	struct istream_private *stream = &sstream->istream;
 	const unsigned char *data;
@@ -174,8 +175,8 @@
 			return FALSE;
 
 		/* read more to buffer */
-		*ret = read_more(sstream);
-		if (*ret <= 0)
+		*ret_r = read_more(sstream);
+		if (*ret_r <= 0)
 			return TRUE;
 
 		/* we should have more now. */
@@ -188,7 +189,8 @@
 	stream->buffer = CONST_PTR_OFFSET(sstream->buffer->data, offset);
 	pos = sstream->buffer->used - offset;
 
-	*ret = pos - stream->pos;
+	*ret_r = pos - stream->pos;
+	i_assert(*ret_r > 0);
 	stream->pos = pos;
 	return TRUE;
 }
@@ -210,6 +212,8 @@
 
 		/* copy everything to temp file and use it as the stream */
 		if (copy_to_temp_file(sstream) < 0) {
+			i_assert(errno != 0);
+			stream->istream.stream_errno = errno;
 			i_stream_close(&stream->istream);
 			return -1;
 		}
@@ -225,6 +229,8 @@
 		/* save to our file */
 		data = i_stream_get_data(sstream->cur_input, &size);
 		if (write_full(sstream->fd, data, size) < 0) {
+			i_assert(errno != 0);
+			stream->istream.stream_errno = errno;
 			i_error("write_full(%s...) failed: %m",
 				sstream->temp_prefix);
 			i_stream_close(&stream->istream);
@@ -255,7 +261,6 @@
 static void i_stream_seekable_seek(struct istream_private *stream,
 				   uoff_t v_offset, bool mark ATTR_UNUSED)
 {
-	stream->istream.stream_errno = 0;
 	stream->istream.v_offset = v_offset;
 	stream->skip = stream->pos = 0;
 }
--- a/src/lib/istream-tee.c	Sat Sep 13 13:02:13 2008 +0300
+++ b/src/lib/istream-tee.c	Sat Sep 13 13:06:06 2008 +0300
@@ -141,6 +141,7 @@
 
 	i_assert(stream->buffer == data);
 	ret = size - stream->pos;
+	i_assert(ret > 0);
 	stream->pos = size;
 	return ret;
 }
--- a/src/lib/istream.c	Sat Sep 13 13:02:13 2008 +0300
+++ b/src/lib/istream.c	Sat Sep 13 13:06:06 2008 +0300
@@ -51,7 +51,7 @@
 	stream->closed = TRUE;
 
 	if (stream->stream_errno == 0)
-		stream->stream_errno = ECONNRESET;
+		stream->stream_errno = EBADFD;
 }
 
 void i_stream_set_max_buffer_size(struct istream *stream, size_t max_size)
@@ -62,12 +62,27 @@
 ssize_t i_stream_read(struct istream *stream)
 {
 	struct istream_private *_stream = stream->real_stream;
+	ssize_t ret;
 
 	if (unlikely(stream->closed))
 		return -1;
 
 	stream->eof = FALSE;
-	return _stream->read(_stream);
+	stream->stream_errno = 0;
+
+	ret = _stream->read(_stream);
+	if (ret < 0) {
+		if (stream->stream_errno != 0) {
+			/* error handling should be easier if we now just
+			   assume the stream is now at EOF */
+			stream->eof = TRUE;
+		} else {
+			i_assert(stream->eof);
+		}
+	} else {
+		i_assert(ret != 0 || !stream->blocking);
+	}
+	return ret;
 }
 
 void i_stream_skip(struct istream *stream, uoff_t count)
@@ -91,6 +106,7 @@
 	if (unlikely(stream->closed))
 		return;
 
+	stream->stream_errno = 0;
 	_stream->seek(_stream, stream->v_offset + count, FALSE);
 }
 
@@ -295,11 +311,14 @@
 		i_assert(!stream->blocking);
 		return 0;
 	}
-	if (read_more && stream->eof) {
-		/* we read at least some new data */
-		return 0;
+	if (stream->eof) {
+		if (read_more) {
+			/* we read at least some new data */
+			return 0;
+		}
+	} else {
+		i_assert(stream->stream_errno != 0);
 	}
-	i_assert(stream->stream_errno != 0);
 	return -1;
 }
 
--- a/src/plugins/zlib/istream-zlib.c	Sat Sep 13 13:02:13 2008 +0300
+++ b/src/plugins/zlib/istream-zlib.c	Sat Sep 13 13:06:06 2008 +0300
@@ -55,11 +55,6 @@
 	size_t size;
 	int ret;
 
-	if (stream->istream.closed)
-		return -1;
-
-	stream->istream.stream_errno = 0;
-
 	if (stream->pos == stream->buffer_size) {
 		if (!zstream->marked && stream->skip > 0) {
 			/* don't try to keep anything cached if we don't
@@ -104,7 +99,7 @@
 			i_assert(!stream->istream.blocking);
 			ret = 0;
 		} else {
-			stream->istream.eof = TRUE;
+			i_assert(errno != 0);
 			stream->istream.stream_errno = errno;
 			return -1;
 		}
@@ -112,7 +107,7 @@
 
 	zstream->seek_offset += ret;
 	stream->pos += ret;
-	i_assert(ret != 0);
+	i_assert(ret > 0);
 	return ret;
 }
 
@@ -122,8 +117,6 @@
 	struct zlib_istream *zstream = (struct zlib_istream *) stream;
 	uoff_t start_offset = stream->istream.v_offset - stream->skip;
 
-	stream->istream.stream_errno = 0;
-
 #ifndef HAVE_GZSEEK
 	if (v_offset < start_offset) {
 		/* need to reopen, but since closing the file closes the