add assertions to isc_buffer macros

if ISC_BUFFER_USEINLINE is defined, then macros are used to implement
isc_buffer primitives (isc_buffer_init(), isc_buffer_region(), etc).
otherwise, functions are used. previously, only the functions had
DbC assertions, which made it possible for coding errors to go
undetected. this commit makes the macro versions enforce the same
requirements.

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 496a2a00ed2b8e84cc14a2636365fbde52a996df..d565356d96097d1d6035c8c45bd474c90cb973f1 100644 (file)
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -595,7 +595,8 @@ unit:gcc:alpine3.16:amd64:
 gcc:oraclelinux7:amd64:
   variables:
     CC: gcc
-    CFLAGS: "${CFLAGS_COMMON}"
+    # -Wno-address suppresses isc_buffer macro warnings
+    CFLAGS: "${CFLAGS_COMMON} -Wno-address"
     EXTRA_CONFIGURE: "--with-libidn2"
   <<: *oraclelinux_7_amd64_image
   <<: *build_job

diff --git a/lib/isc/buffer.c b/lib/isc/buffer.c
index 0e9b5cec160bd6bcd9264b3d83b7b0a9ac97aaef..b284840b0b5e1a60688b33bf44d0b32a98f61d2e 100644 (file)
--- a/lib/isc/buffer.c
+++ b/lib/isc/buffer.c
@@ -30,9 +30,6 @@ isc__buffer_init(isc_buffer_t *b, void *base, unsigned int length) {
         * Make 'b' refer to the 'length'-byte region starting at 'base'.
         * XXXDCL see the comment in buffer.h about base being const.
         */
-
-       REQUIRE(b != NULL);
-
        ISC__BUFFER_INIT(b, base, length);
 }
 
@@ -42,7 +39,6 @@ isc__buffer_initnull(isc_buffer_t *b) {
         * Initialize a new buffer which has no backing store.  This can
         * later be grown as needed and swapped in place.
         */
-
        ISC__BUFFER_INIT(b, NULL, 0);
 }
 
@@ -74,11 +70,6 @@ isc__buffer_invalidate(isc_buffer_t *b) {
        /*
         * Make 'b' an invalid buffer.
         */
-
-       REQUIRE(ISC_BUFFER_VALID(b));
-       REQUIRE(!ISC_LINK_LINKED(b, link));
-       REQUIRE(b->mctx == NULL);
-
        ISC__BUFFER_INVALIDATE(b);
 }
 
@@ -94,10 +85,6 @@ isc__buffer_region(isc_buffer_t *b, isc_region_t *r) {
        /*
         * Make 'r' refer to the region of 'b'.
         */
-
-       REQUIRE(ISC_BUFFER_VALID(b));
-       REQUIRE(r != NULL);
-
        ISC__BUFFER_REGION(b, r);
 }
 
@@ -106,10 +93,6 @@ isc__buffer_usedregion(const isc_buffer_t *b, isc_region_t *r) {
        /*
         * Make 'r' refer to the used region of 'b'.
         */
-
-       REQUIRE(ISC_BUFFER_VALID(b));
-       REQUIRE(r != NULL);
-
        ISC__BUFFER_USEDREGION(b, r);
 }
 
@@ -118,10 +101,6 @@ isc__buffer_availableregion(isc_buffer_t *b, isc_region_t *r) {
        /*
         * Make 'r' refer to the available region of 'b'.
         */
-
-       REQUIRE(ISC_BUFFER_VALID(b));
-       REQUIRE(r != NULL);
-
        ISC__BUFFER_AVAILABLEREGION(b, r);
 }
 
@@ -130,10 +109,6 @@ isc__buffer_add(isc_buffer_t *b, unsigned int n) {
        /*
         * Increase the 'used' region of 'b' by 'n' bytes.
         */
-
-       REQUIRE(ISC_BUFFER_VALID(b));
-       REQUIRE(b->used + n <= b->length);
-
        ISC__BUFFER_ADD(b, n);
 }
 
@@ -142,10 +117,6 @@ isc__buffer_subtract(isc_buffer_t *b, unsigned int n) {
        /*
         * Decrease the 'used' region of 'b' by 'n' bytes.
         */
-
-       REQUIRE(ISC_BUFFER_VALID(b));
-       REQUIRE(b->used >= n);
-
        ISC__BUFFER_SUBTRACT(b, n);
 }
 
@@ -154,9 +125,6 @@ isc__buffer_clear(isc_buffer_t *b) {
        /*
         * Make the used region empty.
         */
-
-       REQUIRE(ISC_BUFFER_VALID(b));
-
        ISC__BUFFER_CLEAR(b);
 }
 
@@ -165,10 +133,6 @@ isc__buffer_consumedregion(isc_buffer_t *b, isc_region_t *r) {
        /*
         * Make 'r' refer to the consumed region of 'b'.
         */
-
-       REQUIRE(ISC_BUFFER_VALID(b));
-       REQUIRE(r != NULL);
-
        ISC__BUFFER_CONSUMEDREGION(b, r);
 }
 
@@ -177,10 +141,6 @@ isc__buffer_remainingregion(isc_buffer_t *b, isc_region_t *r) {
        /*
         * Make 'r' refer to the remaining region of 'b'.
         */
-
-       REQUIRE(ISC_BUFFER_VALID(b));
-       REQUIRE(r != NULL);
-
        ISC__BUFFER_REMAININGREGION(b, r);
 }
 
@@ -189,10 +149,6 @@ isc__buffer_activeregion(isc_buffer_t *b, isc_region_t *r) {
        /*
         * Make 'r' refer to the active region of 'b'.
         */
-
-       REQUIRE(ISC_BUFFER_VALID(b));
-       REQUIRE(r != NULL);
-
        ISC__BUFFER_ACTIVEREGION(b, r);
 }
 
@@ -201,10 +157,6 @@ isc__buffer_setactive(isc_buffer_t *b, unsigned int n) {
        /*
         * Sets the end of the active region 'n' bytes after current.
         */
-
-       REQUIRE(ISC_BUFFER_VALID(b));
-       REQUIRE(b->current + n <= b->used);
-
        ISC__BUFFER_SETACTIVE(b, n);
 }
 
@@ -213,9 +165,6 @@ isc__buffer_first(isc_buffer_t *b) {
        /*
         * Make the consumed region empty.
         */
-
-       REQUIRE(ISC_BUFFER_VALID(b));
-
        ISC__BUFFER_FIRST(b);
 }
 
@@ -224,10 +173,6 @@ isc__buffer_forward(isc_buffer_t *b, unsigned int n) {
        /*
         * Increase the 'consumed' region of 'b' by 'n' bytes.
         */
-
-       REQUIRE(ISC_BUFFER_VALID(b));
-       REQUIRE(b->current + n <= b->used);
-
        ISC__BUFFER_FORWARD(b, n);
 }
 
@@ -236,10 +181,6 @@ isc__buffer_back(isc_buffer_t *b, unsigned int n) {
        /*
         * Decrease the 'consumed' region of 'b' by 'n' bytes.
         */
-
-       REQUIRE(ISC_BUFFER_VALID(b));
-       REQUIRE(n <= b->current);
-
        ISC__BUFFER_BACK(b, n);
 }
 
@@ -292,14 +233,6 @@ isc_buffer_getuint8(isc_buffer_t *b) {
 
 void
 isc__buffer_putuint8(isc_buffer_t *b, uint8_t val) {
-       isc_result_t result;
-       REQUIRE(ISC_BUFFER_VALID(b));
-       if (ISC_UNLIKELY(b->autore)) {
-               result = isc_buffer_reserve(&b, 1);
-               REQUIRE(result == ISC_R_SUCCESS);
-       }
-       REQUIRE(isc_buffer_availablelength(b) >= 1);
-
        ISC__BUFFER_PUTUINT8(b, val);
 }
 
@@ -326,27 +259,11 @@ isc_buffer_getuint16(isc_buffer_t *b) {
 
 void
 isc__buffer_putuint16(isc_buffer_t *b, uint16_t val) {
-       isc_result_t result;
-       REQUIRE(ISC_BUFFER_VALID(b));
-       if (ISC_UNLIKELY(b->autore)) {
-               result = isc_buffer_reserve(&b, 2);
-               REQUIRE(result == ISC_R_SUCCESS);
-       }
-       REQUIRE(isc_buffer_availablelength(b) >= 2);
-
        ISC__BUFFER_PUTUINT16(b, val);
 }
 
 void
 isc__buffer_putuint24(isc_buffer_t *b, uint32_t val) {
-       isc_result_t result;
-       REQUIRE(ISC_BUFFER_VALID(b));
-       if (ISC_UNLIKELY(b->autore)) {
-               result = isc_buffer_reserve(&b, 3);
-               REQUIRE(result == ISC_R_SUCCESS);
-       }
-       REQUIRE(isc_buffer_availablelength(b) >= 3);
-
        ISC__BUFFER_PUTUINT24(b, val);
 }
 
@@ -375,14 +292,6 @@ isc_buffer_getuint32(isc_buffer_t *b) {
 
 void
 isc__buffer_putuint32(isc_buffer_t *b, uint32_t val) {
-       isc_result_t result;
-       REQUIRE(ISC_BUFFER_VALID(b));
-       if (ISC_UNLIKELY(b->autore)) {
-               result = isc_buffer_reserve(&b, 4);
-               REQUIRE(result == ISC_R_SUCCESS);
-       }
-       REQUIRE(isc_buffer_availablelength(b) >= 4);
-
        ISC__BUFFER_PUTUINT32(b, val);
 }
 
@@ -433,39 +342,12 @@ isc__buffer_putuint48(isc_buffer_t *b, uint64_t val) {
 void
 isc__buffer_putmem(isc_buffer_t *b, const unsigned char *base,
                   unsigned int length) {
-       isc_result_t result;
-       REQUIRE(ISC_BUFFER_VALID(b));
-       if (ISC_UNLIKELY(b->autore)) {
-               result = isc_buffer_reserve(&b, length);
-               REQUIRE(result == ISC_R_SUCCESS);
-       }
-       REQUIRE(isc_buffer_availablelength(b) >= length);
-
        ISC__BUFFER_PUTMEM(b, base, length);
 }
 
 void
 isc__buffer_putstr(isc_buffer_t *b, const char *source) {
-       unsigned int l;
-       unsigned char *cp;
-       isc_result_t result;
-
-       REQUIRE(ISC_BUFFER_VALID(b));
-       REQUIRE(source != NULL);
-
-       /*
-        * Do not use ISC__BUFFER_PUTSTR(), so strlen is only done once.
-        */
-       l = strlen(source);
-       if (ISC_UNLIKELY(b->autore)) {
-               result = isc_buffer_reserve(&b, l);
-               REQUIRE(result == ISC_R_SUCCESS);
-       }
-       REQUIRE(isc_buffer_availablelength(b) >= l);
-
-       cp = isc_buffer_used(b);
-       memmove(cp, source, l);
-       b->used += l;
+       ISC__BUFFER_PUTSTR(b, source);
 }
 
 void

diff --git a/lib/isc/include/isc/buffer.h b/lib/isc/include/isc/buffer.h
index b5188975a7669a2fb43f5a31c8a65abff2fbb808..f3becae1cf4292440acecb2635c4691cb53a4ac8 100644 (file)
--- a/lib/isc/include/isc/buffer.h
+++ b/lib/isc/include/isc/buffer.h
@@ -108,6 +108,7 @@
 #include <isc/formatcheck.h>
 #include <isc/lang.h>
 #include <isc/likely.h>
+#include <isc/list.h>
 #include <isc/magic.h>
 #include <isc/types.h>
 
@@ -795,6 +796,7 @@ ISC_LANG_ENDDECLS
  */
 #define ISC__BUFFER_INIT(_b, _base, _length)    \
        do {                                    \
+               ISC_REQUIRE((_b) != NULL);      \
                (_b)->base = _base;             \
                (_b)->length = (_length);       \
                (_b)->used = 0;                 \
@@ -808,41 +810,54 @@ ISC_LANG_ENDDECLS
 
 #define ISC__BUFFER_INITNULL(_b) ISC__BUFFER_INIT(_b, NULL, 0)
 
-#define ISC__BUFFER_INVALIDATE(_b) \
-       do {                       \
-               (_b)->magic = 0;   \
-               (_b)->base = NULL; \
-               (_b)->length = 0;  \
-               (_b)->used = 0;    \
-               (_b)->current = 0; \
-               (_b)->active = 0;  \
+#define ISC__BUFFER_INVALIDATE(_b)                         \
+       do {                                               \
+               ISC_REQUIRE(ISC_BUFFER_VALID(_b));         \
+               ISC_REQUIRE(!ISC_LINK_LINKED((_b), link)); \
+               ISC_REQUIRE((_b)->mctx == NULL);           \
+               (_b)->magic = 0;                           \
+               (_b)->base = NULL;                         \
+               (_b)->length = 0;                          \
+               (_b)->used = 0;                            \
+               (_b)->current = 0;                         \
+               (_b)->active = 0;                          \
        } while (0)
 
-#define ISC__BUFFER_REGION(_b, _r)           \
-       do {                                 \
-               (_r)->base = (_b)->base;     \
-               (_r)->length = (_b)->length; \
+#define ISC__BUFFER_REGION(_b, _r)                 \
+       do {                                       \
+               ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \
+               ISC_REQUIRE((_r) != NULL);         \
+               (_r)->base = (_b)->base;           \
+               (_r)->length = (_b)->length;       \
        } while (0)
 
-#define ISC__BUFFER_USEDREGION(_b, _r)     \
-       do {                               \
-               (_r)->base = (_b)->base;   \
-               (_r)->length = (_b)->used; \
+#define ISC__BUFFER_USEDREGION(_b, _r)             \
+       do {                                       \
+               ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \
+               ISC_REQUIRE((_r) != NULL);         \
+               (_r)->base = (_b)->base;           \
+               (_r)->length = (_b)->used;         \
        } while (0)
 
 #define ISC__BUFFER_AVAILABLEREGION(_b, _r)                    \
        do {                                                   \
+               ISC_REQUIRE(ISC_BUFFER_VALID(_b));             \
+               ISC_REQUIRE((_r) != NULL);                     \
                (_r)->base = isc_buffer_used(_b);              \
                (_r)->length = isc_buffer_availablelength(_b); \
        } while (0)
 
-#define ISC__BUFFER_ADD(_b, _n)     \
-       do {                        \
-               (_b)->used += (_n); \
+#define ISC__BUFFER_ADD(_b, _n)                                 \
+       do {                                                    \
+               ISC_REQUIRE(ISC_BUFFER_VALID(_b));              \
+               ISC_REQUIRE((_b)->used + (_n) <= (_b)->length); \
+               (_b)->used += (_n);                             \
        } while (0)
 
 #define ISC__BUFFER_SUBTRACT(_b, _n)                \
        do {                                        \
+               ISC_REQUIRE(ISC_BUFFER_VALID(_b));  \
+               ISC_REQUIRE((_b)->used >= (_n));    \
                (_b)->used -= (_n);                 \
                if ((_b)->current > (_b)->used)     \
                        (_b)->current = (_b)->used; \
@@ -850,27 +865,34 @@ ISC_LANG_ENDDECLS
                        (_b)->active = (_b)->used;  \
        } while (0)
 
-#define ISC__BUFFER_CLEAR(_b)      \
-       do {                       \
-               (_b)->used = 0;    \
-               (_b)->current = 0; \
-               (_b)->active = 0;  \
+#define ISC__BUFFER_CLEAR(_b)                      \
+       do {                                       \
+               ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \
+               (_b)->used = 0;                    \
+               (_b)->current = 0;                 \
+               (_b)->active = 0;                  \
        } while (0)
 
-#define ISC__BUFFER_CONSUMEDREGION(_b, _r)    \
-       do {                                  \
-               (_r)->base = (_b)->base;      \
-               (_r)->length = (_b)->current; \
+#define ISC__BUFFER_CONSUMEDREGION(_b, _r)         \
+       do {                                       \
+               ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \
+               ISC_REQUIRE((_r) != NULL);         \
+               (_r)->base = (_b)->base;           \
+               (_r)->length = (_b)->current;      \
        } while (0)
 
 #define ISC__BUFFER_REMAININGREGION(_b, _r)                    \
        do {                                                   \
+               ISC_REQUIRE(ISC_BUFFER_VALID(_b));             \
+               ISC_REQUIRE((_r) != NULL);                     \
                (_r)->base = isc_buffer_current(_b);           \
                (_r)->length = isc_buffer_remaininglength(_b); \
        } while (0)
 
 #define ISC__BUFFER_ACTIVEREGION(_b, _r)                            \
        do {                                                        \
+               ISC_REQUIRE(ISC_BUFFER_VALID(_b));                  \
+               ISC_REQUIRE((_r) != NULL);                          \
                if ((_b)->current < (_b)->active) {                 \
                        (_r)->base = isc_buffer_current(_b);        \
                        (_r)->length = isc_buffer_activelength(_b); \
@@ -880,28 +902,36 @@ ISC_LANG_ENDDECLS
                }                                                   \
        } while (0)
 
-#define ISC__BUFFER_SETACTIVE(_b, _n)                \
-       do {                                         \
-               (_b)->active = (_b)->current + (_n); \
+#define ISC__BUFFER_SETACTIVE(_b, _n)                            \
+       do {                                                     \
+               ISC_REQUIRE(ISC_BUFFER_VALID(_b));               \
+               ISC_REQUIRE((_b)->current + (_n) <= (_b)->used); \
+               (_b)->active = (_b)->current + (_n);             \
        } while (0)
 
-#define ISC__BUFFER_FIRST(_b)      \
-       do {                       \
-               (_b)->current = 0; \
+#define ISC__BUFFER_FIRST(_b)                      \
+       do {                                       \
+               ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \
+               (_b)->current = 0;                 \
        } while (0)
 
-#define ISC__BUFFER_FORWARD(_b, _n)    \
-       do {                           \
-               (_b)->current += (_n); \
+#define ISC__BUFFER_FORWARD(_b, _n)                              \
+       do {                                                     \
+               ISC_REQUIRE(ISC_BUFFER_VALID(_b));               \
+               ISC_REQUIRE((_b)->current + (_n) <= (_b)->used); \
+               (_b)->current += (_n);                           \
        } while (0)
 
-#define ISC__BUFFER_BACK(_b, _n)       \
-       do {                           \
-               (_b)->current -= (_n); \
+#define ISC__BUFFER_BACK(_b, _n)                    \
+       do {                                        \
+               ISC_REQUIRE(ISC_BUFFER_VALID(_b));  \
+               ISC_REQUIRE((_n) <= (_b)->current); \
+               (_b)->current -= (_n);              \
        } while (0)
 
 #define ISC__BUFFER_PUTMEM(_b, _base, _length)                            \
        do {                                                              \
+               ISC_REQUIRE(ISC_BUFFER_VALID(_b));                        \
                if (ISC_UNLIKELY((_b)->autore)) {                         \
                        isc_buffer_t *_tmp = _b;                          \
                        ISC_REQUIRE(isc_buffer_reserve(&_tmp, _length) == \
@@ -919,6 +949,8 @@ ISC_LANG_ENDDECLS
        do {                                                              \
                unsigned int   _length;                                   \
                unsigned char *_cp;                                       \
+               ISC_REQUIRE(ISC_BUFFER_VALID(_b));                        \
+               ISC_REQUIRE((_source) != NULL);                           \
                _length = (unsigned int)strlen(_source);                  \
                if (ISC_UNLIKELY((_b)->autore)) {                         \
                        isc_buffer_t *_tmp = _b;                          \
@@ -936,6 +968,7 @@ ISC_LANG_ENDDECLS
                unsigned char *_cp;                                 \
                /* evaluate (_val) only once */                     \
                uint8_t _val2 = (_val);                             \
+               ISC_REQUIRE(ISC_BUFFER_VALID(_b));                  \
                if (ISC_UNLIKELY((_b)->autore)) {                   \
                        isc_buffer_t *_tmp = _b;                    \
                        ISC_REQUIRE(isc_buffer_reserve(&_tmp, 1) == \
@@ -952,6 +985,7 @@ ISC_LANG_ENDDECLS
                unsigned char *_cp;                                 \
                /* evaluate (_val) only once */                     \
                uint16_t _val2 = (_val);                            \
+               ISC_REQUIRE(ISC_BUFFER_VALID(_b));                  \
                if (ISC_UNLIKELY((_b)->autore)) {                   \
                        isc_buffer_t *_tmp = _b;                    \
                        ISC_REQUIRE(isc_buffer_reserve(&_tmp, 2) == \
@@ -969,6 +1003,7 @@ ISC_LANG_ENDDECLS
                unsigned char *_cp;                                 \
                /* evaluate (_val) only once */                     \
                uint32_t _val2 = (_val);                            \
+               ISC_REQUIRE(ISC_BUFFER_VALID(_b));                  \
                if (ISC_UNLIKELY((_b)->autore)) {                   \
                        isc_buffer_t *_tmp = _b;                    \
                        ISC_REQUIRE(isc_buffer_reserve(&_tmp, 3) == \
@@ -987,6 +1022,7 @@ ISC_LANG_ENDDECLS
                unsigned char *_cp;                                 \
                /* evaluate (_val) only once */                     \
                uint32_t _val2 = (_val);                            \
+               ISC_REQUIRE(ISC_BUFFER_VALID(_b));                  \
                if (ISC_UNLIKELY((_b)->autore)) {                   \
                        isc_buffer_t *_tmp = _b;                    \
                        ISC_REQUIRE(isc_buffer_reserve(&_tmp, 4) == \