Merge remote-tracking branch 'origin/pr/338' into patches-2.0

* origin/pr/338:
  evbuffer_add: Use last_with_datap if set, not last.
  Use the free-trailing-chains function in evbuffer_insert_chain too
  test/regress: add tests for evbuffer_add() breakage on empty last chain

Fixes: #335

2 files changed, 97 insertions, 16 deletions

diff --git a/buffer.c b/buffer.c
index 8510955b..cda94458 100644
--- a/buffer.c
+++ b/buffer.c
@@ -298,21 +298,11 @@ evbuffer_chain_insert(struct evbuffer *buf,
 		EVUTIL_ASSERT(buf->first == NULL);
 		buf->first = buf->last = chain;
 	} else {
-		struct evbuffer_chain **ch = buf->last_with_datap;
-		/* Find the first victim chain.  It might be *last_with_datap */
-		while ((*ch) && ((*ch)->off != 0 || CHAIN_PINNED(*ch)))
-			ch = &(*ch)->next;
-		if (*ch == NULL) {
-			/* There is no victim; just append this new chain. */
-			buf->last->next = chain;
-			if (chain->off)
-				buf->last_with_datap = &buf->last->next;
-		} else {
-			/* Replace all victim chains with this chain. */
-			EVUTIL_ASSERT(evbuffer_chains_all_empty(*ch));
-			evbuffer_free_all_chains(*ch);
-			*ch = chain;
-		}
+		struct evbuffer_chain **chp;
+		chp = evbuffer_free_trailing_empty_chains(buf);
+		*chp = chain;
+		if (chain->off)
+			buf->last_with_datap = chp;
 		buf->last = chain;
 	}
 	buf->total_len += chain->off;
@@ -1558,7 +1548,11 @@ evbuffer_add(struct evbuffer *buf, const void *data_in, size_t datlen)
 		goto done;
 	}
 
-	chain = buf->last;
+	if (*buf->last_with_datap == NULL) {
+		chain = buf->last;
+	} else {
+		chain = *buf->last_with_datap;
+	}
 
 	/* If there are no chains allocated for this buffer, allocate one
 	 * big enough to hold all the data. */
diff --git a/test/regress_buffer.c b/test/regress_buffer.c
index d7cc5385..b6b4bb93 100644
--- a/test/regress_buffer.c
+++ b/test/regress_buffer.c
@@ -564,6 +564,60 @@ end:
 	evbuffer_free(buf);
 }
 
+static void
+test_evbuffer_add1(void *ptr)
+{
+	struct evbuffer *buf;
+	char *str;
+
+	buf = evbuffer_new();
+	evbuffer_add(buf, "1", 1);
+	evbuffer_validate(buf);
+	evbuffer_expand(buf, 2048);
+	evbuffer_validate(buf);
+	evbuffer_add(buf, "2", 1);
+	evbuffer_validate(buf);
+	evbuffer_add_printf(buf, "3");
+	evbuffer_validate(buf);
+
+	tt_assert(evbuffer_get_length(buf) == 3);
+	str = (char *)evbuffer_pullup(buf, -1);
+	tt_assert(str[0] == '1');
+	tt_assert(str[1] == '2');
+	tt_assert(str[2] == '3');
+end:
+	evbuffer_free(buf);
+}
+
+static void
+test_evbuffer_add2(void *ptr)
+{
+	struct evbuffer *buf;
+	static char data[4096];
+	int data_len = MIN_BUFFER_SIZE-EVBUFFER_CHAIN_SIZE-10;
+	char *str;
+	int len;
+
+	memset(data, 'P', sizeof(data));
+	buf = evbuffer_new();
+	evbuffer_add(buf, data, data_len);
+	evbuffer_validate(buf);
+	evbuffer_expand(buf, 100);
+	evbuffer_validate(buf);
+	evbuffer_add(buf, "2", 1);
+	evbuffer_validate(buf);
+	evbuffer_add_printf(buf, "3");
+	evbuffer_validate(buf);
+
+	len = evbuffer_get_length(buf);
+	tt_assert(len == data_len+2);
+	str = (char *)evbuffer_pullup(buf, -1);
+	tt_assert(str[len-3] == 'P');
+	tt_assert(str[len-2] == '2');
+	tt_assert(str[len-1] == '3');
+end:
+	evbuffer_free(buf);
+}
 
 static int reference_cb_called;
 static void
@@ -620,6 +674,36 @@ test_evbuffer_reference(void *ptr)
 	evbuffer_free(src);
 }
 
+static void
+test_evbuffer_reference2(void *ptr)
+{
+	struct evbuffer *buf;
+	static char data[4096];
+	int data_len = MIN_BUFFER_SIZE-EVBUFFER_CHAIN_SIZE-10;
+	char *str;
+	int len;
+
+	memset(data, 'P', sizeof(data));
+	buf = evbuffer_new();
+	evbuffer_add(buf, data, data_len);
+	evbuffer_validate(buf);
+	evbuffer_expand(buf, 100);
+	evbuffer_validate(buf);
+	evbuffer_add_reference(buf, "2", 1, no_cleanup, NULL);
+	evbuffer_validate(buf);
+	evbuffer_add_printf(buf, "3");
+	evbuffer_validate(buf);
+
+	len = evbuffer_get_length(buf);
+	tt_assert(len == data_len+2);
+	str = (char *)evbuffer_pullup(buf, -1);
+	tt_assert(str[len-3] == 'P');
+	tt_assert(str[len-2] == '2');
+	tt_assert(str[len-1] == '3');
+end:
+	evbuffer_free(buf);
+}
+
 int _evbuffer_testing_use_sendfile(void);
 int _evbuffer_testing_use_mmap(void);
 int _evbuffer_testing_use_linear_file_access(void);
@@ -1652,7 +1736,10 @@ struct testcase_t evbuffer_testcases[] = {
 	{ "reserve_many2", test_evbuffer_reserve_many, 0, &nil_setup, (void*)"add" },
 	{ "reserve_many3", test_evbuffer_reserve_many, 0, &nil_setup, (void*)"fill" },
 	{ "expand", test_evbuffer_expand, 0, NULL, NULL },
+	{ "add1", test_evbuffer_add1, 0, NULL, NULL },
+	{ "add2", test_evbuffer_add2, 0, NULL, NULL },
 	{ "reference", test_evbuffer_reference, 0, NULL, NULL },
+	{ "reference2", test_evbuffer_reference2, 0, NULL, NULL },
 	{ "iterative", test_evbuffer_iterative, 0, NULL, NULL },
 	{ "readln", test_evbuffer_readln, TT_NO_LOGS, &basic_setup, NULL },
 	{ "search_eol", test_evbuffer_search_eol, 0, NULL, NULL },