Mercurial > dovecot > core-2.2

changeset 20231:f0b26fed18b6

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
lib: Fixed max_buffer_size handling in istream-chain The default max_buffer_size=256 was wrong in all situations. We're now assuming that the underlying istreams' max_buffer_size is always correct. While gluing together two streams we're now allocating enough memory to hold all of the wanted data (instead of assert-crashing as could have happened earlier). This means that the max memory usage is actually the two streams' max_buffer_size summed together. Ideally this would be fixed to limit the max_buffer_size to maximum of the two, but that would require further changes.
author Timo Sirainen <timo.sirainen@dovecot.fi>
date Tue, 31 May 2016 22:19:37 +0300
parents 0e31af41eb37
children 2ae2e3beaa56
files src/lib/istream-chain.c
diffstat 1 files changed, 13 insertions(+), 25 deletions(-) [+]
line wrap: on
line diff
--- a/src/lib/istream-chain.c	Wed Jun 01 08:43:23 2016 +0300
+++ b/src/lib/istream-chain.c	Tue May 31 22:19:37 2016 +0300
@@ -24,6 +24,7 @@
 	struct istream_private istream;
 
 	size_t prev_stream_left, prev_skip;
+	bool have_explicit_max_buffer_size;
 	
 	struct istream_chain chain;
 };
@@ -45,10 +46,9 @@
 		i_stream_ref(stream);	
 
 	if (chain->head == NULL && stream != NULL) {
-		if (chain->stream->istream.max_buffer_size == 0) {
-			chain->stream->istream.max_buffer_size =
-				stream->real_stream->max_buffer_size;
-		} else {
+		struct chain_istream *cstream = (struct chain_istream *)stream;
+
+		if (cstream->have_explicit_max_buffer_size) {
 			i_stream_set_max_buffer_size(stream,
 				chain->stream->istream.max_buffer_size);
 		}
@@ -77,6 +77,7 @@
 	struct chain_istream *cstream = (struct chain_istream *)stream;
 	struct istream_chain_link *link = cstream->chain.head;
 
+	cstream->have_explicit_max_buffer_size = TRUE;
 	cstream->istream.max_buffer_size = max_size;
 	while (link != NULL) {
 		if (link->stream != NULL)
@@ -106,7 +107,7 @@
 	struct istream_chain_link *link = cstream->chain.head;
 	struct istream *prev_input;
 	const unsigned char *data;
-	size_t data_size, size, cur_data_pos;
+	size_t data_size, cur_data_pos;
 
 	i_assert(link != NULL && link->stream != NULL);
 	i_assert(link->stream->eof);
@@ -137,17 +138,12 @@
 		cstream->prev_stream_left = 0;
 	}
 
-	/* we already verified that the data size is less than the
-	   maximum buffer size */
 	if (data_size > 0) {
-		if (!i_stream_try_alloc(&cstream->istream, data_size, &size))
-			i_unreached();
-		i_assert(size >= data_size);
+		memcpy(i_stream_alloc(&cstream->istream, data_size),
+		       data, data_size);
+		cstream->istream.pos += data_size;
+		cstream->prev_stream_left += data_size;
 	}
-	memcpy(cstream->istream.w_buffer + cstream->istream.pos,
-	       data, data_size);
-	cstream->istream.pos += data_size;
-	cstream->prev_stream_left += data_size;
 
 	i_stream_skip(prev_input, i_stream_get_data_size(prev_input));
 	i_stream_unref(&prev_input);
@@ -190,7 +186,7 @@
 	struct chain_istream *cstream = (struct chain_istream *)stream;
 	struct istream_chain_link *link = cstream->chain.head;
 	const unsigned char *data;
-	size_t size, data_size, cur_data_pos, new_pos;
+	size_t data_size, cur_data_pos, new_pos;
 	size_t new_bytes_count;
 	ssize_t ret;
 
@@ -251,16 +247,9 @@
 		   new data with it. */
 		i_assert(data_size > cur_data_pos);
 		new_bytes_count = data_size - cur_data_pos;
-		if (!i_stream_try_alloc(stream, new_bytes_count, &size)) {
-			stream->buffer = stream->w_buffer;
-			return -2;
-		}
+		memcpy(i_stream_alloc(stream, new_bytes_count),
+		       data + cur_data_pos, new_bytes_count);
 		stream->buffer = stream->w_buffer;
-
-		if (new_bytes_count > size)
-			new_bytes_count = size;
-		memcpy(stream->w_buffer + stream->pos,
-		       data + cur_data_pos, new_bytes_count);
 		new_pos = stream->pos + new_bytes_count;
 	}
 
@@ -295,7 +284,6 @@
 
 	cstream = i_new(struct chain_istream, 1);
 	cstream->chain.stream = cstream;
-	cstream->istream.max_buffer_size = 256;
 
 	cstream->istream.iostream.close = i_stream_chain_close;
 	cstream->istream.iostream.destroy = i_stream_chain_destroy;