From 2a6d2a1e4be261e071f5ddb95be183e172643916 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Wed, 10 Mar 2010 22:16:14 -0500
Subject: [PATCH 1/6] Revise evbuffer to add last_with_data

This is the first patch in a series to replace previous_to_last with
last_with_data.  Currently, we can only use two partially empty chains
at the end of an evbuffer, so if we have one with 511 bytes free, and
another with 512 bytes free, and we try to do a 1024 byte read, we
can't just stick another chain on the end: we need to reallocate the
last one.  That's stupid and inefficient.

Instead, this patch adds a last_with_data pointer to eventually
replace previous_to_last.  Instead of pointing to the penultimated
chain (if any) as previous_to_last does, last_with_data points to the
last chain that has any data in it, if any.  If all chains are empty,
last_with_data points to the first chain.  If there are no chains,
last_with_data is NULL.

The next step is to start using last_with_data everywhere that we
currently use previous_to_last.  When that's done, we can remove
previous_to_last and the code that maintains it.
---
 buffer.c              | 59 +++++++++++++++++++++++++++++++++++++++++--
 evbuffer-internal.h   |  4 +++
 test/regress_buffer.c | 35 ++++++++++++++++++++++++-
 3 files changed, 95 insertions(+), 3 deletions(-)

diff --git a/buffer.c b/buffer.c
index 3e93a035..82835975 100644
--- a/buffer.c
+++ b/buffer.c
@@ -220,9 +220,12 @@ evbuffer_chain_insert(struct evbuffer *buf, struct evbuffer_chain *chain)
 	if (buf->first == NULL) {
 		buf->first = buf->last = chain;
 		buf->previous_to_last = NULL;
+		buf->last_with_data = chain;
 	} else {
 		/* the last chain is empty so we can just drop it */
 		if (buf->last->off == 0 && !CHAIN_PINNED(buf->last)) {
+			if (buf->last_with_data == buf->last)
+				buf->last_with_data = chain;
 			evbuffer_chain_free(buf->last);
 			buf->previous_to_last->next = chain;
 			buf->last = chain;
@@ -231,6 +234,8 @@ evbuffer_chain_insert(struct evbuffer *buf, struct evbuffer_chain *chain)
 			buf->last->next = chain;
 			buf->last = chain;
 		}
+		if (chain->off)
+			buf->last_with_data = chain;
 	}
 
 	buf->total_len += chain->off;
@@ -531,6 +536,22 @@ done:
 
 }
 
+static int
+advance_last_with_data(struct evbuffer *buf)
+{
+	int n = 0;
+	ASSERT_EVBUFFER_LOCKED(buf);
+
+	if (!buf->last_with_data)
+		return 0;
+
+	while (buf->last_with_data->next && buf->last_with_data->next->off) {
+		buf->last_with_data = buf->last_with_data->next;
+		++n;
+	}
+	return n;
+}
+
 int
 evbuffer_commit_space(struct evbuffer *buf,
     struct evbuffer_iovec *vec, int n_vecs)
@@ -578,6 +599,7 @@ evbuffer_commit_space(struct evbuffer *buf,
 	buf->total_len += added;
 	buf->n_add_for_cb += added;
 	result = 0;
+	advance_last_with_data(buf);
 	evbuffer_invoke_callbacks(buf);
 
 done:
@@ -590,6 +612,7 @@ done:
 		(dst)->first = NULL;		\
 		(dst)->last = NULL;		\
 		(dst)->previous_to_last = NULL; \
+		(dst)->last_with_data = NULL;	\
 		(dst)->total_len = 0;		\
 	} while (0)
 
@@ -598,6 +621,7 @@ done:
 		ASSERT_EVBUFFER_LOCKED(src);			   \
 		(dst)->first = (src)->first;			   \
 		(dst)->previous_to_last = (src)->previous_to_last; \
+		(dst)->last_with_data = (src)->last_with_data;	   \
 		(dst)->last = (src)->last;			   \
 		(dst)->total_len = (src)->total_len;		   \
 	} while (0)
@@ -608,6 +632,8 @@ done:
 		(dst)->last->next = (src)->first;			\
 		(dst)->previous_to_last = (src)->previous_to_last ?	\
 		    (src)->previous_to_last : (dst)->last;		\
+		if ((src)->last_with_data)				\
+			(dst)->last_with_data = (src)->last_with_data;	\
 		(dst)->last = (src)->last;				\
 		(dst)->total_len += (src)->total_len;			\
 	} while (0)
@@ -620,9 +646,10 @@ done:
 		(dst)->total_len += (src)->total_len;		   \
 		if ((dst)->previous_to_last == NULL)		   \
 			(dst)->previous_to_last = (src)->last;	   \
+		if ((dst)->last_with_data == NULL)		   \
+			(dst)->last_with_data = (src)->last_with_data; \
 	} while (0)
 
-
 int
 evbuffer_add_buffer(struct evbuffer *outbuf, struct evbuffer *inbuf)
 {
@@ -734,6 +761,8 @@ evbuffer_drain(struct evbuffer *buf, size_t len)
 		for (chain = buf->first; len >= chain->off; chain = next) {
 			next = chain->next;
 			len -= chain->off;
+			if (chain == buf->last_with_data)
+				buf->last_with_data = next;
 
 			if (len == 0 && CHAIN_PINNED_R(chain))
 				break;
@@ -789,6 +818,9 @@ evbuffer_remove(struct evbuffer *buf, void *data_out, size_t datlen)
 		data += chain->off;
 		datlen -= chain->off;
 
+		if (chain == buf->last_with_data)
+			buf->last_with_data = chain->next;
+
 		tmp = chain;
 		chain = chain->next;
 		evbuffer_chain_free(tmp);
@@ -855,6 +887,10 @@ evbuffer_remove_buffer(struct evbuffer *src, struct evbuffer *dst,
 
 	/* removes chains if possible */
 	while (chain->off <= datlen) {
+		/* We can't remove the last with data from src unless we
+		 * remove all chains, in which case we would have done the if
+		 * block above */
+		EVUTIL_ASSERT(chain != src->last_with_data);
 		nread += chain->off;
 		datlen -= chain->off;
 		previous_to_previous = previous;
@@ -871,6 +907,7 @@ evbuffer_remove_buffer(struct evbuffer *src, struct evbuffer *dst,
 		}
 		dst->previous_to_last = previous_to_previous;
 		dst->last = previous;
+		dst->last_with_data = dst->last;
 		previous->next = NULL;
 		src->first = chain;
 		if (src->first == src->last)
@@ -907,6 +944,7 @@ evbuffer_pullup(struct evbuffer *buf, ev_ssize_t size)
 	struct evbuffer_chain *chain, *next, *tmp;
 	unsigned char *buffer, *result = NULL;
 	ev_ssize_t remaining;
+	int removed_last_with_data = 0;
 
 	EVBUFFER_LOCK(buf);
 
@@ -976,6 +1014,8 @@ evbuffer_pullup(struct evbuffer *buf, ev_ssize_t size)
 		memcpy(buffer, chain->buffer + chain->misalign, chain->off);
 		size -= chain->off;
 		buffer += chain->off;
+		if (chain ==  buf->last_with_data)
+			removed_last_with_data = 1;
 
 		evbuffer_chain_free(chain);
 	}
@@ -994,6 +1034,13 @@ evbuffer_pullup(struct evbuffer *buf, ev_ssize_t size)
 
 	tmp->next = chain;
 
+	if (removed_last_with_data) {
+		int n;
+		buf->last_with_data = buf->first;
+		n = advance_last_with_data(buf);
+		EVUTIL_ASSERT(n == 0);
+	}
+
 	result = (tmp->buffer + tmp->misalign);
 
 done:
@@ -1382,6 +1429,11 @@ evbuffer_prepend(struct evbuffer *buf, const void *data, size_t datlen)
 	buf->first = tmp;
 	if (buf->previous_to_last == NULL)
 		buf->previous_to_last = tmp;
+	if (buf->last_with_data == NULL)
+		buf->last_with_data = tmp;
+	else if (chain && buf->last_with_data == chain && 0==chain->off)
+		buf->last_with_data = tmp;
+
 	tmp->next = chain;
 
 	tmp->off = datlen;
@@ -1465,6 +1517,8 @@ evbuffer_expand(struct evbuffer *buf, size_t datlen)
 	if (buf->previous_to_last)
 		buf->previous_to_last->next = tmp;
 	buf->last = tmp;
+	if (buf->last->off || buf->last_with_data == chain)
+		buf->last_with_data = tmp;
 
 	evbuffer_chain_free(chain);
 
@@ -1528,8 +1582,9 @@ _evbuffer_expand_fast(struct evbuffer *buf, size_t datlen)
 		if (buf->previous_to_last)
 			buf->previous_to_last->next = tmp;
 		buf->last = tmp;
+		if (chain == buf->last_with_data)
+			buf->last_with_data = tmp;
 		evbuffer_chain_free(chain);
-
 	} else {
 		/* Add a new chunk big enough to hold what won't fit
 		 * in chunk. */
diff --git a/evbuffer-internal.h b/evbuffer-internal.h
index 894dc2f1..ff599077 100644
--- a/evbuffer-internal.h
+++ b/evbuffer-internal.h
@@ -84,6 +84,10 @@ struct evbuffer {
 	 * repurposed accordingly. */
 	struct evbuffer_chain *previous_to_last;
 
+	/** The last chain that has any data in it.  If all chains in the
+	 * buffer are empty, points to the first chain */
+	struct evbuffer_chain *last_with_data;
+
 	/** Total amount of bytes stored in all chains.*/
 	size_t total_len;
 
diff --git a/test/regress_buffer.c b/test/regress_buffer.c
index a89b818a..d3db462e 100644
--- a/test/regress_buffer.c
+++ b/test/regress_buffer.c
@@ -68,6 +68,7 @@ _evbuffer_validate(struct evbuffer *buf)
 {
 	struct evbuffer_chain *chain, *previous = NULL;
 	size_t sum = 0;
+	int found_last_with_data = 0;
 
 	if (buf->first == NULL) {
 		tt_assert(buf->last == NULL);
@@ -81,6 +82,8 @@ _evbuffer_validate(struct evbuffer *buf)
 
 	chain = buf->first;
 	while (chain != NULL) {
+		if (chain == buf->last_with_data)
+			found_last_with_data = 1;
 		sum += chain->off;
 		if (chain->next == NULL) {
 			tt_assert(buf->previous_to_last == previous);
@@ -91,6 +94,23 @@ _evbuffer_validate(struct evbuffer *buf)
 		chain = chain->next;
 	}
 
+	if (buf->first)
+		tt_assert(buf->last_with_data);
+	if (buf->last_with_data) {
+		tt_assert(found_last_with_data);
+		chain = buf->last_with_data;
+		if (chain->off == 0 || buf->total_len == 0) {
+			tt_assert(chain->off == 0)
+			tt_assert(chain == buf->first);
+			tt_assert(buf->total_len == 0);
+		}
+		chain = chain->next;
+		while (chain != NULL) {
+			tt_assert(chain->off == 0);
+			chain = chain->next;
+		}
+	}
+
 	tt_assert(sum == buf->total_len);
 	return 1;
  end:
@@ -239,8 +259,10 @@ test_evbuffer_reserve2(void *ptr)
 	cp = v[0].iov_base;
 	remaining = v[0].iov_len - 512;
 	v[0].iov_len = 512;
+	evbuffer_validate(buf);
 	tt_int_op(0, ==, evbuffer_commit_space(buf, v, 1));
 	tt_int_op(evbuffer_get_length(buf), ==, 512);
+	evbuffer_validate(buf);
 
 	/* Ask for another same-chunk request, in an existing chunk. Use 8
 	 * bytes of it. */
@@ -253,6 +275,7 @@ test_evbuffer_reserve2(void *ptr)
 	tt_int_op(0, ==, evbuffer_commit_space(buf, v, 1));
 	tt_int_op(evbuffer_get_length(buf), ==, 520);
 	remaining -= 8;
+	evbuffer_validate(buf);
 
 	/* Now ask for a request that will be split. Use only one byte of it,
 	   though. */
@@ -268,6 +291,7 @@ test_evbuffer_reserve2(void *ptr)
 	tt_int_op(0, ==, evbuffer_commit_space(buf, v, 1));
 	tt_int_op(evbuffer_get_length(buf), ==, 521);
 	remaining -= 1;
+	evbuffer_validate(buf);
 
 	/* Now ask for a request that will be split. Use some of the first
 	 * part and some of the second. */
@@ -283,7 +307,7 @@ test_evbuffer_reserve2(void *ptr)
 	v[1].iov_len = 60;
 	tt_int_op(0, ==, evbuffer_commit_space(buf, v, 2));
 	tt_int_op(evbuffer_get_length(buf), ==, 981);
-
+	evbuffer_validate(buf);
 
 	/* Now peek to make sure stuff got made how we like. */
 	memset(v,0,sizeof(v));
@@ -358,6 +382,7 @@ test_evbuffer_reference(void *ptr)
 
 	tt_assert(!memcmp(evbuffer_pullup(dst, strlen(data)),
 			  data, strlen(data)));
+	evbuffer_validate(dst);
 
  end:
 	evbuffer_free(dst);
@@ -394,6 +419,7 @@ test_evbuffer_add_file(void *ptr)
 	if (memcmp(compare, data, strlen(data)))
 		tt_abort_msg("Data from add_file differs.");
 
+	evbuffer_validate(src);
  end:
 	EVUTIL_CLOSESOCKET(pair[0]);
 	EVUTIL_CLOSESOCKET(pair[1]);
@@ -711,6 +737,7 @@ test_evbuffer_ptr_set(void *ptr)
 	v[0].iov_len = 5000;
 	memset(v[0].iov_base, 1, v[0].iov_len);
 	evbuffer_commit_space(buf, v, 1);
+	evbuffer_validate(buf);
 
 	evbuffer_reserve_space(buf, 4000, v, 1);
 	v[0].iov_len = 4000;
@@ -721,6 +748,7 @@ test_evbuffer_ptr_set(void *ptr)
 	v[0].iov_len = 3000;
 	memset(v[0].iov_base, 3, v[0].iov_len);
 	evbuffer_commit_space(buf, v, 1);
+	evbuffer_validate(buf);
 
 	tt_int_op(evbuffer_get_length(buf), ==, 12000);
 
@@ -867,7 +895,9 @@ test_evbuffer_callbacks(void *ptr)
 	evbuffer_add_printf(buf, "This will not.");
 	tt_str_op(evbuffer_pullup(buf, -1), ==, "This will not.");
 
+	evbuffer_validate(buf);
 	evbuffer_drain(buf, evbuffer_get_length(buf));
+	evbuffer_validate(buf);
 
 #if 0
 	/* Now let's try a suspended callback. */
@@ -938,6 +968,7 @@ test_evbuffer_add_reference(void *ptr)
 	/* Make sure that prepending does not meddle with immutable data */
 	tt_int_op(evbuffer_prepend(buf1, "I have ", 7), ==, 0);
 	tt_int_op(memcmp(chunk1, "If you", 6), ==, 0);
+	evbuffer_validate(buf1);
 
 	/* Make sure that when the chunk is over, the callback is invoked. */
 	evbuffer_drain(buf1, 7); /* Remove prepended stuff. */
@@ -949,6 +980,7 @@ test_evbuffer_add_reference(void *ptr)
 	tt_assert(ref_done_cb_called_with_data == chunk1);
 	tt_assert(ref_done_cb_called_with_len == len1);
 	tt_int_op(ref_done_cb_called_count, ==, 1);
+	evbuffer_validate(buf1);
 
 	/* Drain some of the remaining chunk, then add it to another buffer */
 	evbuffer_drain(buf1, 6); /* Remove the ", you ". */
@@ -964,6 +996,7 @@ test_evbuffer_add_reference(void *ptr)
 	evbuffer_drain(buf2, evbuffer_get_length(buf2));
 	tt_int_op(ref_done_cb_called_count, ==, 2);
 	tt_assert(ref_done_cb_called_with == (void*)222);
+	evbuffer_validate(buf2);
 
 	/* Now add more stuff to buf1 and make sure that it gets removed on
 	 * free. */

From c8ac57f1f58358dfdb1030143ab043b6addd0c3c Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Wed, 10 Mar 2010 23:24:14 -0500
Subject: [PATCH 2/6] Use last_with_data in place of previous_to_last

This actually makes some of the code a lot simpler.  The only
ones that actually used previous_to_last for anything were reserving
and committing space.
---
 buffer.c              | 285 ++++++++++++++++++++++++------------------
 evbuffer-internal.h   |   6 +-
 test/regress_buffer.c |   1 +
 3 files changed, 169 insertions(+), 123 deletions(-)

diff --git a/buffer.c b/buffer.c
index 82835975..b58d2253 100644
--- a/buffer.c
+++ b/buffer.c
@@ -525,9 +525,9 @@ evbuffer_reserve_space(struct evbuffer *buf, ev_ssize_t size,
 		vec[0].iov_len = CHAIN_SPACE_LEN(chain);
 		n = 1;
 	} else {
-		if (_evbuffer_expand_fast(buf, size)<0)
+		if (_evbuffer_expand_fast(buf, size, n_vecs)<0)
 			goto done;
-		n = _evbuffer_read_setup_vecs(buf, size, vec, &chain, 0);
+		n = _evbuffer_read_setup_vecs(buf, size, vec, n_vecs, &chain, 0);
 	}
 
 done:
@@ -556,50 +556,64 @@ int
 evbuffer_commit_space(struct evbuffer *buf,
     struct evbuffer_iovec *vec, int n_vecs)
 {
-	struct evbuffer_chain *last, *prev;
+	struct evbuffer_chain *firstchain, *chain;
 	int result = -1;
-	size_t added;
+	size_t added = 0;
+	int i;
 
 	EVBUFFER_LOCK(buf);
 
-	prev = buf->previous_to_last;
-	last = buf->last;
-
 	if (buf->freeze_end)
 		goto done;
-	if (n_vecs < 1 || n_vecs > 2)
+	if (n_vecs == 0) {
+		result = 0;
 		goto done;
-	if (n_vecs == 2) {
-		if (!prev || !last ||
-		    vec[0].iov_base != CHAIN_SPACE_PTR(prev) ||
-		    vec[1].iov_base != CHAIN_SPACE_PTR(last) ||
-		    vec[0].iov_len > CHAIN_SPACE_LEN(prev) ||
-		    vec[1].iov_len > CHAIN_SPACE_LEN(last))
+	} else if (n_vecs == 1 &&
+	    (buf->last && vec[0].iov_base == CHAIN_SPACE_PTR(buf->last))) {
+		/* The user only got or used one chain; it might not
+		 * be the first one with space in it. */
+		if (vec[0].iov_len > CHAIN_SPACE_LEN(buf->last))
 			goto done;
-
-		prev->off += vec[0].iov_len;
-		last->off += vec[1].iov_len;
-		added = vec[0].iov_len + vec[1].iov_len;
-	} else {
-		/* n_vecs == 1 */
-		struct evbuffer_chain *chain;
-		if (prev && vec[0].iov_base == CHAIN_SPACE_PTR(prev))
-			chain = prev;
-		else if (last && vec[0].iov_base == CHAIN_SPACE_PTR(last))
-			chain = last;
-		else
-			goto done;
-		if (vec[0].iov_len > CHAIN_SPACE_LEN(chain))
-			goto done;
-
-		chain->off += vec[0].iov_len;
+		buf->last->off += vec[0].iov_len;
 		added = vec[0].iov_len;
+		if (added)
+			buf->last_with_data = buf->last;
+		goto okay;
 	}
 
+	/* Advance 'firstchain' to the first chain with space in it. */
+	firstchain = buf->last_with_data;
+	if (!firstchain)
+		goto done;
+	if (CHAIN_SPACE_LEN(firstchain) == 0) {
+		firstchain = firstchain->next;
+	}
+
+	chain = firstchain;
+	/* pass 1: make sure that the pointers and lengths of vecs[] are in
+	 * bounds before we try to commit anything. */
+	for (i=0; i<n_vecs; ++i) {
+		if (!chain)
+			goto done;
+		if (vec[i].iov_base != CHAIN_SPACE_PTR(chain) ||
+		    vec[i].iov_len > CHAIN_SPACE_LEN(chain))
+			goto done;
+		chain = chain->next;
+	}
+	/* pass 2: actually adjust all the chains. */
+	chain = firstchain;
+	for (i=0; i<n_vecs; ++i) {
+		chain->off += vec[i].iov_len;
+		added += vec[i].iov_len;
+		if (vec[i].iov_len)
+			buf->last_with_data = chain;
+		chain = chain->next;
+	}
+
+okay:
 	buf->total_len += added;
 	buf->n_add_for_cb += added;
 	result = 0;
-	advance_last_with_data(buf);
 	evbuffer_invoke_callbacks(buf);
 
 done:
@@ -1529,17 +1543,21 @@ err:
 	return result;
 }
 
-/* Make sure that datlen bytes are available for writing in the last two
+/* Make sure that datlen bytes are available for writing in the last n
  * chains.  Never copies or moves data. */
 int
-_evbuffer_expand_fast(struct evbuffer *buf, size_t datlen)
+_evbuffer_expand_fast(struct evbuffer *buf, size_t datlen, int n)
 {
-	struct evbuffer_chain *chain = buf->last, *tmp;
-	size_t avail, avail_in_prev = 0;
+	struct evbuffer_chain *chain = buf->last, *tmp, *next;
+	size_t avail;
+	int used;
 
 	ASSERT_EVBUFFER_LOCKED(buf);
+	EVUTIL_ASSERT(n >= 2);
 
 	if (chain == NULL || (chain->flags & EVBUFFER_IMMUTABLE)) {
+		/* There is no last chunk, or we can't touch the last chunk.
+		 * Just add a new chunk. */
 		chain = evbuffer_chain_new(datlen);
 		if (chain == NULL)
 			return (-1);
@@ -1548,57 +1566,95 @@ _evbuffer_expand_fast(struct evbuffer *buf, size_t datlen)
 		return (0);
 	}
 
-	/* How many bytes can we stick at the end of chain? */
-
-	if (chain->off) {
-		avail = chain->buffer_len - (chain->off + chain->misalign);
-		avail_in_prev = 0;
-	} else {
-		/* No data in chain; realign it. */
-		chain->misalign = 0;
-		avail = chain->buffer_len;
-		/* Can we stick some data in the penultimate chain? */
-		if (buf->previous_to_last) {
-			struct evbuffer_chain *prev = buf->previous_to_last;
-			avail_in_prev = CHAIN_SPACE_LEN(prev);
+	used = 0; /* number of chains we're using space in. */
+	avail = 0; /* how much space they have. */
+	/* How many bytes can we stick at the end of buffer as it is?  Iterate
+	 * over the chains at the end of the buffer, tring to see how much
+	 * space we have in the first n. */
+	for (chain = buf->last_with_data; chain; chain = chain->next) {
+		if (chain->off) {
+			size_t space = CHAIN_SPACE_LEN(chain);
+			EVUTIL_ASSERT(chain == buf->last_with_data);
+			if (space) {
+				avail += space;
+				++used;
+			}
+		} else {
+			/* No data in chain; realign it. */
+			chain->misalign = 0;
+			avail += chain->buffer_len;
+			++used;
 		}
+		if (avail >= datlen) {
+			/* There is already enough space.  Just return */
+			return (0);
+		}
+		if (used == n)
+			break;
 	}
 
-	/* If we can fit all the data, then we don't have to do anything */
-	if (avail+avail_in_prev >= datlen)
-		return (0);
+	/* There wasn't enough space in the first n chains with space in
+	 * them. Either add a new chain with enough space, or replace all
+	 * empty chains with one that has enough space, depending on n. */
+	if (used < n) {
+		/* The loop ran off the end of the chains before it hit n
+		 * chains; we can add another. */
+		EVUTIL_ASSERT(chain == NULL);
 
-	/* Otherwise, we need a bigger chunk. */
-	if (chain->off == 0) {
-		/* If there are no bytes on this chain, free it and
-		   replace it with a better one. */
-		/* XXX round up. */
-		tmp = evbuffer_chain_new(datlen-avail_in_prev);
-		if (tmp == NULL)
-			return -1;
-		/* XXX write functions to in new chains */
-		if (buf->first == chain)
-			buf->first = tmp;
-		if (buf->previous_to_last)
-			buf->previous_to_last->next = tmp;
-		buf->last = tmp;
-		if (chain == buf->last_with_data)
-			buf->last_with_data = tmp;
-		evbuffer_chain_free(chain);
-	} else {
-		/* Add a new chunk big enough to hold what won't fit
-		 * in chunk. */
-		/*XXX round this up. */
-		tmp = evbuffer_chain_new(datlen-avail);
+		tmp = evbuffer_chain_new(datlen - avail);
 		if (tmp == NULL)
 			return (-1);
 
-		buf->previous_to_last = chain;
-		chain->next = tmp;
+		buf->previous_to_last = buf->last;
+		buf->last->next = tmp;
 		buf->last = tmp;
-	}
+		/* (we would only set last_with_data if we added the first
+		 * chain. But if the buffer had no chains, we would have
+		 * just allocated a new chain earlier) */
+		return (0);
+	} else {
+		/* Nuke _all_ the empty chains. */
+		int rmv_all = 0; /* True iff we removed last_with_data. */
+		chain = buf->last_with_data;
+		if (!chain->off) {
+			EVUTIL_ASSERT(chain == buf->first);
+			rmv_all = 1;
+			avail = 0;
+		} else {
+			avail = CHAIN_SPACE_LEN(chain);
+			chain = chain->next;
+		}
 
-	return (0);
+
+		for (; chain; chain = next) {
+			next = chain->next;
+			EVUTIL_ASSERT(chain->off == 0);
+			evbuffer_chain_free(chain);
+		}
+		tmp = evbuffer_chain_new(datlen - avail);
+		if (tmp == NULL) {
+			if (rmv_all) {
+				ZERO_CHAIN(buf);
+			} else {
+				/* XXX This error case should set
+				 * previous_to_last to something better. */
+				buf->last = buf->last_with_data;
+				buf->last_with_data->next = NULL;
+				buf->previous_to_last = NULL;
+			}
+			return (-1);
+		}
+
+		if (rmv_all) {
+			buf->first = buf->last = buf->last_with_data = tmp;
+			buf->previous_to_last = NULL;
+		} else {
+			buf->last_with_data->next = tmp;
+			buf->last = tmp;
+			buf->previous_to_last = buf->last_with_data;
+		}
+		return (0);
+	}
 }
 
 /*
@@ -1633,7 +1689,8 @@ _evbuffer_expand_fast(struct evbuffer *buf, size_t datlen)
 
     @param buf The buffer to read into
     @param howmuch How much we want to read.
-    @param vecs An array of two iovecs or WSABUFs.
+    @param vecs An array of two or more iovecs or WSABUFs.
+    @param n_vecs_avail The length of vecs
     @param chainp A pointer to a variable to hold the first chain we're
       reading into.
     @param exact Boolean: if true, we do not provide more than 'howmuch'
@@ -1642,52 +1699,36 @@ _evbuffer_expand_fast(struct evbuffer *buf, size_t datlen)
  */
 int
 _evbuffer_read_setup_vecs(struct evbuffer *buf, ev_ssize_t howmuch,
-    struct evbuffer_iovec *vecs, struct evbuffer_chain **chainp, int exact)
+    struct evbuffer_iovec *vecs, int n_vecs_avail,
+    struct evbuffer_chain **chainp, int exact)
 {
-	struct evbuffer_chain *chain;
-	int nvecs;
+	struct evbuffer_chain *chain, *firstchain;
+	size_t so_far;
+	int i;
+	ASSERT_EVBUFFER_LOCKED(buf);
 
 	if (howmuch < 0)
 		return -1;
 
-	chain = buf->last;
+	so_far = 0;
+	/* Let firstchain be the first chain with any space on it */
+	firstchain =  buf->last_with_data;
+	if (CHAIN_SPACE_LEN(firstchain) == 0)
+		firstchain = firstchain->next;
 
-	if (chain->off == 0 && buf->previous_to_last &&
-	    CHAIN_SPACE_LEN(buf->previous_to_last)) {
-		/* The last chain is empty, so it's safe to
-		   use the space in the next-to-last chain.
-		*/
-		struct evbuffer_chain *prev = buf->previous_to_last;
-		vecs[0].iov_base = CHAIN_SPACE_PTR(prev);
-		vecs[0].iov_len = CHAIN_SPACE_LEN(prev);
-		vecs[1].iov_base = CHAIN_SPACE_PTR(chain);
-		vecs[1].iov_len = CHAIN_SPACE_LEN(chain);
-		if (vecs[0].iov_len >= (size_t)howmuch) {
-			/* The next-to-last chain has enough
-			 * space on its own. */
-			chain = prev;
-			nvecs = 1;
-		} else {
-			/* We'll need both chains. */
-			chain = prev;
-			nvecs = 2;
-			if (exact &&
-			    (vecs[0].iov_len + vecs[1].iov_len > (size_t)howmuch)) {
-				vecs[1].iov_len = howmuch - vecs[0].iov_len;
-			}
-		}
-	} else {
-		/* There's data in the last chain, so we're
-		 * not allowed to use the next-to-last. */
-		nvecs = 1;
-		vecs[0].iov_base = CHAIN_SPACE_PTR(chain);
-		vecs[0].iov_len = CHAIN_SPACE_LEN(chain);
-		if (exact && (vecs[0].iov_len > (size_t)howmuch))
-			vecs[0].iov_len = howmuch;
+	chain = firstchain;
+	for (i = 0; i < n_vecs_avail && so_far < howmuch; ++i) {
+		size_t avail = CHAIN_SPACE_LEN(chain);
+		if (avail > howmuch && exact)
+			avail = howmuch;
+		vecs[i].iov_base = CHAIN_SPACE_PTR(chain);
+		vecs[i].iov_len = avail;
+		so_far += avail;
+		chain = chain->next;
 	}
 
-	*chainp = chain;
-	return nvecs;
+	*chainp = firstchain;
+	return i;
 }
 
 /* TODO(niels): should this function return ev_ssize_t and take ev_ssize_t
@@ -1744,19 +1785,19 @@ evbuffer_read(struct evbuffer *buf, evutil_socket_t fd, int howmuch)
 #ifdef USE_IOVEC_IMPL
 	/* Since we can use iovecs, we're willing to use the last
 	 * _two_ chains. */
-	if (_evbuffer_expand_fast(buf, howmuch) == -1) {
+	if (_evbuffer_expand_fast(buf, howmuch, 2) == -1) {
 		result = -1;
 		goto done;
 	} else {
 		IOV_TYPE vecs[2];
 #ifdef _EVBUFFER_IOVEC_IS_NATIVE
-		nvecs = _evbuffer_read_setup_vecs(buf, howmuch, vecs,
+		nvecs = _evbuffer_read_setup_vecs(buf, howmuch, vecs, 2,
 		    &chain, 1);
 #else
 		/* We aren't using the native struct iovec.  Therefore,
 		   we are on win32. */
 		struct evbuffer_iovec ev_vecs[2];
-		nvecs = _evbuffer_read_setup_vecs(buf, howmuch, ev_vecs,
+		nvecs = _evbuffer_read_setup_vecs(buf, howmuch, ev_vecs, 2,
 		    &chain, 1);
 
 		if (nvecs == 2) {
@@ -1822,14 +1863,18 @@ evbuffer_read(struct evbuffer *buf, evutil_socket_t fd, int howmuch)
 		if (space < n) {
 			chain->off += space;
 			chain->next->off += n-space;
+			buf->last_with_data = chain->next;
 		} else {
 			chain->off += n;
+			buf->last_with_data = chain;
 		}
 	} else {
 		chain->off += n;
+		buf->last_with_data = chain;
 	}
 #else
 	chain->off += n;
+	buf->last_with_data = chain;
 #endif
 	buf->total_len += n;
 	buf->n_add_for_cb += n;
diff --git a/evbuffer-internal.h b/evbuffer-internal.h
index ff599077..d59b6a36 100644
--- a/evbuffer-internal.h
+++ b/evbuffer-internal.h
@@ -238,8 +238,8 @@ void _evbuffer_chain_unpin(struct evbuffer_chain *chain, unsigned flag);
 void _evbuffer_decref_and_unlock(struct evbuffer *buffer);
 
 /** As evbuffer_expand, but does not guarantee that the newly allocated memory
- * is contiguous.  Instead, it may be split across two chunks. */
-int _evbuffer_expand_fast(struct evbuffer *, size_t);
+ * is contiguous.  Instead, it may be split across two or more chunks. */
+int _evbuffer_expand_fast(struct evbuffer *, size_t, int);
 
 /** Helper: prepares for a readv/WSARecv call by expanding the buffer to
  * hold enough memory to read 'howmuch' bytes in possibly noncontiguous memory.
@@ -248,7 +248,7 @@ int _evbuffer_expand_fast(struct evbuffer *, size_t);
  * Returns the number of vecs used.
  */
 int _evbuffer_read_setup_vecs(struct evbuffer *buf, ev_ssize_t howmuch,
-    struct evbuffer_iovec *vecs, struct evbuffer_chain **chainp, int exact);
+    struct evbuffer_iovec *vecs, int n_vecs, struct evbuffer_chain **chainp, int exact);
 
 /* Helper macro: copies an evbuffer_iovec in ei to a win32 WSABUF in i. */
 #define WSABUF_FROM_EVBUFFER_IOV(i,ei) do {		\
diff --git a/test/regress_buffer.c b/test/regress_buffer.c
index d3db462e..d5e25c92 100644
--- a/test/regress_buffer.c
+++ b/test/regress_buffer.c
@@ -296,6 +296,7 @@ test_evbuffer_reserve2(void *ptr)
 	/* Now ask for a request that will be split. Use some of the first
 	 * part and some of the second. */
 	n = evbuffer_reserve_space(buf, remaining+64, v, 2);
+	evbuffer_validate(buf);
 	tt_int_op(n, ==, 2);
 	tt_assert(cp + 521 == v[0].iov_base);
 	tt_int_op(remaining, ==, v[0].iov_len);

From 6f47bd12ed27085f68573a02dfa2e2761eb1e6b0 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Wed, 10 Mar 2010 23:28:51 -0500
Subject: [PATCH 3/6] Remove previous_to_last from evbuffer

---
 buffer.c              | 33 +--------------------------------
 evbuffer-internal.h   | 13 ++-----------
 test/regress_buffer.c |  9 +--------
 3 files changed, 4 insertions(+), 51 deletions(-)

diff --git a/buffer.c b/buffer.c
index b58d2253..700a1945 100644
--- a/buffer.c
+++ b/buffer.c
@@ -219,7 +219,6 @@ evbuffer_chain_insert(struct evbuffer *buf, struct evbuffer_chain *chain)
 	ASSERT_EVBUFFER_LOCKED(buf);
 	if (buf->first == NULL) {
 		buf->first = buf->last = chain;
-		buf->previous_to_last = NULL;
 		buf->last_with_data = chain;
 	} else {
 		/* the last chain is empty so we can just drop it */
@@ -227,10 +226,8 @@ evbuffer_chain_insert(struct evbuffer *buf, struct evbuffer_chain *chain)
 			if (buf->last_with_data == buf->last)
 				buf->last_with_data = chain;
 			evbuffer_chain_free(buf->last);
-			buf->previous_to_last->next = chain;
 			buf->last = chain;
 		} else {
-			buf->previous_to_last = buf->last;
 			buf->last->next = chain;
 			buf->last = chain;
 		}
@@ -625,7 +622,6 @@ done:
 		ASSERT_EVBUFFER_LOCKED(dst);	\
 		(dst)->first = NULL;		\
 		(dst)->last = NULL;		\
-		(dst)->previous_to_last = NULL; \
 		(dst)->last_with_data = NULL;	\
 		(dst)->total_len = 0;		\
 	} while (0)
@@ -634,7 +630,6 @@ done:
 		ASSERT_EVBUFFER_LOCKED(dst);			   \
 		ASSERT_EVBUFFER_LOCKED(src);			   \
 		(dst)->first = (src)->first;			   \
-		(dst)->previous_to_last = (src)->previous_to_last; \
 		(dst)->last_with_data = (src)->last_with_data;	   \
 		(dst)->last = (src)->last;			   \
 		(dst)->total_len = (src)->total_len;		   \
@@ -644,8 +639,6 @@ done:
 		ASSERT_EVBUFFER_LOCKED(dst);				\
 		ASSERT_EVBUFFER_LOCKED(src);				\
 		(dst)->last->next = (src)->first;			\
-		(dst)->previous_to_last = (src)->previous_to_last ?	\
-		    (src)->previous_to_last : (dst)->last;		\
 		if ((src)->last_with_data)				\
 			(dst)->last_with_data = (src)->last_with_data;	\
 		(dst)->last = (src)->last;				\
@@ -658,8 +651,6 @@ done:
 		(src)->last->next = (dst)->first;		   \
 		(dst)->first = (src)->first;			   \
 		(dst)->total_len += (src)->total_len;		   \
-		if ((dst)->previous_to_last == NULL)		   \
-			(dst)->previous_to_last = (src)->last;	   \
 		if ((dst)->last_with_data == NULL)		   \
 			(dst)->last_with_data = (src)->last_with_data; \
 	} while (0)
@@ -784,8 +775,6 @@ evbuffer_drain(struct evbuffer *buf, size_t len)
 		}
 
 		buf->first = chain;
-		if (buf->first == buf->last)
-			buf->previous_to_last = NULL;
 		chain->misalign += len;
 		chain->off -= len;
 	}
@@ -843,8 +832,6 @@ evbuffer_remove(struct evbuffer *buf, void *data_out, size_t datlen)
 	buf->first = chain;
 	if (chain == NULL)
 		buf->last = NULL;
-	if (buf->first == buf->last)
-		buf->previous_to_last = NULL;
 
 	if (datlen) {
 		memcpy(data, chain->buffer + chain->misalign, datlen);
@@ -873,7 +860,7 @@ evbuffer_remove_buffer(struct evbuffer *src, struct evbuffer *dst,
 	/*XXX We should have an option to force this to be zero-copy.*/
 
 	/*XXX can fail badly on sendfile case. */
-	struct evbuffer_chain *chain, *previous, *previous_to_previous = NULL;
+	struct evbuffer_chain *chain, *previous;
 	size_t nread = 0;
 	int result;
 
@@ -907,7 +894,6 @@ evbuffer_remove_buffer(struct evbuffer *src, struct evbuffer *dst,
 		EVUTIL_ASSERT(chain != src->last_with_data);
 		nread += chain->off;
 		datlen -= chain->off;
-		previous_to_previous = previous;
 		previous = chain;
 		chain = chain->next;
 	}
@@ -919,13 +905,10 @@ evbuffer_remove_buffer(struct evbuffer *src, struct evbuffer *dst,
 		} else {
 			dst->last->next = src->first;
 		}
-		dst->previous_to_last = previous_to_previous;
 		dst->last = previous;
 		dst->last_with_data = dst->last;
 		previous->next = NULL;
 		src->first = chain;
-		if (src->first == src->last)
-			src->previous_to_last = NULL;
 
 		dst->total_len += nread;
 		dst->n_add_for_cb += nread;
@@ -1038,12 +1021,8 @@ evbuffer_pullup(struct evbuffer *buf, ev_ssize_t size)
 		memcpy(buffer, chain->buffer + chain->misalign, size);
 		chain->misalign += size;
 		chain->off -= size;
-		if (chain == buf->last)
-			buf->previous_to_last = tmp;
 	} else {
 		buf->last = tmp;
-		/* the last is already the first, so we have no previous */
-		buf->previous_to_last = NULL;
 	}
 
 	tmp->next = chain;
@@ -1441,8 +1420,6 @@ evbuffer_prepend(struct evbuffer *buf, const void *data, size_t datlen)
 	if ((tmp = evbuffer_chain_new(datlen)) == NULL)
 		goto done;
 	buf->first = tmp;
-	if (buf->previous_to_last == NULL)
-		buf->previous_to_last = tmp;
 	if (buf->last_with_data == NULL)
 		buf->last_with_data = tmp;
 	else if (chain && buf->last_with_data == chain && 0==chain->off)
@@ -1528,8 +1505,6 @@ evbuffer_expand(struct evbuffer *buf, size_t datlen)
 	/* fix up the chain */
 	if (buf->first == chain)
 		buf->first = tmp;
-	if (buf->previous_to_last)
-		buf->previous_to_last->next = tmp;
 	buf->last = tmp;
 	if (buf->last->off || buf->last_with_data == chain)
 		buf->last_with_data = tmp;
@@ -1605,7 +1580,6 @@ _evbuffer_expand_fast(struct evbuffer *buf, size_t datlen, int n)
 		if (tmp == NULL)
 			return (-1);
 
-		buf->previous_to_last = buf->last;
 		buf->last->next = tmp;
 		buf->last = tmp;
 		/* (we would only set last_with_data if we added the first
@@ -1636,22 +1610,17 @@ _evbuffer_expand_fast(struct evbuffer *buf, size_t datlen, int n)
 			if (rmv_all) {
 				ZERO_CHAIN(buf);
 			} else {
-				/* XXX This error case should set
-				 * previous_to_last to something better. */
 				buf->last = buf->last_with_data;
 				buf->last_with_data->next = NULL;
-				buf->previous_to_last = NULL;
 			}
 			return (-1);
 		}
 
 		if (rmv_all) {
 			buf->first = buf->last = buf->last_with_data = tmp;
-			buf->previous_to_last = NULL;
 		} else {
 			buf->last_with_data->next = tmp;
 			buf->last = tmp;
-			buf->previous_to_last = buf->last_with_data;
 		}
 		return (0);
 	}
diff --git a/evbuffer-internal.h b/evbuffer-internal.h
index d59b6a36..e8555dbd 100644
--- a/evbuffer-internal.h
+++ b/evbuffer-internal.h
@@ -73,19 +73,10 @@ struct evbuffer {
 	struct evbuffer_chain *first;
 	/** The last chain in this buffer's linked list of chains. */
 	struct evbuffer_chain *last;
-	/** The next-to-last chain in this buffer's linked list of chains.
-	 * NULL if the buffer has 0 or 1 chains.  Used in case there's an
-	 * ongoing read that needs to be split across multiple chains: we want
-	 * to add a new chain as a read target, but we don't want to lose our
-	 * pointer to the next-to-last chain if the read turns out to be
-	 * incomplete.
-	 */
-	/* FIXME: This should probably be called last_with_space and
-	 * repurposed accordingly. */
-	struct evbuffer_chain *previous_to_last;
 
 	/** The last chain that has any data in it.  If all chains in the
-	 * buffer are empty, points to the first chain */
+	 * buffer are empty, points to the first chain.  If the buffer has no
+	 * chains, this is NULL. */
 	struct evbuffer_chain *last_with_data;
 
 	/** Total amount of bytes stored in all chains.*/
diff --git a/test/regress_buffer.c b/test/regress_buffer.c
index d5e25c92..55f02f7e 100644
--- a/test/regress_buffer.c
+++ b/test/regress_buffer.c
@@ -66,31 +66,24 @@
 static int
 _evbuffer_validate(struct evbuffer *buf)
 {
-	struct evbuffer_chain *chain, *previous = NULL;
+	struct evbuffer_chain *chain;
 	size_t sum = 0;
 	int found_last_with_data = 0;
 
 	if (buf->first == NULL) {
 		tt_assert(buf->last == NULL);
-		tt_assert(buf->previous_to_last == NULL);
 		tt_assert(buf->total_len == 0);
 	}
 
-	if (buf->previous_to_last == NULL) {
-		tt_assert(buf->first == buf->last);
-	}
-
 	chain = buf->first;
 	while (chain != NULL) {
 		if (chain == buf->last_with_data)
 			found_last_with_data = 1;
 		sum += chain->off;
 		if (chain->next == NULL) {
-			tt_assert(buf->previous_to_last == previous);
 			tt_assert(buf->last == chain);
 		}
 		tt_assert(chain->buffer_len >= chain->misalign + chain->off);
-		previous = chain;
 		chain = chain->next;
 	}
 

From e470ad3c354c586a3a995ce3a1fe7d093ebf999c Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Wed, 10 Mar 2010 23:39:30 -0500
Subject: [PATCH 4/6] Allow evbuffer_read() to split across more than 2 iovecs

Previously it would only accept 2 iovecs at most, because our
previous_to_last nonsense didn't let it take any more.  This forced us
to do more reallocations in some cases when an extra small malloc
would have sufficed.
---
 buffer.c | 46 +++++++++++++++++++++-------------------------
 1 file changed, 21 insertions(+), 25 deletions(-)

diff --git a/buffer.c b/buffer.c
index 700a1945..6f56dcb8 100644
--- a/buffer.c
+++ b/buffer.c
@@ -1639,17 +1639,18 @@ _evbuffer_expand_fast(struct evbuffer *buf, size_t datlen, int n)
 #ifdef _EVENT_HAVE_SYS_UIO_H
 /* number of iovec we use for writev, fragmentation is going to determine
  * how much we end up writing */
-#define NUM_IOVEC 128
+#define NUM_WRITE_IOVEC 128
 #define IOV_TYPE struct iovec
 #define IOV_PTR_FIELD iov_base
 #define IOV_LEN_FIELD iov_len
 #else
-#define NUM_IOVEC 16
+#define NUM_WRITE_IOVEC 16
 #define IOV_TYPE WSABUF
 #define IOV_PTR_FIELD buf
 #define IOV_LEN_FIELD len
 #endif
 #endif
+#define NUM_READ_IOVEC 4
 
 #define EVBUFFER_MAX_READ	4096
 
@@ -1710,7 +1711,7 @@ evbuffer_read(struct evbuffer *buf, evutil_socket_t fd, int howmuch)
 	int result;
 
 #ifdef USE_IOVEC_IMPL
-	int nvecs;
+	int nvecs, i, remaining;
 #else
 	unsigned char *p;
 #endif
@@ -1753,28 +1754,24 @@ evbuffer_read(struct evbuffer *buf, evutil_socket_t fd, int howmuch)
 
 #ifdef USE_IOVEC_IMPL
 	/* Since we can use iovecs, we're willing to use the last
-	 * _two_ chains. */
-	if (_evbuffer_expand_fast(buf, howmuch, 2) == -1) {
+	 * NUM_READ_IOVEC chains. */
+	if (_evbuffer_expand_fast(buf, howmuch, NUM_READ_IOVEC) == -1) {
 		result = -1;
 		goto done;
 	} else {
-		IOV_TYPE vecs[2];
+		IOV_TYPE vecs[NUM_READ_IOVEC];
 #ifdef _EVBUFFER_IOVEC_IS_NATIVE
-		nvecs = _evbuffer_read_setup_vecs(buf, howmuch, vecs, 2,
-		    &chain, 1);
+		nvecs = _evbuffer_read_setup_vecs(buf, howmuch, vecs,
+		    NUM_READ_IOVEC, &chain, 1);
 #else
 		/* We aren't using the native struct iovec.  Therefore,
 		   we are on win32. */
-		struct evbuffer_iovec ev_vecs[2];
+		struct evbuffer_iovec ev_vecs[NUM_READ_IOVEC];
 		nvecs = _evbuffer_read_setup_vecs(buf, howmuch, ev_vecs, 2,
 		    &chain, 1);
 
-		if (nvecs == 2) {
-			WSABUF_FROM_EVBUFFER_IOV(&vecs[1], &ev_vecs[1]);
-			WSABUF_FROM_EVBUFFER_IOV(&vecs[0], &ev_vecs[0]);
-		} else if (nvecs == 1) {
-			WSABUF_FROM_EVBUFFER_IOV(&vecs[0], &ev_vecs[0]);
-		}
+		for (i=0; i < n_vecs; ++i)
+			WSABUF_FROM_EVBUFFER_IOV(&vecs[i], &ev_vecs[i]);
 #endif
 
 #ifdef WIN32
@@ -1827,19 +1824,18 @@ evbuffer_read(struct evbuffer *buf, evutil_socket_t fd, int howmuch)
 	}
 
 #ifdef USE_IOVEC_IMPL
-	if (nvecs == 2) {
+	remaining = n;
+	for (i=0; i < nvecs; ++i) {
 		ev_ssize_t space = CHAIN_SPACE_LEN(chain);
-		if (space < n) {
+		if (space < remaining) {
 			chain->off += space;
-			chain->next->off += n-space;
-			buf->last_with_data = chain->next;
+			remaining -= space;
 		} else {
-			chain->off += n;
+			chain->off += remaining;
 			buf->last_with_data = chain;
+			break;
 		}
-	} else {
-		chain->off += n;
-		buf->last_with_data = chain;
+		chain = chain->next;
 	}
 #else
 	chain->off += n;
@@ -1861,7 +1857,7 @@ static inline int
 evbuffer_write_iovec(struct evbuffer *buffer, evutil_socket_t fd,
     ev_ssize_t howmuch)
 {
-	IOV_TYPE iov[NUM_IOVEC];
+	IOV_TYPE iov[NUM_WRITE_IOVEC];
 	struct evbuffer_chain *chain = buffer->first;
 	int n, i = 0;
 
@@ -1872,7 +1868,7 @@ evbuffer_write_iovec(struct evbuffer *buffer, evutil_socket_t fd,
 	/* XXX make this top out at some maximal data length?  if the
 	 * buffer has (say) 1MB in it, split over 128 chains, there's
 	 * no way it all gets written in one go. */
-	while (chain != NULL && i < NUM_IOVEC && howmuch) {
+	while (chain != NULL && i < NUM_WRITE_IOVEC && howmuch) {
 #ifdef USE_SENDFILE
 		/* we cannot write the file info via writev */
 		if (chain->flags & EVBUFFER_SENDFILE)

From 1e7b986827f2654de500678ef56dcbaecd5a08d4 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Thu, 11 Mar 2010 14:23:02 -0500
Subject: [PATCH 5/6] Fix last_with_data compilation on windows

---
 buffer.c      | 2 +-
 buffer_iocp.c | 7 +++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/buffer.c b/buffer.c
index 6f56dcb8..20454197 100644
--- a/buffer.c
+++ b/buffer.c
@@ -1770,7 +1770,7 @@ evbuffer_read(struct evbuffer *buf, evutil_socket_t fd, int howmuch)
 		nvecs = _evbuffer_read_setup_vecs(buf, howmuch, ev_vecs, 2,
 		    &chain, 1);
 
-		for (i=0; i < n_vecs; ++i)
+		for (i=0; i < nvecs; ++i)
 			WSABUF_FROM_EVBUFFER_IOV(&vecs[i], &ev_vecs[i]);
 #endif
 
diff --git a/buffer_iocp.c b/buffer_iocp.c
index 40007cd1..cdf2eaa5 100644
--- a/buffer_iocp.c
+++ b/buffer_iocp.c
@@ -249,12 +249,15 @@ evbuffer_launch_read(struct evbuffer *buf, size_t at_most,
 	buf_o->n_buffers = 0;
 	memset(buf_o->buffers, 0, sizeof(buf_o->buffers));
 
-	if (_evbuffer_expand_fast(buf, at_most) == -1)
+	if (_evbuffer_expand_fast(buf, at_most, 2) == -1)
 		goto done;
 	evbuffer_freeze(buf, 0);
 
+	/* XXX This and evbuffer_read_setup_vecs() should say MAX_WSABUFS,
+	 * not "2".  But commit_read() above can't handle more than two
+	 * buffers yet. */
 	nvecs = _evbuffer_read_setup_vecs(buf, at_most,
-	    vecs, &chain, 1);
+	    vecs, 2, &chain, 1);
 	for (i=0;i<nvecs;++i) {
 		WSABUF_FROM_EVBUFFER_IOV(
 			&buf_o->buffers[i],

From 17da042db6b8f7fec9064747dc53fa2855e96ab7 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Thu, 11 Mar 2010 15:39:44 -0500
Subject: [PATCH 6/6] Add some glass-box tests for the last_with_data code.

---
 test/regress_buffer.c | 77 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/test/regress_buffer.c b/test/regress_buffer.c
index 55f02f7e..a018d72d 100644
--- a/test/regress_buffer.c
+++ b/test/regress_buffer.c
@@ -328,6 +328,80 @@ end:
 	evbuffer_free(buf);
 }
 
+static void
+test_evbuffer_reserve_many(void *ptr)
+{
+	/* This is a glass-box test to handle expanding a buffer with more
+	 * chunks and reallocating chunks as needed */
+	struct evbuffer *buf = evbuffer_new();
+	struct evbuffer_iovec v[8];
+	int n;
+	size_t sz;
+	int add_data = ptr && !strcmp(ptr, "add");
+	int fill_first = ptr && !strcmp(ptr, "fill");
+	char *cp1, *cp2;
+
+	/* When reserving the the first chunk, we just allocate it */
+	n = evbuffer_reserve_space(buf, 128, v, 2);
+	evbuffer_validate(buf);
+	tt_int_op(n, ==, 1);
+	tt_assert(v[0].iov_len >= 128);
+	sz = v[0].iov_len;
+	cp1 = v[0].iov_base;
+	if (add_data) {
+		*(char*)v[0].iov_base = 'X';
+		v[0].iov_len = 1;
+		n = evbuffer_commit_space(buf, v, 1);
+		tt_int_op(n, ==, 0);
+	} else if (fill_first) {
+		memset(v[0].iov_base, 'X', v[0].iov_len);
+		n = evbuffer_commit_space(buf, v, 1);
+		tt_int_op(n, ==, 0);
+		n = evbuffer_reserve_space(buf, 128, v, 2);
+		tt_int_op(n, ==, 1);
+		sz = v[0].iov_len;
+		tt_assert(v[0].iov_base != cp1);
+		cp1 = v[0].iov_base;
+	}
+
+	/* Make another chunk get added. */
+	n = evbuffer_reserve_space(buf, sz+128, v, 2);
+	evbuffer_validate(buf);
+	tt_int_op(n, ==, 2);
+	sz = v[0].iov_len + v[1].iov_len;
+	tt_int_op(sz, >=, v[0].iov_len+128);
+	if (add_data) {
+		tt_assert(v[0].iov_base == cp1 + 1);
+	} else {
+		tt_assert(v[0].iov_base == cp1);
+	}
+	cp1 = v[0].iov_base;
+	cp2 = v[1].iov_base;
+
+	/* And a third chunk. */
+	n = evbuffer_reserve_space(buf, sz+128, v, 3);
+	evbuffer_validate(buf);
+	tt_int_op(n, ==, 3);
+	tt_assert(cp1 == v[0].iov_base);
+	tt_assert(cp2 == v[1].iov_base);
+	sz = v[0].iov_len + v[1].iov_len + v[2].iov_len;
+
+	/* Now force a reallocation by asking for more space in only 2
+	 * buffers. */
+	n = evbuffer_reserve_space(buf, sz+128, v, 2);
+	evbuffer_validate(buf);
+	if (add_data) {
+		tt_int_op(n, ==, 2);
+		tt_assert(cp1 == v[0].iov_base);
+	} else {
+		tt_int_op(n, ==, 1);
+	}
+
+end:
+	evbuffer_free(buf);
+}
+
+
 static int reference_cb_called;
 static void
 reference_cb(const void *data, size_t len, void *extra)
@@ -1277,6 +1351,9 @@ static const struct testcase_setup_t nil_setup = {
 struct testcase_t evbuffer_testcases[] = {
 	{ "evbuffer", test_evbuffer, 0, NULL, NULL },
 	{ "reserve2", test_evbuffer_reserve2, 0, NULL, NULL },
+	{ "reserve_many", test_evbuffer_reserve_many, 0, NULL, NULL },
+	{ "reserve_many2", test_evbuffer_reserve_many, 0, &nil_setup, (void*)"add" },
+	{ "reserve_many3", test_evbuffer_reserve_many, 0, &nil_setup, (void*)"fill" },
 	{ "reference", test_evbuffer_reference, 0, NULL, NULL },
 	{ "iterative", test_evbuffer_iterative, 0, NULL, NULL },
 	{ "readln", test_evbuffer_readln, TT_NO_LOGS, &basic_setup, NULL },