diff --git a/buffer.c b/buffer.c index aa81cdd8..173ebb65 100644 --- a/buffer.c +++ b/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; diff --git a/test/regress_buffer.c b/test/regress_buffer.c index 275bd3d0..eb35c33c 100644 --- a/test/regress_buffer.c +++ b/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" },