r15242@tombo: nickm | 2008-04-18 09:24:44 -0400

 Make tagging code thread-safe, and fix a bug in encode_int_internal

svn:r721

diff --git a/ChangeLog b/ChangeLog
index 7af9c6f0c40dc23b65c63717acb6a85e531901eb..6a95909d06bc3b107891f1d87c7ed5a54f66c346 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -67,6 +67,7 @@ Changes in current version:
  o Correctly handle timeouts larger than 35 minutes on Linux with epoll.c.  This is probably a kernel defect, but we'll have to support old kernels anyway even if it gets fixed.
  o Make name_from_addr() threadsafe in http.c
  o Add new thread-safe interfaces to evdns functions.
+ o Make all event_tagging interfaces threadsafe.
 
 Changes in 1.4.0:
  o allow \r or \n individually to separate HTTP headers instead of the standard "\r\n"; from Charles Kerr.

diff --git a/event_tagging.c b/event_tagging.c
index 955a63fea77986ea40a1804653719f36f091e788..8e4f383dcc533658891b6d3c240e9dee21589c4c 100644 (file)
--- a/event_tagging.c
+++ b/event_tagging.c
@@ -71,22 +71,20 @@ int evtag_decode_int(ev_uint32_t *pnumber, struct evbuffer *evbuf);
 int evtag_encode_tag(struct evbuffer *evbuf, ev_uint32_t tag);
 int evtag_decode_tag(ev_uint32_t *ptag, struct evbuffer *evbuf);
 
-static struct evbuffer *_buf;  /* not thread safe */
-
 void
 evtag_init(void)
 {
-       if (_buf != NULL)
-               return;
-
-       if ((_buf = evbuffer_new()) == NULL)
-               event_err(1, "%s: malloc", __func__);
 }
 
-/* 
- * We encode integer's by nibbles; the first nibble contains the number
+/*
+ * We encode integers by nibbles; the first nibble contains the number
  * of significant nibbles - 1;  this allows us to encode up to 64-bit
  * integers.  This function is byte-order independent.
+ *
+ * @param number a 32-bit unsigned integer to encode
+ * @param data a pointer to where the data should be written.  Must
+ *    have at least 5 bytes free.
+ * @return the number of bytes written into data.
  */
 
 static inline int
@@ -94,7 +92,7 @@ encode_int_internal(ev_uint8_t *data, ev_uint32_t number)
 {
        int off = 1, nibbles = 0;
 
-       memset(data, 0, sizeof(data));
+       memset(data, 0, sizeof(uint32_t)+1);
        while (number) {
                if (off & 0x1)
                        data[off/2] = (data[off/2] & 0xf0) | (number & 0x0f);
@@ -245,6 +243,14 @@ evtag_marshal_timeval(struct evbuffer *evbuf, ev_uint32_t tag, struct timeval *t
        evtag_marshal(evbuf, tag, data, len);
 }
 
+/* Internal: decode an integer from an evbuffer, without draining it.
+ *  Only integers up to 32-bits are supported.
+ *
+ * @param evbuf the buffer to read from
+ * @param offset an index into the buffer at which we should start reading.
+ * @param pnumber a pointer to receive the integer.
+ * @return The length of the number as encoded, or -1 on error.
+ */
 static int
 decode_int_internal(ev_uint32_t *pnumber, struct evbuffer *evbuf, int offset)
 {
@@ -386,6 +392,7 @@ evtag_unmarshal_int(struct evbuffer *evbuf, ev_uint32_t need_tag,
        ev_uint32_t tag;
        ev_uint32_t len;
        ev_uint32_t integer;
+       int result;
 
        if (decode_tag_internal(&tag, evbuf, 1 /* dodrain */) == -1)
                return (-1);
@@ -397,14 +404,13 @@ evtag_unmarshal_int(struct evbuffer *evbuf, ev_uint32_t need_tag,
 
        if (EVBUFFER_LENGTH(evbuf) < len)
                return (-1);
-       
-       evbuffer_drain(_buf, EVBUFFER_LENGTH(_buf));
-       if (evbuffer_add(_buf, evbuffer_pullup(evbuf, len), len) == -1)
-               return (-1);
 
+       result = decode_int_internal(pinteger, evbuf, 0);
        evbuffer_drain(evbuf, len);
-
-       return (evtag_decode_int(pinteger, _buf));
+       if (result < 0 || result > len) /* XXX Should this be != rather than > ?*/
+               return (-1);
+       else
+               return result;
 }
 
 /* Unmarshal a fixed length tag */
@@ -454,17 +460,24 @@ evtag_unmarshal_timeval(struct evbuffer *evbuf, ev_uint32_t need_tag,
 {
        ev_uint32_t tag;
        ev_uint32_t integer;
+       int len, offset, offset2;
+       int result = -1;
 
-       evbuffer_drain(_buf, EVBUFFER_LENGTH(_buf));
-       if (evtag_unmarshal(evbuf, &tag, _buf) == -1 || tag != need_tag)
-               return (-1);
-
-       if (evtag_decode_int(&integer, _buf) == -1)
+       if ((len = evtag_unmarshal_header(evbuf, &tag)) == -1)
                return (-1);
+       if (tag != need_tag)
+               goto done;
+       if ((offset = decode_int_internal(&integer, evbuf, 0)) == -1)
+               goto done;
        ptv->tv_sec = integer;
-       if (evtag_decode_int(&integer, _buf) == -1)
-               return (-1);
+       if ((offset2 = decode_int_internal(&integer, evbuf, offset)) == -1)
+               goto done;
        ptv->tv_usec = integer;
+       if (offset + offset2 > len) /* XXX Should this be != instead of > ? */
+               goto done;
 
-       return (0);
+       result = 0;
+ done:
+       evbuffer_drain(evbuf, len);
+       return result;
 }