Refactor the isc_buffer_{get,put}uintN, add isc_buffer_peekuintN

The Stream DNS implementation needs a peek methods that read the value
from the buffer, but it doesn't advance the current position.  Add
isc_buffer_peekuintX methods, refactor the isc_buffer_{get,put}uintN
methods to modern integer types, and move the isc_buffer_getuintN to the
header as static inline functions.

diff --git a/lib/isc/buffer.c b/lib/isc/buffer.c
index 87411c890cc5554a27be14a7ef09f7fb4ee9619a..0cd4a24ee8af38ca2f726b483155a04913d1626d 100644 (file)
--- a/lib/isc/buffer.c
+++ b/lib/isc/buffer.c
@@ -10,7 +10,6 @@
  * See the COPYRIGHT file distributed with this work for additional
  * information regarding copyright ownership.
  */
-
 /*! \file */
 
 #include <inttypes.h>
@@ -82,94 +81,6 @@ isc_buffer_compact(isc_buffer_t *b) {
        b->used = length;
 }
 
-uint8_t
-isc_buffer_getuint8(isc_buffer_t *b) {
-       unsigned char *cp;
-       uint8_t result;
-
-       /*
-        * Read an unsigned 8-bit integer from 'b' and return it.
-        */
-
-       REQUIRE(ISC_BUFFER_VALID(b));
-       REQUIRE(b->used - b->current >= 1);
-
-       cp = isc_buffer_current(b);
-       b->current += 1;
-       result = ((uint8_t)(cp[0]));
-
-       return (result);
-}
-
-uint16_t
-isc_buffer_getuint16(isc_buffer_t *b) {
-       unsigned char *cp;
-       uint16_t result;
-
-       /*
-        * Read an unsigned 16-bit integer in network byte order from 'b',
-        * convert it to host byte order, and return it.
-        */
-
-       REQUIRE(ISC_BUFFER_VALID(b));
-       REQUIRE(b->used - b->current >= 2);
-
-       cp = isc_buffer_current(b);
-       b->current += 2;
-       result = ((unsigned int)(cp[0])) << 8;
-       result |= ((unsigned int)(cp[1]));
-
-       return (result);
-}
-
-uint32_t
-isc_buffer_getuint32(isc_buffer_t *b) {
-       unsigned char *cp;
-       uint32_t result;
-
-       /*
-        * Read an unsigned 32-bit integer in network byte order from 'b',
-        * convert it to host byte order, and return it.
-        */
-
-       REQUIRE(ISC_BUFFER_VALID(b));
-       REQUIRE(b->used - b->current >= 4);
-
-       cp = isc_buffer_current(b);
-       b->current += 4;
-       result = ((unsigned int)(cp[0])) << 24;
-       result |= ((unsigned int)(cp[1])) << 16;
-       result |= ((unsigned int)(cp[2])) << 8;
-       result |= ((unsigned int)(cp[3]));
-
-       return (result);
-}
-
-uint64_t
-isc_buffer_getuint48(isc_buffer_t *b) {
-       unsigned char *cp;
-       uint64_t result;
-
-       /*
-        * Read an unsigned 48-bit integer in network byte order from 'b',
-        * convert it to host byte order, and return it.
-        */
-
-       REQUIRE(ISC_BUFFER_VALID(b));
-       REQUIRE(b->used - b->current >= 6);
-
-       cp = isc_buffer_current(b);
-       b->current += 6;
-       result = ((int64_t)(cp[0])) << 40;
-       result |= ((int64_t)(cp[1])) << 32;
-       result |= ((int64_t)(cp[2])) << 24;
-       result |= ((int64_t)(cp[3])) << 16;
-       result |= ((int64_t)(cp[4])) << 8;
-       result |= ((int64_t)(cp[5]));
-
-       return (result);
-}
-
 void
 isc_buffer_putdecint(isc_buffer_t *b, int64_t v) {
        unsigned int l = 0;

diff --git a/lib/isc/include/isc/buffer.h b/lib/isc/include/isc/buffer.h
index 8548d533c769a3dc0d366b831d6971626e16d41e..71a03db11d97c5e8cba1cc832376abc13fe94efc 100644 (file)
--- a/lib/isc/include/isc/buffer.h
+++ b/lib/isc/include/isc/buffer.h
@@ -104,6 +104,7 @@
 #include <stdbool.h>
 
 #include <isc/assertions.h>
+#include <isc/endian.h>
 #include <isc/formatcheck.h>
 #include <isc/lang.h>
 #include <isc/list.h>
@@ -282,89 +283,143 @@ isc_buffer_compact(isc_buffer_t *b);
  *     are those of the remaining region (as it was before the call).
  */
 
-uint8_t
+static inline isc_result_t
+isc_buffer_peekuint8(const isc_buffer_t *b, uint8_t *valp);
+static inline uint8_t
 isc_buffer_getuint8(isc_buffer_t *b);
+static inline void
+isc_buffer_putuint8(isc_buffer_t *b, uint8_t val);
 /*!<
- * \brief Read an unsigned 8-bit integer from 'b' and return it.
+ * \brief Peek/Read/Write an unsigned 8-bit integer from/to 'b'.
  *
  * Requires:
  *
  *\li  'b' is a valid buffer.
  *
- *\li  The length of the remaining region of 'b' is at least 1.
+ *\li  The length of the available region of 'b' is at least 1
+ *     or the buffer has autoreallocation enabled.
  *
- * Ensures:
+ * Ensures (for Read):
  *
  *\li  The current pointer in 'b' is advanced by 1.
  *
- * Returns:
+ * Ensures (for Write):
  *
- *\li  A 8-bit unsigned integer.
+ *\li  The used pointer in 'b' is advanced by 1.
+ *
+ * Returns (for Peek and Read):
+ *
+ *\li  A 8-bit unsigned integer. (peek and get)
  */
 
-uint16_t
+static inline isc_result_t
+isc_buffer_peekuint16(const isc_buffer_t *b, uint16_t *valp);
+static inline uint16_t
 isc_buffer_getuint16(isc_buffer_t *b);
+static inline void
+isc_buffer_putuint16(isc_buffer_t *b, uint16_t val);
 /*!<
- * \brief Read an unsigned 16-bit integer in network byte order from 'b',
- * convert it to host byte order, and return it.
+ * \brief Peek/Read/Write an unsigned 16-bit integer in network byte order
+ * from/to 'b', convert it to/from host byte order..
  *
  * Requires:
  *
  *\li  'b' is a valid buffer.
  *
- *\li  The length of the remaining region of 'b' is at least 2.
+ *\li  The length of the available region of 'b' is at least 2
+ *     or the buffer has autoreallocation enabled.
  *
- * Ensures:
+ * Ensures (for Read):
  *
  *\li  The current pointer in 'b' is advanced by 2.
  *
- * Returns:
+ * Ensures (for Write):
+ *
+ *\li  The used pointer in 'b' is advanced by 2.
+ *
+ * Returns (for Peek and Read):
  *
  *\li  A 16-bit unsigned integer.
  */
 
-uint32_t
+static inline isc_result_t
+isc_buffer_peekuint32(const isc_buffer_t *b, uint32_t *valp);
+static inline uint32_t
 isc_buffer_getuint32(isc_buffer_t *b);
+static inline void
+isc_buffer_putuint32(isc_buffer_t *b, uint32_t val);
 /*!<
- * \brief Read an unsigned 32-bit integer in network byte order from 'b',
- * convert it to host byte order, and return it.
+ * \brief Peek/Read/Write an unsigned 32-bit integer in network byte order
+ * from/to 'b', convert it to/from host byte order.
  *
  * Requires:
  *
  *\li  'b' is a valid buffer.
  *
- *\li  The length of the remaining region of 'b' is at least 4.
+ *\li  The length of the available region of 'b' is at least 4
+ *     or the buffer has autoreallocation enabled.
  *
- * Ensures:
+ * Ensures (for Read):
  *
  *\li  The current pointer in 'b' is advanced by 4.
  *
- * Returns:
+ * Ensures (for Write):
+ *
+ *\li  The used pointer in 'b' is advanced by 4.
+ *
+ * Returns (for Peek and Read):
  *
  *\li  A 32-bit unsigned integer.
  */
 
-uint64_t
+static inline isc_result_t
+isc_buffer_peekuint48(const isc_buffer_t *b, uint64_t *valp);
+static inline uint64_t
 isc_buffer_getuint48(isc_buffer_t *b);
+static inline void
+isc_buffer_putuint48(isc_buffer_t *b, uint64_t val);
 /*!<
- * \brief Read an unsigned 48-bit integer in network byte order from 'b',
- * convert it to host byte order, and return it.
+ * \brief Peek/Read/Write an unsigned 48-bit integer in network byte order
+ * from/to 'b', convert it to/from host byte order.
  *
  * Requires:
  *
  *\li  'b' is a valid buffer.
  *
- *\li  The length of the remaining region of 'b' is at least 6.
+ *\li  The length of the available region of 'b' is at least 6
+ *     or the buffer has autoreallocation enabled.
  *
- * Ensures:
+ * Ensures (for Read):
  *
  *\li  The current pointer in 'b' is advanced by 6.
  *
- * Returns:
+ * Ensures (for Write):
+ *
+ *\li  The used pointer in 'b' is advanced by 6.
+ *
+ * Returns (for Peek and Read):
  *
  *\li  A 48-bit unsigned integer (stored in a 64-bit integer).
  */
 
+static inline void
+isc_buffer_putmem(isc_buffer_t *b, const unsigned char *base,
+                 unsigned int length);
+/*!<
+ * \brief Copy 'length' bytes of memory at 'base' into 'b'.
+ *
+ * Requires:
+ *\li  'b' is a valid buffer.
+ *
+ *\li  'base' points to 'length' bytes of valid memory.
+ *
+ *\li  The length of the available region of 'b' is at least 'length'
+ *     or the buffer has autoreallocation enabled.
+ *
+ * Ensures:
+ *\li  The used pointer in 'b' is advanced by 'length'.
+ */
+
 void
 isc_buffer_putdecint(isc_buffer_t *b, int64_t v);
 /*!<
@@ -782,193 +837,156 @@ isc_buffer_back(isc_buffer_t *b, unsigned int n) {
        b->current -= n;
 }
 
-/*!
- * \brief Store an unsigned 8-bit integer from 'val' into 'b'.
- *
- * Requires:
- *\li  'b' is a valid buffer.
- *
- *\li  The length of the available region of 'b' is at least 1
- *     or the buffer has autoreallocation enabled.
- *
- * Ensures:
- *\li  The used pointer in 'b' is advanced by 1.
- */
-static inline void
-isc_buffer_putuint8(isc_buffer_t *b, uint8_t val) {
-       unsigned char *cp;
+#define ISC_BUFFER_PEEK_CHECK(b, s)                 \
+       {                                           \
+               REQUIRE(ISC_BUFFER_VALID(b));       \
+               if ((b)->used - (b)->current < s) { \
+                       return (ISC_R_NOMORE);      \
+               }                                   \
+       }
 
-       ISC_REQUIRE(ISC_BUFFER_VALID(b));
+static inline isc_result_t
+isc_buffer_peekuint8(const isc_buffer_t *b, uint8_t *valp) {
+       ISC_BUFFER_PEEK_CHECK(b, sizeof(*valp));
 
-       if (b->autore) {
-               isc_buffer_t *tmp = b;
-               ISC_REQUIRE(isc_buffer_reserve(tmp, 1) == ISC_R_SUCCESS);
+       uint8_t *cp = isc_buffer_current(b);
+       if (valp != NULL) {
+               *valp = (uint8_t)(cp[0]);
        }
+       return (ISC_R_SUCCESS);
+}
 
-       ISC_REQUIRE(isc_buffer_availablelength(b) >= 1U);
+static inline uint8_t
+isc_buffer_getuint8(isc_buffer_t *b) {
+       uint8_t      val = 0;
+       isc_result_t result = isc_buffer_peekuint8(b, &val);
+       ENSURE(result == ISC_R_SUCCESS);
+       b->current += sizeof(val);
+       return (val);
+}
 
-       cp = isc_buffer_used(b);
-       b->used++;
+#define ISC_BUFFER_PUT_RESERVE(b, v)                                           \
+       {                                                                      \
+               REQUIRE(ISC_BUFFER_VALID(b));                                  \
+                                                                               \
+               if (b->autore) {                                               \
+                       isc_result_t result = isc_buffer_reserve(b,            \
+                                                                sizeof(val)); \
+                       ENSURE(result == ISC_R_SUCCESS);                       \
+               }                                                              \
+                                                                               \
+               REQUIRE(isc_buffer_availablelength(b) >= sizeof(val));         \
+       }
+
+static inline void
+isc_buffer_putuint8(isc_buffer_t *b, uint8_t val) {
+       ISC_BUFFER_PUT_RESERVE(b, val);
+
+       uint8_t *cp = isc_buffer_used(b);
+       b->used += sizeof(val);
        cp[0] = val;
 }
 
-/*!
- * \brief Store an unsigned 16-bit integer in host byte order from 'val'
- * into 'b' in network byte order.
- *
- * Requires:
- *\li  'b' is a valid buffer.
- *
- *\li  The length of the available region of 'b' is at least 2
- *     or the buffer has autoreallocation enabled.
- *
- * Ensures:
- *\li  The used pointer in 'b' is advanced by 2.
- */
-static inline void
-isc_buffer_putuint16(isc_buffer_t *b, uint16_t val) {
-       unsigned char *cp;
+static inline isc_result_t
+isc_buffer_peekuint16(const isc_buffer_t *b, uint16_t *valp) {
+       ISC_BUFFER_PEEK_CHECK(b, sizeof(*valp));
 
-       ISC_REQUIRE(ISC_BUFFER_VALID(b));
+       uint8_t *cp = isc_buffer_current(b);
 
-       if (b->autore) {
-               isc_buffer_t *tmp = b;
-               ISC_REQUIRE(isc_buffer_reserve(tmp, 2) == ISC_R_SUCCESS);
+       if (valp != NULL) {
+               *valp = ISC_U8TO16_BE(cp);
        }
+       return (ISC_R_SUCCESS);
+}
 
-       ISC_REQUIRE(isc_buffer_availablelength(b) >= 2U);
-
-       cp = isc_buffer_used(b);
-       b->used += 2;
-       cp[0] = (unsigned char)(val >> 8);
-       cp[1] = (unsigned char)val;
+static inline uint16_t
+isc_buffer_getuint16(isc_buffer_t *b) {
+       uint16_t     val = 0;
+       isc_result_t result = isc_buffer_peekuint16(b, &val);
+       ENSURE(result == ISC_R_SUCCESS);
+       b->current += sizeof(val);
+       return (val);
 }
 
-/*!
- * Store an unsigned 24-bit integer in host byte order from 'val'
- * into 'b' in network byte order.
- *
- * Requires:
- *\li  'b' is a valid buffer.
- *
- *\li  The length of the available region of 'b' is at least 3
- *     or the buffer has autoreallocation enabled.
- *
- * Ensures:
- *\li  The used pointer in 'b' is advanced by 3.
- */
 static inline void
-isc_buffer_putuint24(isc_buffer_t *b, uint32_t val) {
-       unsigned char *cp;
+isc_buffer_putuint16(isc_buffer_t *b, uint16_t val) {
+       ISC_BUFFER_PUT_RESERVE(b, val);
 
-       ISC_REQUIRE(ISC_BUFFER_VALID(b));
+       uint8_t *cp = isc_buffer_used(b);
+       b->used += sizeof(val);
+       ISC_U16TO8_BE(cp, val);
+}
 
-       if (b->autore) {
-               isc_buffer_t *tmp = b;
-               ISC_REQUIRE(isc_buffer_reserve(tmp, 3) == ISC_R_SUCCESS);
-       }
+static inline isc_result_t
+isc_buffer_peekuint32(const isc_buffer_t *b, uint32_t *valp) {
+       ISC_BUFFER_PEEK_CHECK(b, sizeof(*valp));
 
-       ISC_REQUIRE(isc_buffer_availablelength(b) >= 3U);
+       uint8_t *cp = isc_buffer_current(b);
 
-       cp = isc_buffer_used(b);
-       b->used += 3;
-       cp[0] = (unsigned char)(val >> 16);
-       cp[1] = (unsigned char)(val >> 8);
-       cp[2] = (unsigned char)val;
+       if (valp != NULL) {
+               *valp = ISC_U8TO32_BE(cp);
+       }
+       return (ISC_R_SUCCESS);
+}
+
+uint32_t
+isc_buffer_getuint32(isc_buffer_t *b) {
+       uint32_t     val = 0;
+       isc_result_t result = isc_buffer_peekuint32(b, &val);
+       ENSURE(result == ISC_R_SUCCESS);
+       b->current += sizeof(val);
+       return (val);
 }
 
-/*!
- * \brief Store an unsigned 32-bit integer in host byte order from 'val'
- * into 'b' in network byte order.
- *
- * Requires:
- *\li  'b' is a valid buffer.
- *
- *\li  The length of the available region of 'b' is at least 4
- *     or the buffer has autoreallocation enabled.
- *
- * Ensures:
- *\li  The used pointer in 'b' is advanced by 4.
- */
 static inline void
 isc_buffer_putuint32(isc_buffer_t *b, uint32_t val) {
-       unsigned char *cp;
+       ISC_BUFFER_PUT_RESERVE(b, val);
 
-       ISC_REQUIRE(ISC_BUFFER_VALID(b));
-
-       if (b->autore) {
-               isc_buffer_t *tmp = b;
-               ISC_REQUIRE(isc_buffer_reserve(tmp, 4) == ISC_R_SUCCESS);
-       }
-
-       ISC_REQUIRE(isc_buffer_availablelength(b) >= 4U);
+       uint8_t *cp = isc_buffer_used(b);
+       b->used += sizeof(val);
 
-       cp = isc_buffer_used(b);
-       b->used += 4;
-       cp[0] = (unsigned char)(val >> 24);
-       cp[1] = (unsigned char)(val >> 16);
-       cp[2] = (unsigned char)(val >> 8);
-       cp[3] = (unsigned char)val;
+       ISC_U32TO8_BE(cp, val);
 }
 
-/*!
- * \brief Store an unsigned 48-bit integer in host byte order from 'val'
- * into 'b' in network byte order.
- *
- * Requires:
- *\li  'b' is a valid buffer.
- *
- *\li  The length of the available region of 'b' is at least 6
- *     or the buffer has autoreallocation enabled.
- *
- * Ensures:
- *\li  The used pointer in 'b' is advanced by 6.
- */
-static inline void
-isc_buffer_putuint48(isc_buffer_t *b, uint64_t val) {
-       unsigned char *cp = NULL;
+static inline isc_result_t
+isc_buffer_peekuint48(const isc_buffer_t *b, uint64_t *valp) {
+       ISC_BUFFER_PEEK_CHECK(b, 6); /* 48-bits */
 
-       ISC_REQUIRE(ISC_BUFFER_VALID(b));
+       uint8_t *cp = isc_buffer_current(b);
 
-       if (b->autore) {
-               isc_buffer_t *tmp = b;
-               ISC_REQUIRE(isc_buffer_reserve(tmp, 6) == ISC_R_SUCCESS);
+       if (valp != NULL) {
+               *valp = ISC_U8TO48_BE(cp);
        }
+       return (ISC_R_SUCCESS);
+}
 
-       ISC_REQUIRE(isc_buffer_availablelength(b) >= 6U);
+static inline uint64_t
+isc_buffer_getuint48(isc_buffer_t *b) {
+       uint64_t     val = 0;
+       isc_result_t result = isc_buffer_peekuint48(b, &val);
+       ENSURE(result == ISC_R_SUCCESS);
+       b->current += 6; /* 48-bits */
+       return (val);
+}
 
-       cp = isc_buffer_used(b);
+static inline void
+isc_buffer_putuint48(isc_buffer_t *b, uint64_t val) {
+       ISC_BUFFER_PUT_RESERVE(b, val);
+
+       uint8_t *cp = isc_buffer_used(b);
        b->used += 6;
-       cp[0] = (unsigned char)(val >> 40);
-       cp[1] = (unsigned char)(val >> 32);
-       cp[2] = (unsigned char)(val >> 24);
-       cp[3] = (unsigned char)(val >> 16);
-       cp[4] = (unsigned char)(val >> 8);
-       cp[5] = (unsigned char)val;
+
+       ISC_U48TO8_BE(cp, val);
 }
 
-/*!
- * \brief Copy 'length' bytes of memory at 'base' into 'b'.
- *
- * Requires:
- *\li  'b' is a valid buffer.
- *
- *\li  'base' points to 'length' bytes of valid memory.
- *
- *\li  The length of the available region of 'b' is at least 'length'
- *     or the buffer has autoreallocation enabled.
- *
- * Ensures:
- *\li  The used pointer in 'b' is advanced by 'length'.
- */
 static inline void
 isc_buffer_putmem(isc_buffer_t *b, const unsigned char *base,
                  unsigned int length) {
        ISC_REQUIRE(ISC_BUFFER_VALID(b));
 
        if (b->autore) {
-               isc_buffer_t *tmp = b;
-               ISC_REQUIRE(isc_buffer_reserve(tmp, length) == ISC_R_SUCCESS);
+               isc_result_t result = isc_buffer_reserve(b, length);
+               ISC_REQUIRE(result == ISC_R_SUCCESS);
        }
 
        ISC_REQUIRE(isc_buffer_availablelength(b) >= (unsigned int)length);
@@ -1003,8 +1021,8 @@ isc_buffer_putstr(isc_buffer_t *b, const char *source) {
 
        length = (unsigned int)strlen(source);
        if (b->autore) {
-               isc_buffer_t *tmp = b;
-               ISC_REQUIRE(isc_buffer_reserve(tmp, length) == ISC_R_SUCCESS);
+               isc_result_t result = isc_buffer_reserve(b, length);
+               ISC_ENSURE(result == ISC_R_SUCCESS);
        }
 
        ISC_REQUIRE(isc_buffer_availablelength(b) >= length);

diff --git a/lib/ns/client.c b/lib/ns/client.c
index 39ef9acd9f6314c7a7e9b2474f70437c2de395ec..0bab7dbb51649b89acc97feca5735b7d33433795 100644 (file)
--- a/lib/ns/client.c
+++ b/lib/ns/client.c
@@ -1166,7 +1166,8 @@ compute_cookie(ns_client_t *client, uint32_t when, uint32_t nonce,
                cp = isc_buffer_used(buf);
                isc_buffer_putmem(buf, client->cookie, 8);
                isc_buffer_putuint8(buf, NS_COOKIE_VERSION_1);
-               isc_buffer_putuint24(buf, 0); /* Reserved */
+               isc_buffer_putuint8(buf, 0);  /* Reserved */
+               isc_buffer_putuint16(buf, 0); /* Reserved */
                isc_buffer_putuint32(buf, when);
 
                memmove(input, cp, 16);

diff --git a/tests/isc/buffer_test.c b/tests/isc/buffer_test.c
index 207ae37d36dc453a741f175f4a0894b1d2762a4c..fc205e993dd237fa49194dd82ae435c219706b59 100644 (file)
--- a/tests/isc/buffer_test.c
+++ b/tests/isc/buffer_test.c
@@ -136,14 +136,6 @@ ISC_RUN_TEST_IMPL(isc_buffer_dynamic) {
 
        assert_true(b->length - last_length >= 10000 * 2);
 
-       last_length += 10000 * 2;
-       for (i = 0; i < 10000; i++) {
-               isc_buffer_putuint24(b, 1);
-       }
-       assert_true(b->length - last_length >= 10000 * 3);
-
-       last_length += 10000 * 3;
-
        for (i = 0; i < 10000; i++) {
                isc_buffer_putuint32(b, 1);
        }