diff --git a/buffer.c b/buffer.c index 2b5a8255..41d4bc27 100644 --- a/buffer.c +++ b/buffer.c @@ -330,6 +330,7 @@ evbuffer_remove_buffer(struct evbuffer *src, struct evbuffer *dst, return (nread); } +/* XXX shouldn't the second arg be ssize_t? */ u_char * evbuffer_pullup(struct evbuffer *buf, int size) { @@ -338,12 +339,17 @@ evbuffer_pullup(struct evbuffer *buf, int size) if (size == -1) size = buf->total_len; + /* XXX Does it make more sense, if size > buf->total_len, to + * clip it downwards? */ if (size == 0 || size > buf->total_len) return (NULL); + /* No need to pull up anything; the first size bytes are already here. */ if (chain->off >= size) return chain->buffer + chain->misalign; + /* XXX is it possible that buf->chain will already have enough space + * to hold size bytes? */ if ((tmp = evbuffer_chain_new(size)) == NULL) { event_warn("%s: out of memory\n", __func__); return (NULL); @@ -352,6 +358,7 @@ evbuffer_pullup(struct evbuffer *buf, int size) tmp->off = size; buffer = tmp->buffer; + /* Copy and free every chunk that will be entirely pulled into tmp */ for (chain = buf->first; chain != NULL && size >= chain->off; chain = next) { next = chain->next; @@ -563,6 +570,8 @@ evbuffer_add(struct evbuffer *buf, const void *data_in, size_t datlen) const u_char *data = data_in; size_t old_len = buf->total_len, remain, to_alloc; + /* If there are no chains allocated for this buffer, allocate one + * big enough to hold all the data. */ if (chain == NULL) { if (evbuffer_expand(buf, datlen) == -1) return (-1); @@ -571,7 +580,7 @@ evbuffer_add(struct evbuffer *buf, const void *data_in, size_t datlen) remain = chain->buffer_len - chain->misalign - chain->off; if (remain >= datlen) { - /* we have enough space */ + /*there's enough space to hold all the data in the current last chain*/ memcpy(chain->buffer + chain->misalign + chain->off, data, datlen); chain->off += datlen; @@ -580,6 +589,7 @@ evbuffer_add(struct evbuffer *buf, const void *data_in, size_t datlen) } /* we need to add another chain */ + /* XXX Does this double the length of every successive chain? */ to_alloc = chain->buffer_len << 1; if (datlen > to_alloc) to_alloc = datlen; @@ -592,7 +602,7 @@ evbuffer_add(struct evbuffer *buf, const void *data_in, size_t datlen) memcpy(chain->buffer + chain->misalign + chain->off, data, remain); chain->off += remain; - + data += remain; datlen -= remain; @@ -628,6 +638,9 @@ evbuffer_prepend(struct evbuffer *buf, const void *data, size_t datlen) chain->misalign -= datlen; } else { struct evbuffer_chain *tmp; + /* XXX we should copy as much of the data into chain as possible + * before we put any into tmp. */ + /* we need to add another chain */ if ((tmp = evbuffer_chain_new(datlen)) == NULL) return (-1); @@ -647,6 +660,8 @@ evbuffer_prepend(struct evbuffer *buf, const void *data, size_t datlen) return (0); } +/** Helper: realigns the memory in chain->buffer so that misalign is + * 0. */ static void evbuffer_chain_align(struct evbuffer_chain *chain) { @@ -677,16 +692,16 @@ evbuffer_expand(struct evbuffer *buf, size_t datlen) if (chain->buffer_len >= need) return (0); - /* - * If the misalignment fulfills our data needs, we just force an - * alignment to happen. Afterwards, we have enough space. + /* If the misalignment plus the remaining space fulfils our data needs, we + * just force an alignment to happen. Afterwards, we have enough space. */ - if (chain->misalign >= datlen) { + if (chain->buffer_len - chain->off >= datlen) { evbuffer_chain_align(chain); return (0); } - /* avoid a memcpy if we can just a new chain */ + /* avoid a memcpy if we can just append a new chain */ + /* XXX in practice, does this result in lots of leftover space? */ length = chain->buffer_len << 1; if (length < datlen) length = datlen; @@ -734,11 +749,13 @@ evbuffer_read(struct evbuffer *buf, evutil_socket_t fd, int howmuch) else if (n > chain->buffer_len << 2) n = chain->buffer_len << 2; } -#endif +#endif if (howmuch < 0 || howmuch > n) howmuch = n; /* If we don't have FIONREAD, we might waste some space here */ + /* XXX we _will_ waste some space here if there is any space left + * over on buf->last. */ if (evbuffer_expand(buf, howmuch) == -1) return (-1); @@ -777,6 +794,9 @@ evbuffer_write(struct evbuffer *buffer, evutil_socket_t fd) struct iovec iov[NUM_IOVEC]; struct evbuffer_chain *chain = buffer->first; int i = 0; + /* 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) { iov[i].iov_base = chain->buffer + chain->misalign; iov[i++].iov_len = chain->off; @@ -885,7 +905,8 @@ evbuffer_add_printf(struct evbuffer *buf, const char *fmt, ...) return (res); } -void evbuffer_setcb(struct evbuffer *buffer, +void +evbuffer_setcb(struct evbuffer *buffer, void (*cb)(struct evbuffer *, size_t, size_t, void *), void *cbarg) { diff --git a/configure.in b/configure.in index fa37e13a..e6582a03 100644 --- a/configure.in +++ b/configure.in @@ -39,7 +39,7 @@ AC_CHECK_LIB(nsl, inet_ntoa) dnl Checks for header files. AC_HEADER_STDC -AC_CHECK_HEADERS(fcntl.h stdarg.h inttypes.h stdint.h poll.h signal.h unistd.h sys/epoll.h sys/time.h sys/queue.h sys/event.h sys/param.h sys/ioctl.h sys/select.h sys/devpoll.h port.h netinet/in6.h sys/socket.h sys/uio.h) +AC_CHECK_HEADERS(fcntl.h stdarg.h inttypes.h stdint.h stddef.h poll.h signal.h unistd.h sys/epoll.h sys/time.h sys/queue.h sys/event.h sys/param.h sys/ioctl.h sys/select.h sys/devpoll.h port.h netinet/in6.h sys/socket.h sys/uio.h) if test "x$ac_cv_header_sys_queue_h" = "xyes"; then AC_MSG_CHECKING(for TAILQ_FOREACH in sys/queue.h) AC_EGREP_CPP(yes, diff --git a/evbuffer-internal.h b/evbuffer-internal.h index 25901da1..5df1b1a2 100644 --- a/evbuffer-internal.h +++ b/evbuffer-internal.h @@ -32,6 +32,7 @@ extern "C" { #endif #include "config.h" +#include "evutil.h" /* minimum allocation */ #define MIN_BUFFER_SIZE 256 @@ -55,18 +56,16 @@ struct evbuffer_chain { /** points to next buffer in the chain */ struct evbuffer_chain *next; - size_t buffer_len; + size_t buffer_len; /**< total allocation available in the buffer field. */ - size_t misalign;/** unused space at the beginning of buffer */ - size_t off; /** write pointer into buffer + misalign */ + size_t misalign; /**< unused space at the beginning of buffer */ + size_t off; /**< Offset into buffer + misalign at which to start writing. + * In other words, the total number of bytes actually stored + * in buffer. */ u_char buffer[1]; }; -#ifndef offsetof -#define offsetof(type, field) ((size_t)(&((type *)0)->field)) -#endif - #define EVBUFFER_CHAIN_SIZE offsetof(struct evbuffer_chain, buffer[0]) #ifdef __cplusplus diff --git a/evutil.h b/evutil.h index 54295014..d597db0a 100644 --- a/evutil.h +++ b/evutil.h @@ -50,6 +50,9 @@ extern "C" { #ifdef _EVENT_HAVE_SYS_TYPES_H #include #endif +#ifdef _EVENT_HAVE_STDDEF_H +#include +#endif #ifdef _EVENT_HAVE_UINT64_T #define ev_uint64_t uint64_t @@ -171,6 +174,12 @@ int evutil_make_socket_nonblocking(evutil_socket_t sock); #define evutil_timerisset(tvp) ((tvp)->tv_sec || (tvp)->tv_usec) #endif +#ifdef offsetof +#define evutil_offsetof(type, field) offsetof(type, field) +#else +#define evutil_offsetof(type, field) ((off_t)(&((type *)0)->field)) +#endif + /* big-int related functions */ ev_int64_t evutil_strtoll(const char *s, char **endptr, int base); diff --git a/include/event2/buffer.h b/include/event2/buffer.h index 7cd631f0..09276538 100644 --- a/include/event2/buffer.h +++ b/include/event2/buffer.h @@ -95,7 +95,8 @@ size_t evbuffer_length(struct evbuffer *buf); /** Expands the available space in an event buffer. - Expands the available space in the event buffer to at least datlen + Expands the available space in the event buffer to at least datlen, so that + appending datlen additional bytes will not require any new allocations. @param buf the event buffer to be expanded @param datlen the new minimum length requirement @@ -271,10 +272,11 @@ void evbuffer_setcb(struct evbuffer *buffer, void (*cb)(struct evbuffer *, size_t, size_t, void *), void *cbarg); /** - Makes the memory in an evbuffer contiguous + Makes the memory at the begging of an evbuffer contiguous @param buf the evbuffer to make contiguous - @param size the number of bytes to make contiguous + @param size the number of bytes to make contiguous, or -1 to make the + entire buffer contiguous. @return a pointer to the contigous memory areay */ @@ -300,6 +302,18 @@ int evbuffer_prepend(struct evbuffer *buf, const void *data, size_t size); void evbuffer_prepend_buffer(struct evbuffer *dst, struct evbuffer* src); +/* XXX missing APIs: + + A better find-string that returns a smart offset structure rather than a + pointer. It should also be able to start searching _at_ an offset. + + A variant of evbuffer_write() that takes a maximum number of bytes to + write. + + A check-representation functions for testing, so we can assert() that + nothing has gone screwy inside an evbuffer. +*/ + /** deprecated in favor of calling the functions directly */ #define EVBUFFER_LENGTH(x) evbuffer_length(x) #define EVBUFFER_DATA(x) evbuffer_pullup(x, -1)