Merge remote-tracking branch 'origin/patches-2.0'

buffer.c

@@ -244,6 +244,31 @@ static inline int evbuffer_chains_all_empty(struct evbuffer_chain *chain) {
 }
 #endif
 
+/* Free all trailing chains in 'buf' that are neither pinned nor empty, prior
+ * to replacing them all with a new chain.  Return a pointer to the place
+ * where the new chain will go.
+ *
+ * Internal; requires lock.  The caller must fix up buf->last and buf->first
+ * as needed; they might have been freed.
+ */
+static struct evbuffer_chain **
+evbuffer_free_trailing_empty_chains(struct evbuffer *buf)
+{
+	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) {
+		EVUTIL_ASSERT(evbuffer_chains_all_empty(*ch));
+		evbuffer_free_all_chains(*ch);
+		*ch = NULL;
+	}
+	return ch;
+}
+
+/* Add a single chain 'chain' to the end of 'buf', freeing trailing empty
+ * chains as necessary.  Requires lock.  Does not schedule callbacks.
+ */
 static void
 evbuffer_chain_insert(struct evbuffer *buf,
     struct evbuffer_chain *chain)
@@ -1092,10 +1117,13 @@ evbuffer_remove_buffer(struct evbuffer *src, struct evbuffer *dst,
 
 	if (nread) {
 		/* we can remove the chain */
+		struct evbuffer_chain **chp;
+		chp = evbuffer_free_trailing_empty_chains(dst);
+
 		if (dst->first == NULL) {
 			dst->first = src->first;
 		} else {
-			dst->last->next = src->first;
+			*chp = src->first;
 		}
 		dst->last = previous;
 		previous->next = NULL;
@@ -1113,6 +1141,9 @@ evbuffer_remove_buffer(struct evbuffer *src, struct evbuffer *dst,
 	chain->off -= datlen;
 	nread += datlen;
 
+	/* You might think we would want to increment dst->n_add_for_cb
+	 * here too.  But evbuffer_add above already took care of that.
+	 */
 	src->total_len -= nread;
 	src->n_del_for_cb += nread;
 

test/regress_buffer.c

@@ -285,6 +285,43 @@ test_evbuffer(void *ptr)
 	evbuffer_free(evb_two);
 }
 
+static void
+no_cleanup(const void *data, size_t datalen, void *extra)
+{
+}
+
+static void
+test_evbuffer_remove_buffer_with_empty(void *ptr)
+{
+    struct evbuffer *src = evbuffer_new();
+    struct evbuffer *dst = evbuffer_new();
+    char buf[2];
+
+    evbuffer_validate(src);
+    evbuffer_validate(dst);
+
+    /* setup the buffers */
+    /* we need more data in src than we will move later */
+    evbuffer_add_reference(src, buf, sizeof(buf), no_cleanup, NULL);
+    evbuffer_add_reference(src, buf, sizeof(buf), no_cleanup, NULL);
+    /* we need one buffer in dst and one empty buffer at the end */
+    evbuffer_add(dst, buf, sizeof(buf));
+    evbuffer_add_reference(dst, buf, 0, no_cleanup, NULL);
+
+    evbuffer_validate(src);
+    evbuffer_validate(dst);
+
+    /* move three bytes over */
+    evbuffer_remove_buffer(src, dst, 3);
+
+    evbuffer_validate(src);
+    evbuffer_validate(dst);
+
+end:
+    evbuffer_free(src);
+    evbuffer_free(dst);
+}
+
 static void
 test_evbuffer_reserve2(void *ptr)
 {
@@ -1809,6 +1846,7 @@ static const struct testcase_setup_t nil_setup = {
 
 struct testcase_t evbuffer_testcases[] = {
 	{ "evbuffer", test_evbuffer, 0, NULL, NULL },
+	{ "remove_buffer_with_empty", test_evbuffer_remove_buffer_with_empty, 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" },