diff --git a/buffer.c b/buffer.c index 3e93a035..20454197 100644 --- a/buffer.c +++ b/buffer.c @@ -219,18 +219,20 @@ 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 */ 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; } else { - buf->previous_to_last = buf->last; buf->last->next = chain; buf->last = chain; } + if (chain->off) + buf->last_with_data = chain; } buf->total_len += chain->off; @@ -520,9 +522,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: @@ -531,50 +533,81 @@ 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) { - 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 CHAIN_SPACE_LEN(chain)) + goto done; + chain = chain->next; + } + /* pass 2: actually adjust all the chains. */ + chain = firstchain; + for (i=0; ioff += 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; @@ -589,7 +622,7 @@ 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) @@ -597,7 +630,7 @@ 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; \ } while (0) @@ -606,8 +639,8 @@ 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; \ (dst)->total_len += (src)->total_len; \ } while (0) @@ -618,11 +651,10 @@ 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) - int evbuffer_add_buffer(struct evbuffer *outbuf, struct evbuffer *inbuf) { @@ -734,6 +766,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; @@ -741,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; } @@ -789,6 +821,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); @@ -797,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); @@ -827,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; @@ -855,9 +888,12 @@ 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; previous = chain; chain = chain->next; } @@ -869,12 +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; @@ -907,6 +941,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 +1011,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); } @@ -984,16 +1021,19 @@ 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; + 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: @@ -1380,8 +1420,11 @@ 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) + buf->last_with_data = tmp; + tmp->next = chain; tmp->off = datlen; @@ -1462,9 +1505,9 @@ 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; evbuffer_chain_free(chain); @@ -1475,17 +1518,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); @@ -1494,56 +1541,89 @@ _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; - 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->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 { + buf->last = buf->last_with_data; + buf->last_with_data->next = NULL; + } + return (-1); + } + + if (rmv_all) { + buf->first = buf->last = buf->last_with_data = tmp; + } else { + buf->last_with_data->next = tmp; + buf->last = tmp; + } + return (0); + } } /* @@ -1559,17 +1639,18 @@ _evbuffer_expand_fast(struct evbuffer *buf, size_t datlen) #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 @@ -1578,7 +1659,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' @@ -1587,52 +1669,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 @@ -1645,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 @@ -1688,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) == -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, - &chain, 1); + 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]; - nvecs = _evbuffer_read_setup_vecs(buf, howmuch, ev_vecs, + 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 < nvecs; ++i) + WSABUF_FROM_EVBUFFER_IOV(&vecs[i], &ev_vecs[i]); #endif #ifdef WIN32 @@ -1762,19 +1824,22 @@ 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; + remaining -= space; } else { - chain->off += n; + chain->off += remaining; + buf->last_with_data = chain; + break; } - } else { - chain->off += n; + chain = chain->next; } #else chain->off += n; + buf->last_with_data = chain; #endif buf->total_len += n; buf->n_add_for_cb += n; @@ -1792,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; @@ -1803,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) 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;ibuffers[i], diff --git a/evbuffer-internal.h b/evbuffer-internal.h index 894dc2f1..e8555dbd 100644 --- a/evbuffer-internal.h +++ b/evbuffer-internal.h @@ -73,16 +73,11 @@ 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. If the buffer has no + * chains, this is NULL. */ + struct evbuffer_chain *last_with_data; /** Total amount of bytes stored in all chains.*/ size_t total_len; @@ -234,8 +229,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. @@ -244,7 +239,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 a89b818a..a018d72d 100644 --- a/test/regress_buffer.c +++ b/test/regress_buffer.c @@ -66,31 +66,44 @@ 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; } + 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 +252,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 +268,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,10 +284,12 @@ 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. */ 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); @@ -283,7 +301,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)); @@ -310,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) @@ -358,6 +450,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 +487,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 +805,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 +816,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 +963,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 +1036,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 +1048,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 +1064,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. */ @@ -1250,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 },