From aea251f3bce7665651abbf1beb6de61a285a9453 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 15 Dec 2022 11:54:51 +0100 Subject: [PATCH 01/11] Change the isc_buffer_reserve() to take just buffer pointer The isc_buffer_reserve() would be passed a reference to the buffer pointer, which was unnecessary as the pointer would never be changed in the current implementation. Remove the extra dereference. --- bin/named/server.c | 10 +++++----- bin/tests/wire_test.c | 4 ++-- bin/tools/dnstap-read.c | 2 +- lib/dns/adb.c | 2 +- lib/dns/catz.c | 6 +++--- lib/dns/dnstap.c | 4 ++-- lib/dns/keytable.c | 2 +- lib/dns/nta.c | 2 +- lib/dns/resolver.c | 2 +- lib/isc/buffer.c | 28 +++++++++++++--------------- lib/isc/include/isc/buffer.h | 25 +++++++++---------------- lib/isccc/cc.c | 12 ++++++------ tests/isc/buffer_test.c | 10 +++++----- 13 files changed, 50 insertions(+), 59 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index d6f5ed771b..3027bafe75 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -7971,8 +7971,8 @@ data_to_cfg(dns_view_t *view, MDB_val *key, MDB_val *data, isc_buffer_t **text, INSIST(zone_config != NULL && zone_config_len > 0); /* zone zonename { config; }; */ - result = isc_buffer_reserve(text, 6 + zone_name_len + 2 + - zone_config_len + 2); + result = isc_buffer_reserve(*text, 6 + zone_name_len + 2 + + zone_config_len + 2); if (result != ISC_R_SUCCESS) { goto cleanup; } @@ -12474,7 +12474,7 @@ named_server_testgen(isc_lex_t *lex, isc_buffer_t **text) { count = strtoul(ptr, NULL, 10); } - CHECK(isc_buffer_reserve(text, count)); + CHECK(isc_buffer_reserve(*text, count)); for (i = 0; i < count; i++) { CHECK(putuint8(text, chars[i % (sizeof(chars) - 1)])); } @@ -15422,7 +15422,7 @@ static isc_result_t putmem(isc_buffer_t **b, const char *str, size_t len) { isc_result_t result; - result = isc_buffer_reserve(b, (unsigned int)len); + result = isc_buffer_reserve(*b, (unsigned int)len); if (result != ISC_R_SUCCESS) { return (ISC_R_NOSPACE); } @@ -15440,7 +15440,7 @@ static isc_result_t putuint8(isc_buffer_t **b, uint8_t val) { isc_result_t result; - result = isc_buffer_reserve(b, 1); + result = isc_buffer_reserve(*b, 1); if (result != ISC_R_SUCCESS) { return (ISC_R_NOSPACE); } diff --git a/bin/tests/wire_test.c b/bin/tests/wire_test.c index 8729fad89f..da25fe17ce 100644 --- a/bin/tests/wire_test.c +++ b/bin/tests/wire_test.c @@ -186,7 +186,7 @@ main(int argc, char *argv[]) { if (rawdata) { while (fread(&c, 1, 1, f) != 0) { - result = isc_buffer_reserve(&input, 1); + result = isc_buffer_reserve(input, 1); RUNTIME_CHECK(result == ISC_R_SUCCESS); isc_buffer_putuint8(input, (uint8_t)c); } @@ -223,7 +223,7 @@ main(int argc, char *argv[]) { c = fromhex(*rp++); c *= 16; c += fromhex(*rp++); - result = isc_buffer_reserve(&input, 1); + result = isc_buffer_reserve(input, 1); RUNTIME_CHECK(result == ISC_R_SUCCESS); isc_buffer_putuint8(input, (uint8_t)c); } diff --git a/bin/tools/dnstap-read.c b/bin/tools/dnstap-read.c index 933e76ad04..ee4aae12f5 100644 --- a/bin/tools/dnstap-read.c +++ b/bin/tools/dnstap-read.c @@ -157,7 +157,7 @@ print_packet(dns_dtdata_t *dt, const dns_master_style_t *style) { } for (;;) { - isc_buffer_reserve(&b, textlen); + isc_buffer_reserve(b, textlen); if (b == NULL) { fatal("out of memory"); } diff --git a/lib/dns/adb.c b/lib/dns/adb.c index d105107f95..d096c6185d 100644 --- a/lib/dns/adb.c +++ b/lib/dns/adb.c @@ -2713,7 +2713,7 @@ static isc_result_t putstr(isc_buffer_t **b, const char *str) { isc_result_t result; - result = isc_buffer_reserve(b, strlen(str)); + result = isc_buffer_reserve(*b, strlen(str)); if (result != ISC_R_SUCCESS) { return (result); } diff --git a/lib/dns/catz.c b/lib/dns/catz.c index b755e6cce5..03704555e8 100644 --- a/lib/dns/catz.c +++ b/lib/dns/catz.c @@ -1568,7 +1568,7 @@ catz_process_apl(dns_catz_zone_t *zone, isc_buffer_t **aclbp, if (apl_ent.negative) { isc_buffer_putuint8(aclb, '!'); } - isc_buffer_reserve(&aclb, INET6_ADDRSTRLEN); + isc_buffer_reserve(aclb, INET6_ADDRSTRLEN); result = isc_netaddr_totext(&addr, aclb); RUNTIME_CHECK(result == ISC_R_SUCCESS); if ((apl_ent.family == 1 && apl_ent.prefix < 32) || @@ -1872,7 +1872,7 @@ dns_catz_generate_masterfilename(dns_catz_zone_t *zone, dns_catz_entry_t *entry, rlen += strlen(entry->opts.zonedir) + 1; } - result = isc_buffer_reserve(buffer, (unsigned int)rlen); + result = isc_buffer_reserve(*buffer, (unsigned int)rlen); if (result != ISC_R_SUCCESS) { goto cleanup; } @@ -1981,7 +1981,7 @@ dns_catz_generate_zonecfg(dns_catz_zone_t *zone, dns_catz_entry_t *entry, } isc_netaddr_fromsockaddr(&netaddr, &entry->opts.masters.addrs[i]); - isc_buffer_reserve(&buffer, INET6_ADDRSTRLEN); + isc_buffer_reserve(buffer, INET6_ADDRSTRLEN); result = isc_netaddr_totext(&netaddr, buffer); RUNTIME_CHECK(result == ISC_R_SUCCESS); diff --git a/lib/dns/dnstap.c b/lib/dns/dnstap.c index 9a03b33593..784acd92f1 100644 --- a/lib/dns/dnstap.c +++ b/lib/dns/dnstap.c @@ -884,7 +884,7 @@ static isc_result_t putstr(isc_buffer_t **b, const char *str) { isc_result_t result; - result = isc_buffer_reserve(b, strlen(str)); + result = isc_buffer_reserve(*b, strlen(str)); if (result != ISC_R_SUCCESS) { return (ISC_R_NOSPACE); } @@ -1357,7 +1357,7 @@ dns_dt_datatotext(dns_dtdata_t *d, isc_buffer_t **dest) { CHECK(putstr(dest, d->typebuf)); } - CHECK(isc_buffer_reserve(dest, 1)); + CHECK(isc_buffer_reserve(*dest, 1)); isc_buffer_putuint8(*dest, 0); cleanup: diff --git a/lib/dns/keytable.c b/lib/dns/keytable.c index 56dceb2dc1..2e9922509b 100644 --- a/lib/dns/keytable.c +++ b/lib/dns/keytable.c @@ -618,7 +618,7 @@ static isc_result_t putstr(isc_buffer_t **b, const char *str) { isc_result_t result; - result = isc_buffer_reserve(b, strlen(str)); + result = isc_buffer_reserve(*b, strlen(str)); if (result != ISC_R_SUCCESS) { return (result); } diff --git a/lib/dns/nta.c b/lib/dns/nta.c index 44714cfa71..92fb14c535 100644 --- a/lib/dns/nta.c +++ b/lib/dns/nta.c @@ -478,7 +478,7 @@ static isc_result_t putstr(isc_buffer_t **b, const char *str) { isc_result_t result; - result = isc_buffer_reserve(b, strlen(str)); + result = isc_buffer_reserve(*b, strlen(str)); if (result != ISC_R_SUCCESS) { return (result); } diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 295c1d424a..99ad350afe 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -11415,7 +11415,7 @@ dns_resolver_dumpquota(dns_resolver_t *res, isc_buffer_t **buf) { " spilled %" PRIuFAST32 ")", nb, count, allowed, dropped); - result = isc_buffer_reserve(buf, strlen(text)); + result = isc_buffer_reserve(*buf, strlen(text)); if (result != ISC_R_SUCCESS) { goto cleanup; } diff --git a/lib/isc/buffer.c b/lib/isc/buffer.c index e7e84dff28..87411c890c 100644 --- a/lib/isc/buffer.c +++ b/lib/isc/buffer.c @@ -183,7 +183,7 @@ isc_buffer_putdecint(isc_buffer_t *b, int64_t v) { l = snprintf(buf, 21, "%" PRId64, v); RUNTIME_CHECK(l <= 21); if (b->autore) { - result = isc_buffer_reserve(&b, l); + result = isc_buffer_reserve(b, l); REQUIRE(result == ISC_R_SUCCESS); } REQUIRE(isc_buffer_availablelength(b) >= l); @@ -220,7 +220,7 @@ isc_buffer_copyregion(isc_buffer_t *b, const isc_region_t *r) { REQUIRE(r != NULL); if (b->autore) { - result = isc_buffer_reserve(&b, r->length); + result = isc_buffer_reserve(b, r->length); if (result != ISC_R_SUCCESS) { return (result); } @@ -256,23 +256,22 @@ isc_buffer_allocate(isc_mem_t *mctx, isc_buffer_t **dynbuffer, } isc_result_t -isc_buffer_reserve(isc_buffer_t **dynbuffer, unsigned int size) { +isc_buffer_reserve(isc_buffer_t *dynbuffer, unsigned int size) { size_t len; - REQUIRE(dynbuffer != NULL); - REQUIRE(ISC_BUFFER_VALID(*dynbuffer)); + REQUIRE(ISC_BUFFER_VALID(dynbuffer)); - len = (*dynbuffer)->length; - if ((len - (*dynbuffer)->used) >= size) { + len = dynbuffer->length; + if ((len - dynbuffer->used) >= size) { return (ISC_R_SUCCESS); } - if ((*dynbuffer)->mctx == NULL) { + if (dynbuffer->mctx == NULL) { return (ISC_R_NOSPACE); } /* Round to nearest buffer size increment */ - len = size + (*dynbuffer)->used; + len = size + dynbuffer->used; len = (len + ISC_BUFFER_INCR - 1 - ((len - 1) % ISC_BUFFER_INCR)); /* Cap at UINT_MAX */ @@ -280,14 +279,13 @@ isc_buffer_reserve(isc_buffer_t **dynbuffer, unsigned int size) { len = UINT_MAX; } - if ((len - (*dynbuffer)->used) < size) { + if ((len - dynbuffer->used) < size) { return (ISC_R_NOMEMORY); } - (*dynbuffer)->base = isc_mem_reget((*dynbuffer)->mctx, - (*dynbuffer)->base, - (*dynbuffer)->length, len); - (*dynbuffer)->length = (unsigned int)len; + dynbuffer->base = isc_mem_reget(dynbuffer->mctx, dynbuffer->base, + dynbuffer->length, len); + dynbuffer->length = (unsigned int)len; return (ISC_R_SUCCESS); } @@ -328,7 +326,7 @@ isc_buffer_printf(isc_buffer_t *b, const char *format, ...) { } if (b->autore) { - result = isc_buffer_reserve(&b, n + 1); + result = isc_buffer_reserve(b, n + 1); if (result != ISC_R_SUCCESS) { return (result); } diff --git a/lib/isc/include/isc/buffer.h b/lib/isc/include/isc/buffer.h index 1be0081e4c..8548d533c7 100644 --- a/lib/isc/include/isc/buffer.h +++ b/lib/isc/include/isc/buffer.h @@ -206,24 +206,17 @@ isc_buffer_allocate(isc_mem_t *mctx, isc_buffer_t **dynbuffer, */ isc_result_t -isc_buffer_reserve(isc_buffer_t **dynbuffer, unsigned int size); +isc_buffer_reserve(isc_buffer_t *dynbuffer, unsigned int size); /*!< * \brief Make "size" bytes of space available in the buffer. The buffer * pointer may move when you call this function. * * Requires: - *\li "dynbuffer" is not NULL. - * - *\li "*dynbuffer" is a valid dynamic buffer. + *\li "dynbuffer" is a valid dynamic buffer. * * Returns: *\li ISC_R_SUCCESS - success *\li ISC_R_NOMEMORY - no memory available - * - * Ensures: - *\li "*dynbuffer" will be valid on return and will contain all the - * original data. However, the buffer pointer may be moved during - * reallocation. */ void @@ -809,7 +802,7 @@ isc_buffer_putuint8(isc_buffer_t *b, uint8_t val) { if (b->autore) { isc_buffer_t *tmp = b; - ISC_REQUIRE(isc_buffer_reserve(&tmp, 1) == ISC_R_SUCCESS); + ISC_REQUIRE(isc_buffer_reserve(tmp, 1) == ISC_R_SUCCESS); } ISC_REQUIRE(isc_buffer_availablelength(b) >= 1U); @@ -840,7 +833,7 @@ isc_buffer_putuint16(isc_buffer_t *b, uint16_t val) { if (b->autore) { isc_buffer_t *tmp = b; - ISC_REQUIRE(isc_buffer_reserve(&tmp, 2) == ISC_R_SUCCESS); + ISC_REQUIRE(isc_buffer_reserve(tmp, 2) == ISC_R_SUCCESS); } ISC_REQUIRE(isc_buffer_availablelength(b) >= 2U); @@ -872,7 +865,7 @@ isc_buffer_putuint24(isc_buffer_t *b, uint32_t val) { if (b->autore) { isc_buffer_t *tmp = b; - ISC_REQUIRE(isc_buffer_reserve(&tmp, 3) == ISC_R_SUCCESS); + ISC_REQUIRE(isc_buffer_reserve(tmp, 3) == ISC_R_SUCCESS); } ISC_REQUIRE(isc_buffer_availablelength(b) >= 3U); @@ -905,7 +898,7 @@ isc_buffer_putuint32(isc_buffer_t *b, uint32_t val) { if (b->autore) { isc_buffer_t *tmp = b; - ISC_REQUIRE(isc_buffer_reserve(&tmp, 4) == ISC_R_SUCCESS); + ISC_REQUIRE(isc_buffer_reserve(tmp, 4) == ISC_R_SUCCESS); } ISC_REQUIRE(isc_buffer_availablelength(b) >= 4U); @@ -939,7 +932,7 @@ isc_buffer_putuint48(isc_buffer_t *b, uint64_t val) { if (b->autore) { isc_buffer_t *tmp = b; - ISC_REQUIRE(isc_buffer_reserve(&tmp, 6) == ISC_R_SUCCESS); + ISC_REQUIRE(isc_buffer_reserve(tmp, 6) == ISC_R_SUCCESS); } ISC_REQUIRE(isc_buffer_availablelength(b) >= 6U); @@ -975,7 +968,7 @@ isc_buffer_putmem(isc_buffer_t *b, const unsigned char *base, if (b->autore) { isc_buffer_t *tmp = b; - ISC_REQUIRE(isc_buffer_reserve(&tmp, length) == ISC_R_SUCCESS); + ISC_REQUIRE(isc_buffer_reserve(tmp, length) == ISC_R_SUCCESS); } ISC_REQUIRE(isc_buffer_availablelength(b) >= (unsigned int)length); @@ -1011,7 +1004,7 @@ 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_REQUIRE(isc_buffer_reserve(tmp, length) == ISC_R_SUCCESS); } ISC_REQUIRE(isc_buffer_availablelength(b) >= length); diff --git a/lib/isccc/cc.c b/lib/isccc/cc.c index 52914ed5ae..23eaf9d629 100644 --- a/lib/isccc/cc.c +++ b/lib/isccc/cc.c @@ -113,14 +113,14 @@ value_towire(isccc_sexpr_t *elt, isc_buffer_t **buffer) { if (isccc_sexpr_binaryp(elt)) { vr = isccc_sexpr_tobinary(elt); len = REGION_SIZE(*vr); - result = isc_buffer_reserve(buffer, 1 + 4); + result = isc_buffer_reserve(*buffer, 1 + 4); if (result != ISC_R_SUCCESS) { return (ISC_R_NOSPACE); } isc_buffer_putuint8(*buffer, ISCCC_CCMSGTYPE_BINARYDATA); isc_buffer_putuint32(*buffer, len); - result = isc_buffer_reserve(buffer, len); + result = isc_buffer_reserve(*buffer, len); if (result != ISC_R_SUCCESS) { return (ISC_R_NOSPACE); } @@ -129,7 +129,7 @@ value_towire(isccc_sexpr_t *elt, isc_buffer_t **buffer) { unsigned int used; isc_buffer_t b; - result = isc_buffer_reserve(buffer, 1 + 4); + result = isc_buffer_reserve(*buffer, 1 + 4); if (result != ISC_R_SUCCESS) { return (ISC_R_NOSPACE); } @@ -163,7 +163,7 @@ value_towire(isccc_sexpr_t *elt, isc_buffer_t **buffer) { unsigned int used; isc_buffer_t b; - result = isc_buffer_reserve(buffer, 1 + 4); + result = isc_buffer_reserve(*buffer, 1 + 4); if (result != ISC_R_SUCCESS) { return (ISC_R_NOSPACE); } @@ -217,7 +217,7 @@ table_towire(isccc_sexpr_t *alist, isc_buffer_t **buffer) { /* * Emit the key name. */ - result = isc_buffer_reserve(buffer, 1 + len); + result = isc_buffer_reserve(*buffer, 1 + len); if (result != ISC_R_SUCCESS) { return (ISC_R_NOSPACE); } @@ -313,7 +313,7 @@ isccc_cc_towire(isccc_sexpr_t *alist, isc_buffer_t **buffer, uint32_t algorithm, unsigned int hmac_base, signed_base; isc_result_t result; - result = isc_buffer_reserve(buffer, + result = isc_buffer_reserve(*buffer, 4 + ((algorithm == ISCCC_ALG_HMACMD5) ? sizeof(auth_hmd5) : sizeof(auth_hsha))); diff --git a/tests/isc/buffer_test.c b/tests/isc/buffer_test.c index 8335248fa0..207ae37d36 100644 --- a/tests/isc/buffer_test.c +++ b/tests/isc/buffer_test.c @@ -49,7 +49,7 @@ ISC_RUN_TEST_IMPL(isc_buffer_reserve) { * 1024 bytes should already be available, so this call does * nothing. */ - result = isc_buffer_reserve(&b, 1024); + result = isc_buffer_reserve(b, 1024); assert_int_equal(result, ISC_R_SUCCESS); assert_true(ISC_BUFFER_VALID(b)); assert_non_null(b); @@ -59,7 +59,7 @@ ISC_RUN_TEST_IMPL(isc_buffer_reserve) { * This call should grow it to 2048 bytes as only 1024 bytes are * available in the buffer. */ - result = isc_buffer_reserve(&b, 1025); + result = isc_buffer_reserve(b, 1025); assert_int_equal(result, ISC_R_SUCCESS); assert_true(ISC_BUFFER_VALID(b)); assert_non_null(b); @@ -69,7 +69,7 @@ ISC_RUN_TEST_IMPL(isc_buffer_reserve) { * 2048 bytes should already be available, so this call does * nothing. */ - result = isc_buffer_reserve(&b, 2000); + result = isc_buffer_reserve(b, 2000); assert_int_equal(result, ISC_R_SUCCESS); assert_true(ISC_BUFFER_VALID(b)); assert_non_null(b); @@ -79,7 +79,7 @@ ISC_RUN_TEST_IMPL(isc_buffer_reserve) { * This call should grow it to 4096 bytes as only 2048 bytes are * available in the buffer. */ - result = isc_buffer_reserve(&b, 3000); + result = isc_buffer_reserve(b, 3000); assert_int_equal(result, ISC_R_SUCCESS); assert_true(ISC_BUFFER_VALID(b)); assert_non_null(b); @@ -91,7 +91,7 @@ ISC_RUN_TEST_IMPL(isc_buffer_reserve) { /* * This call should fail and leave buffer untouched. */ - result = isc_buffer_reserve(&b, UINT_MAX); + result = isc_buffer_reserve(b, UINT_MAX); assert_int_equal(result, ISC_R_NOMEMORY); assert_true(ISC_BUFFER_VALID(b)); assert_non_null(b); From a1d45685e605d535770d0c5eecaab86a2be6a7dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 15 Dec 2022 12:58:31 +0100 Subject: [PATCH 02/11] Move and extend the uint8_t low-endian to uint{32,64}t to endian.h Move the U8TO{32,64}_LE and U{32,64}TO8_LE macros to endian.h and extend the macros for 16-bit and Big-Endian variants. Use the macros both in isc_siphash (LE) and isc_buffer (BE) units. --- lib/isc/include/isc/endian.h | 98 ++++++++++++++++++++++++++++++++++++ lib/isc/siphash.c | 42 +++++----------- 2 files changed, 110 insertions(+), 30 deletions(-) diff --git a/lib/isc/include/isc/endian.h b/lib/isc/include/isc/endian.h index be91b1dcfa..6d5e6c2b4f 100644 --- a/lib/isc/include/isc/endian.h +++ b/lib/isc/include/isc/endian.h @@ -164,3 +164,101 @@ #endif /* WORDS_BIGENDIAN */ #endif /* !htobe64 */ + +/* + * Macros to convert uint8_t arrays to integers. + */ + +/* Low-Endian */ + +#define ISC_U16TO8_LE(p, v) \ + (p)[0] = (uint8_t)((v)); \ + (p)[1] = (uint8_t)((v) >> 8); + +#define ISC_U8TO16_LE(p) (((uint16_t)((p)[0])) | ((uint16_t)((p)[1]) << 8)) + +#define ISC_U32TO8_LE(p, v) \ + (p)[0] = (uint8_t)((v)); \ + (p)[1] = (uint8_t)((v) >> 8); \ + (p)[2] = (uint8_t)((v) >> 16); \ + (p)[3] = (uint8_t)((v) >> 24); + +#define ISC_U8TO32_LE(p) \ + (((uint32_t)((p)[0])) | ((uint32_t)((p)[1]) << 8) | \ + ((uint32_t)((p)[2]) << 16) | ((uint32_t)((p)[3]) << 24)) + +#define ISC_U48TO8_LE(p, v) \ + (p)[0] = (uint8_t)((v)); \ + (p)[1] = (uint8_t)((v) >> 8); \ + (p)[2] = (uint8_t)((v) >> 16); \ + (p)[3] = (uint8_t)((v) >> 24); \ + (p)[4] = (uint8_t)((v) >> 32); \ + (p)[5] = (uint8_t)((v) >> 40); + +#define ISC_U8TO48_LE(p) \ + (((uint64_t)((p)[0])) | ((uint64_t)((p)[1]) << 8) | \ + ((uint64_t)((p)[2]) << 16) | ((uint64_t)((p)[3]) << 24) | \ + ((uint64_t)((p)[4]) << 32) | ((uint64_t)((p)[5]) << 40)) + +#define ISC_U64TO8_LE(p, v) \ + (p)[0] = (uint8_t)((v)); \ + (p)[1] = (uint8_t)((v) >> 8); \ + (p)[2] = (uint8_t)((v) >> 16); \ + (p)[3] = (uint8_t)((v) >> 24); \ + (p)[4] = (uint8_t)((v) >> 32); \ + (p)[5] = (uint8_t)((v) >> 40); \ + (p)[6] = (uint8_t)((v) >> 48); \ + (p)[7] = (uint8_t)((v) >> 56); + +#define ISC_U8TO64_LE(p) \ + (((uint64_t)((p)[0])) | ((uint64_t)((p)[1]) << 8) | \ + ((uint64_t)((p)[2]) << 16) | ((uint64_t)((p)[3]) << 24) | \ + ((uint64_t)((p)[4]) << 32) | ((uint64_t)((p)[5]) << 40) | \ + ((uint64_t)((p)[6]) << 48) | ((uint64_t)((p)[7]) << 56)) + +/* Big-Endian */ + +#define ISC_U16TO8_BE(p, v) \ + (p)[0] = (uint8_t)((v) >> 8); \ + (p)[1] = (uint8_t)((v)); + +#define ISC_U8TO16_BE(p) (((uint16_t)((p)[0]) << 8) | ((uint16_t)((p)[1]))) + +#define ISC_U32TO8_BE(p, v) \ + (p)[0] = (uint8_t)((v) >> 24); \ + (p)[1] = (uint8_t)((v) >> 16); \ + (p)[2] = (uint8_t)((v) >> 8); \ + (p)[3] = (uint8_t)((v)); + +#define ISC_U8TO32_BE(p) \ + (((uint32_t)((p)[0]) << 24) | ((uint32_t)((p)[1]) << 16) | \ + ((uint32_t)((p)[2]) << 8) | ((uint32_t)((p)[3]))) + +#define ISC_U48TO8_BE(p, v) \ + (p)[0] = (uint8_t)((v) >> 40); \ + (p)[1] = (uint8_t)((v) >> 32); \ + (p)[2] = (uint8_t)((v) >> 24); \ + (p)[3] = (uint8_t)((v) >> 16); \ + (p)[4] = (uint8_t)((v) >> 8); \ + (p)[5] = (uint8_t)((v)); + +#define ISC_U8TO48_BE(p) \ + (((uint64_t)((p)[0]) << 40) | ((uint64_t)((p)[1]) << 32) | \ + ((uint64_t)((p)[2]) << 24) | ((uint64_t)((p)[3]) << 16) | \ + ((uint64_t)((p)[4]) << 8) | ((uint64_t)((p)[5]))) + +#define ISC_U64TO8_BE(p, v) \ + (p)[0] = (uint8_t)((v) >> 56); \ + (p)[1] = (uint8_t)((v) >> 48); \ + (p)[2] = (uint8_t)((v) >> 40); \ + (p)[3] = (uint8_t)((v) >> 32); \ + (p)[4] = (uint8_t)((v) >> 24); \ + (p)[5] = (uint8_t)((v) >> 16); \ + (p)[6] = (uint8_t)((v) >> 8); \ + (p)[7] = (uint8_t)((v)); + +#define ISC_U8TO64_BE(p) \ + (((uint64_t)((p)[0]) << 56) | ((uint64_t)((p)[1]) << 48) | \ + ((uint64_t)((p)[2]) << 40) | ((uint64_t)((p)[3]) << 32) | \ + ((uint64_t)((p)[4]) << 24) | ((uint64_t)((p)[5]) << 16) | \ + ((uint64_t)((p)[6]) << 8) | ((uint64_t)((p)[7]))) diff --git a/lib/isc/siphash.c b/lib/isc/siphash.c index 8d13c61243..179e9f31a8 100644 --- a/lib/isc/siphash.c +++ b/lib/isc/siphash.c @@ -67,29 +67,9 @@ #define HALFSIPROUND FULL_ROUND32 -#define U32TO8_LE(p, v) \ - (p)[0] = (uint8_t)((v)); \ - (p)[1] = (uint8_t)((v) >> 8); \ - (p)[2] = (uint8_t)((v) >> 16); \ - (p)[3] = (uint8_t)((v) >> 24); - -#define U8TO32_LE(p) \ - (((uint32_t)((p)[0])) | ((uint32_t)((p)[1]) << 8) | \ - ((uint32_t)((p)[2]) << 16) | ((uint32_t)((p)[3]) << 24)) - #define U8TO32_ONE(case_sensitive, byte) \ (uint32_t)(case_sensitive ? byte : isc__ascii_tolower1(byte)) -#define U64TO8_LE(p, v) \ - U32TO8_LE((p), (uint32_t)((v))); \ - U32TO8_LE((p) + 4, (uint32_t)((v) >> 32)); - -#define U8TO64_LE(p) \ - (((uint64_t)((p)[0])) | ((uint64_t)((p)[1]) << 8) | \ - ((uint64_t)((p)[2]) << 16) | ((uint64_t)((p)[3]) << 24) | \ - ((uint64_t)((p)[4]) << 32) | ((uint64_t)((p)[5]) << 40) | \ - ((uint64_t)((p)[6]) << 48) | ((uint64_t)((p)[7]) << 56)) - #define U8TO64_ONE(case_sensitive, byte) \ (uint64_t)(case_sensitive ? byte : isc__ascii_tolower1(byte)) @@ -99,8 +79,8 @@ isc_siphash24(const uint8_t *k, const uint8_t *in, const size_t inlen, REQUIRE(k != NULL); REQUIRE(out != NULL); - uint64_t k0 = U8TO64_LE(k); - uint64_t k1 = U8TO64_LE(k + 8); + uint64_t k0 = ISC_U8TO64_LE(k); + uint64_t k1 = ISC_U8TO64_LE(k + 8); uint64_t v0 = UINT64_C(0x736f6d6570736575) ^ k0; uint64_t v1 = UINT64_C(0x646f72616e646f6d) ^ k1; @@ -113,8 +93,9 @@ isc_siphash24(const uint8_t *k, const uint8_t *in, const size_t inlen, const size_t left = inlen & 7; for (; in != end; in += 8) { - uint64_t m = case_sensitive ? U8TO64_LE(in) - : isc_ascii_tolower8(U8TO64_LE(in)); + uint64_t m = case_sensitive + ? ISC_U8TO64_LE(in) + : isc_ascii_tolower8(ISC_U8TO64_LE(in)); v3 ^= m; @@ -169,7 +150,7 @@ isc_siphash24(const uint8_t *k, const uint8_t *in, const size_t inlen, b = v0 ^ v1 ^ v2 ^ v3; - U64TO8_LE(out, b); + ISC_U64TO8_LE(out, b); } void @@ -178,8 +159,8 @@ isc_halfsiphash24(const uint8_t *k, const uint8_t *in, const size_t inlen, REQUIRE(k != NULL); REQUIRE(out != NULL); - uint32_t k0 = U8TO32_LE(k); - uint32_t k1 = U8TO32_LE(k + 4); + uint32_t k0 = ISC_U8TO32_LE(k); + uint32_t k1 = ISC_U8TO32_LE(k + 4); uint32_t v0 = UINT32_C(0x00000000) ^ k0; uint32_t v1 = UINT32_C(0x00000000) ^ k1; @@ -192,8 +173,9 @@ isc_halfsiphash24(const uint8_t *k, const uint8_t *in, const size_t inlen, const int left = inlen & 3; for (; in != end; in += 4) { - uint32_t m = case_sensitive ? U8TO32_LE(in) - : isc_ascii_tolower4(U8TO32_LE(in)); + uint32_t m = case_sensitive + ? ISC_U8TO32_LE(in) + : isc_ascii_tolower4(ISC_U8TO32_LE(in)); v3 ^= m; @@ -235,5 +217,5 @@ isc_halfsiphash24(const uint8_t *k, const uint8_t *in, const size_t inlen, } b = v1 ^ v3; - U32TO8_LE(out, b); + ISC_U32TO8_LE(out, b); } From 2a94123d5bcca41da1730af6b13cb922eb8e2ded Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 15 Dec 2022 12:38:31 +0100 Subject: [PATCH 03/11] 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. --- lib/isc/buffer.c | 89 --------- lib/isc/include/isc/buffer.h | 370 ++++++++++++++++++----------------- lib/ns/client.c | 3 +- tests/isc/buffer_test.c | 8 - 4 files changed, 196 insertions(+), 274 deletions(-) diff --git a/lib/isc/buffer.c b/lib/isc/buffer.c index 87411c890c..0cd4a24ee8 100644 --- 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 @@ -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 8548d533c7..71a03db11d 100644 --- a/lib/isc/include/isc/buffer.h +++ b/lib/isc/include/isc/buffer.h @@ -104,6 +104,7 @@ #include #include +#include #include #include #include @@ -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; - - ISC_REQUIRE(ISC_BUFFER_VALID(b)); - - if (b->autore) { - isc_buffer_t *tmp = b; - ISC_REQUIRE(isc_buffer_reserve(tmp, 1) == ISC_R_SUCCESS); +#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_availablelength(b) >= 1U); +static inline isc_result_t +isc_buffer_peekuint8(const isc_buffer_t *b, uint8_t *valp) { + ISC_BUFFER_PEEK_CHECK(b, sizeof(*valp)); - cp = isc_buffer_used(b); - b->used++; + uint8_t *cp = isc_buffer_current(b); + if (valp != NULL) { + *valp = (uint8_t)(cp[0]); + } + return (ISC_R_SUCCESS); +} + +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); +} + +#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 isc_result_t +isc_buffer_peekuint16(const isc_buffer_t *b, uint16_t *valp) { + ISC_BUFFER_PEEK_CHECK(b, sizeof(*valp)); + + uint8_t *cp = isc_buffer_current(b); + + if (valp != NULL) { + *valp = ISC_U8TO16_BE(cp); + } + return (ISC_R_SUCCESS); +} + +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); +} + static inline void isc_buffer_putuint16(isc_buffer_t *b, uint16_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, 2) == 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; + uint8_t *cp = isc_buffer_used(b); + b->used += sizeof(val); + ISC_U16TO8_BE(cp, 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; +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_VALID(b)); + uint8_t *cp = isc_buffer_current(b); - if (b->autore) { - isc_buffer_t *tmp = b; - ISC_REQUIRE(isc_buffer_reserve(tmp, 3) == ISC_R_SUCCESS); + if (valp != NULL) { + *valp = ISC_U8TO32_BE(cp); } + return (ISC_R_SUCCESS); +} - ISC_REQUIRE(isc_buffer_availablelength(b) >= 3U); - - 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; +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)); + uint8_t *cp = isc_buffer_used(b); + b->used += sizeof(val); - if (b->autore) { - isc_buffer_t *tmp = b; - ISC_REQUIRE(isc_buffer_reserve(tmp, 4) == ISC_R_SUCCESS); + ISC_U32TO8_BE(cp, val); +} + +static inline isc_result_t +isc_buffer_peekuint48(const isc_buffer_t *b, uint64_t *valp) { + ISC_BUFFER_PEEK_CHECK(b, 6); /* 48-bits */ + + uint8_t *cp = isc_buffer_current(b); + + if (valp != NULL) { + *valp = ISC_U8TO48_BE(cp); } - - ISC_REQUIRE(isc_buffer_availablelength(b) >= 4U); + return (ISC_R_SUCCESS); +} - 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; +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); } -/*! - * \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; + 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, 6) == ISC_R_SUCCESS); - } - - ISC_REQUIRE(isc_buffer_availablelength(b) >= 6U); - - cp = isc_buffer_used(b); + 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 39ef9acd9f..0bab7dbb51 100644 --- 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 207ae37d36..fc205e993d 100644 --- 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); } From 135ec7a0f01d947ebb87ac380d7b37b4134f1edb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 15 Dec 2022 22:15:37 +0100 Subject: [PATCH 04/11] Remove single use isc_buffer_putdecint() function The isc_buffer_putdecint() could be easily replaced with isc_buffer_printf() with just a small overhead of calling vsnprintf() twice instead once. This is not on a hot-path (dns_catz unit), so we can ignore the overhead and instead have less single-use code in favor of using reusable more generic function. --- lib/dns/catz.c | 2 +- lib/isc/buffer.c | 23 ----------------------- lib/isc/include/isc/buffer.h | 15 --------------- 3 files changed, 1 insertion(+), 39 deletions(-) diff --git a/lib/dns/catz.c b/lib/dns/catz.c index 03704555e8..600a9bb345 100644 --- a/lib/dns/catz.c +++ b/lib/dns/catz.c @@ -1575,7 +1575,7 @@ catz_process_apl(dns_catz_zone_t *zone, isc_buffer_t **aclbp, (apl_ent.family == 2 && apl_ent.prefix < 128)) { isc_buffer_putuint8(aclb, '/'); - isc_buffer_putdecint(aclb, apl_ent.prefix); + isc_buffer_printf(aclb, "%" PRId8, apl_ent.prefix); } isc_buffer_putstr(aclb, "; "); } diff --git a/lib/isc/buffer.c b/lib/isc/buffer.c index 0cd4a24ee8..c1630bfdb6 100644 --- a/lib/isc/buffer.c +++ b/lib/isc/buffer.c @@ -81,29 +81,6 @@ isc_buffer_compact(isc_buffer_t *b) { b->used = length; } -void -isc_buffer_putdecint(isc_buffer_t *b, int64_t v) { - unsigned int l = 0; - unsigned char *cp; - char buf[21]; - isc_result_t result; - - REQUIRE(ISC_BUFFER_VALID(b)); - - /* xxxwpk do it more low-level way ? */ - l = snprintf(buf, 21, "%" PRId64, v); - RUNTIME_CHECK(l <= 21); - if (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, buf, l); - b->used += l; -} - isc_result_t isc_buffer_dup(isc_mem_t *mctx, isc_buffer_t **dstp, const isc_buffer_t *src) { isc_buffer_t *dst = NULL; diff --git a/lib/isc/include/isc/buffer.h b/lib/isc/include/isc/buffer.h index 71a03db11d..6247a89456 100644 --- a/lib/isc/include/isc/buffer.h +++ b/lib/isc/include/isc/buffer.h @@ -420,21 +420,6 @@ isc_buffer_putmem(isc_buffer_t *b, const unsigned char *base, *\li The used pointer in 'b' is advanced by 'length'. */ -void -isc_buffer_putdecint(isc_buffer_t *b, int64_t v); -/*!< - * \brief Put decimal representation of 'v' in b - * - * Requires: - *\li 'b' is a valid buffer. - * - *\li The length of the available region of 'b' is at least strlen(dec('v')) - * or the buffer has autoreallocation enabled. - * - * Ensures: - *\li The used pointer in 'b' is advanced by strlen(dec('v')). - */ - isc_result_t isc_buffer_copyregion(isc_buffer_t *b, const isc_region_t *r); /*!< From 6bd2b34180d74146423a6c637f58d3bb8164a29c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 15 Dec 2022 22:27:12 +0100 Subject: [PATCH 05/11] Enable auto-reallocation for all isc_buffer_allocate() buffers When isc_buffer_t buffer is created with isc_buffer_allocate() assume that we want it to always auto-reallocate instead of having an extra call to enable auto-reallocation. --- lib/dns/catz.c | 2 -- lib/isc/buffer.c | 15 +++----------- lib/isc/httpd.c | 1 - lib/isc/include/isc/buffer.h | 18 +++-------------- lib/isc/netmgr/http.c | 39 +++++++++++++++++++++--------------- tests/isc/buffer_test.c | 12 +---------- 6 files changed, 30 insertions(+), 57 deletions(-) diff --git a/lib/dns/catz.c b/lib/dns/catz.c index 600a9bb345..56d4ed65bb 100644 --- a/lib/dns/catz.c +++ b/lib/dns/catz.c @@ -1548,7 +1548,6 @@ catz_process_apl(dns_catz_zone_t *zone, isc_buffer_t **aclbp, return (result); } isc_buffer_allocate(zone->catzs->mctx, &aclb, 16); - isc_buffer_setautorealloc(aclb, true); for (result = dns_rdata_apl_first(&rdata_apl); result == ISC_R_SUCCESS; result = dns_rdata_apl_next(&rdata_apl)) { @@ -1940,7 +1939,6 @@ dns_catz_generate_zonecfg(dns_catz_zone_t *zone, dns_catz_entry_t *entry, */ isc_buffer_allocate(zone->catzs->mctx, &buffer, ISC_BUFFER_INCR); - isc_buffer_setautorealloc(buffer, true); isc_buffer_putstr(buffer, "zone \""); dns_name_totext(&entry->name, true, buffer); isc_buffer_putstr(buffer, "\" { type secondary; primaries"); diff --git a/lib/isc/buffer.c b/lib/isc/buffer.c index c1630bfdb6..8d9873adad 100644 --- a/lib/isc/buffer.c +++ b/lib/isc/buffer.c @@ -36,7 +36,7 @@ isc_buffer_reinit(isc_buffer_t *b, void *base, unsigned int length) { */ REQUIRE(b->length <= length); REQUIRE(base != NULL); - REQUIRE(!b->autore); + REQUIRE(b->mctx == NULL); if (b->length > 0U) { (void)memmove(base, b->base, b->length); @@ -46,13 +46,6 @@ isc_buffer_reinit(isc_buffer_t *b, void *base, unsigned int length) { b->length = length; } -void -isc_buffer_setautorealloc(isc_buffer_t *b, bool enable) { - REQUIRE(ISC_BUFFER_VALID(b)); - REQUIRE(b->mctx != NULL); - b->autore = enable; -} - void isc_buffer_compact(isc_buffer_t *b) { unsigned int length; @@ -107,7 +100,7 @@ isc_buffer_copyregion(isc_buffer_t *b, const isc_region_t *r) { REQUIRE(ISC_BUFFER_VALID(b)); REQUIRE(r != NULL); - if (b->autore) { + if (b->mctx) { result = isc_buffer_reserve(b, r->length); if (result != ISC_R_SUCCESS) { return (result); @@ -136,8 +129,6 @@ isc_buffer_allocate(isc_mem_t *mctx, isc_buffer_t **dynbuffer, isc_buffer_init(dbuf, bdata, length); - ENSURE(ISC_BUFFER_VALID(dbuf)); - dbuf->mctx = mctx; *dynbuffer = dbuf; @@ -213,7 +204,7 @@ isc_buffer_printf(isc_buffer_t *b, const char *format, ...) { return (ISC_R_FAILURE); } - if (b->autore) { + if (b->mctx) { result = isc_buffer_reserve(b, n + 1); if (result != ISC_R_SUCCESS) { return (result); diff --git a/lib/isc/httpd.c b/lib/isc/httpd.c index 039de4da1a..6cdd5b01ca 100644 --- a/lib/isc/httpd.c +++ b/lib/isc/httpd.c @@ -562,7 +562,6 @@ isc__httpd_sendreq_new(isc_httpd_t *httpd) { */ isc_buffer_allocate(req->mctx, &req->sendbuffer, HTTP_SENDLEN); isc_buffer_clear(req->sendbuffer); - isc_buffer_setautorealloc(req->sendbuffer, true); isc_buffer_initnull(&req->bodybuffer); diff --git a/lib/isc/include/isc/buffer.h b/lib/isc/include/isc/buffer.h index 6247a89456..6ce0ba45ce 100644 --- a/lib/isc/include/isc/buffer.h +++ b/lib/isc/include/isc/buffer.h @@ -182,8 +182,6 @@ struct isc_buffer { ISC_LINK(isc_buffer_t) link; /*! private internal elements */ isc_mem_t *mctx; - /* automatically realloc buffer at put* */ - bool autore; }; /*** @@ -253,16 +251,6 @@ isc_buffer_reinit(isc_buffer_t *b, void *base, unsigned int length); * */ -void -isc_buffer_setautorealloc(isc_buffer_t *b, bool enable); -/*!< - * \brief Enable or disable autoreallocation on 'b'. - * - * Requires: - *\li 'b' is a valid dynamic buffer (b->mctx != NULL). - * - */ - void isc_buffer_compact(isc_buffer_t *b); /*!< @@ -854,7 +842,7 @@ isc_buffer_getuint8(isc_buffer_t *b) { { \ REQUIRE(ISC_BUFFER_VALID(b)); \ \ - if (b->autore) { \ + if (b->mctx) { \ isc_result_t result = isc_buffer_reserve(b, \ sizeof(val)); \ ENSURE(result == ISC_R_SUCCESS); \ @@ -969,7 +957,7 @@ isc_buffer_putmem(isc_buffer_t *b, const unsigned char *base, unsigned int length) { ISC_REQUIRE(ISC_BUFFER_VALID(b)); - if (b->autore) { + if (b->mctx) { isc_result_t result = isc_buffer_reserve(b, length); ISC_REQUIRE(result == ISC_R_SUCCESS); } @@ -1005,7 +993,7 @@ isc_buffer_putstr(isc_buffer_t *b, const char *source) { ISC_REQUIRE(source != NULL); length = (unsigned int)strlen(source); - if (b->autore) { + if (b->mctx) { isc_result_t result = isc_buffer_reserve(b, length); ISC_ENSURE(result == ISC_R_SUCCESS); } diff --git a/lib/isc/netmgr/http.c b/lib/isc/netmgr/http.c index cf04ac3d46..7cb69b5f97 100644 --- a/lib/isc/netmgr/http.c +++ b/lib/isc/netmgr/http.c @@ -443,7 +443,6 @@ new_http_cstream(isc_nmsocket_t *sock, http_cstream_t **streamp) { isc_buffer_allocate(mctx, &stream->rbuf, INITIAL_DNS_MESSAGE_BUFFER_SIZE); - isc_buffer_setautorealloc(stream->rbuf, true); ISC_LIST_PREPEND(sock->h2.session->cstreams, stream, link); *streamp = stream; @@ -1005,7 +1004,6 @@ http_readcb(isc_nmhandle_t *handle, isc_result_t result, isc_region_t *region, if (session->buf == NULL) { isc_buffer_allocate(session->mctx, &session->buf, unread_size); - isc_buffer_setautorealloc(session->buf, true); } isc_buffer_putmem(session->buf, region->base + readlen, unread_size); @@ -1121,8 +1119,6 @@ http_send_outgoing(isc_nm_http_session_t *session, isc_nmhandle_t *httphandle, isc_buffer_allocate(session->mctx, &session->pending_write_data, INITIAL_DNS_MESSAGE_BUFFER_SIZE); - isc_buffer_setautorealloc(session->pending_write_data, - true); } isc_buffer_putmem(session->pending_write_data, data, pending); total = new_total; @@ -1267,13 +1263,15 @@ http_do_bio(isc_nm_http_session_t *session, isc_nmhandle_t *send_httphandle, if (nghttp2_session_want_read(session->ngsession) != 0) { if (!session->reading) { - /* We have not yet started reading from this handle */ + /* We have not yet started + * reading from this handle */ isc_nm_read(session->handle, http_readcb, session); session->reading = true; } else if (session->buf != NULL) { size_t remaining = isc_buffer_remaininglength(session->buf); - /* Leftover data in the buffer, use it */ + /* Leftover data in the + * buffer, use it */ size_t readlen = nghttp2_session_mem_recv( session->ngsession, isc_buffer_current(session->buf), remaining); @@ -1288,7 +1286,9 @@ http_do_bio(isc_nm_http_session_t *session, isc_nmhandle_t *send_httphandle, send_cbarg); return; } else { - /* Resume reading, it's idempotent, wait for more */ + /* Resume reading, it's + * idempotent, wait for more + */ isc_nm_read(session->handle, http_readcb, session); } } else { @@ -1406,9 +1406,10 @@ transport_connect_cb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { NGHTTP2_PROTO_VERSION_ID_LEN) != 0) { /* - * HTTP/2 negotiation error. Any sensible DoH - * client will fail if HTTP/2 cannot be - * negotiated via ALPN. + * HTTP/2 negotiation error. + * Any sensible DoH client + * will fail if HTTP/2 cannot + * be negotiated via ALPN. */ result = ISC_R_HTTP2ALPNERROR; goto error; @@ -2346,9 +2347,12 @@ server_on_frame_recv_callback(nghttp2_session *ngsession, ngsession, frame->hd.stream_id); /* - * For DATA and HEADERS frame, this callback may be - * called after on_stream_close_callback. Check that - * the stream is still alive. + * For DATA and HEADERS frame, + * this callback may be called + * after + * on_stream_close_callback. + * Check that the stream is + * still alive. */ if (socket == NULL) { return (0); @@ -3168,9 +3172,12 @@ isc__nm_base64_to_base64url(isc_mem_t *mem, const char *base64, break; default: /* - * All other characters from the alphabet are the same - * for both base64 and base64url, so we can reuse the - * validation table for the rest of the characters. + * All other characters from + * the alphabet are the same + * for both base64 and + * base64url, so we can reuse + * the validation table for + * the rest of the characters. */ if (base64[i] != '-' && base64[i] != '_' && base64url_validation_table[(size_t)base64[i]]) diff --git a/tests/isc/buffer_test.c b/tests/isc/buffer_test.c index fc205e993d..94ed6f8017 100644 --- a/tests/isc/buffer_test.c +++ b/tests/isc/buffer_test.c @@ -113,8 +113,6 @@ ISC_RUN_TEST_IMPL(isc_buffer_dynamic) { assert_non_null(b); assert_int_equal(b->length, last_length); - isc_buffer_setautorealloc(b, true); - isc_buffer_putuint8(b, 1); for (i = 0; i < 1000; i++) { @@ -166,16 +164,9 @@ ISC_RUN_TEST_IMPL(isc_buffer_copyregion) { assert_int_equal(result, ISC_R_SUCCESS); /* - * Appending more data to the buffer should fail. + * Appending should succeed. */ result = isc_buffer_copyregion(b, &r); - assert_int_equal(result, ISC_R_NOSPACE); - - /* - * Enable auto reallocation and retry. Appending should now succeed. - */ - isc_buffer_setautorealloc(b, true); - result = isc_buffer_copyregion(b, &r); assert_int_equal(result, ISC_R_SUCCESS); isc_buffer_free(&b); @@ -196,7 +187,6 @@ ISC_RUN_TEST_IMPL(isc_buffer_printf) { */ b = NULL; isc_buffer_allocate(mctx, &b, 0); - isc_buffer_setautorealloc(b, true); /* * Sanity check. From 2ddea1e41c632550afb28ddb60aa1ee19a05bda5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 15 Dec 2022 22:51:52 +0100 Subject: [PATCH 06/11] Add a static pre-allocated buffer to isc_buffer_t When the buffer is allocated via isc_buffer_allocate() and the size is smaller or equal ISC_BUFFER_STATIC_SIZE (currently 512 bytes), the buffer will be allocated as a flexible array member in the buffer structure itself instead of allocating it on the heap. This should help when the buffer is used on the hot-path with small allocations. --- lib/isc/buffer.c | 66 ++++++++++++++++++++---------------- lib/isc/include/isc/buffer.h | 31 ++++++++++------- tests/isc/buffer_test.c | 33 ++++++++---------- 3 files changed, 69 insertions(+), 61 deletions(-) diff --git a/lib/isc/buffer.c b/lib/isc/buffer.c index 8d9873adad..9e2c3bf339 100644 --- a/lib/isc/buffer.c +++ b/lib/isc/buffer.c @@ -120,72 +120,80 @@ isc_buffer_copyregion(isc_buffer_t *b, const isc_region_t *r) { } void -isc_buffer_allocate(isc_mem_t *mctx, isc_buffer_t **dynbuffer, +isc_buffer_allocate(isc_mem_t *mctx, isc_buffer_t **dbufp, unsigned int length) { - REQUIRE(dynbuffer != NULL && *dynbuffer == NULL); + REQUIRE(dbufp != NULL && *dbufp == NULL); - isc_buffer_t *dbuf = isc_mem_get(mctx, sizeof(isc_buffer_t)); - unsigned char *bdata = isc_mem_get(mctx, length); + isc_buffer_t *dbuf = isc_mem_get(mctx, sizeof(*dbuf) + length); + uint8_t *bdata = (uint8_t *)dbuf + sizeof(*dbuf); isc_buffer_init(dbuf, bdata, length); - + dbuf->extra = length; dbuf->mctx = mctx; - *dynbuffer = dbuf; + *dbufp = dbuf; } isc_result_t -isc_buffer_reserve(isc_buffer_t *dynbuffer, unsigned int size) { +isc_buffer_reserve(isc_buffer_t *dbuf, unsigned int size) { + REQUIRE(ISC_BUFFER_VALID(dbuf)); + size_t len; + uint8_t *bdata = (uint8_t *)dbuf + sizeof(*dbuf); - REQUIRE(ISC_BUFFER_VALID(dynbuffer)); - - len = dynbuffer->length; - if ((len - dynbuffer->used) >= size) { + len = dbuf->length; + if ((len - dbuf->used) >= size) { return (ISC_R_SUCCESS); } - if (dynbuffer->mctx == NULL) { + if (dbuf->mctx == NULL) { return (ISC_R_NOSPACE); } /* Round to nearest buffer size increment */ - len = size + dynbuffer->used; - len = (len + ISC_BUFFER_INCR - 1 - ((len - 1) % ISC_BUFFER_INCR)); + len = size + dbuf->used; + len = ISC_ALIGN(len, ISC_BUFFER_INCR); /* Cap at UINT_MAX */ if (len > UINT_MAX) { len = UINT_MAX; } - if ((len - dynbuffer->used) < size) { + if ((len - dbuf->used) < size) { return (ISC_R_NOMEMORY); } - dynbuffer->base = isc_mem_reget(dynbuffer->mctx, dynbuffer->base, - dynbuffer->length, len); - dynbuffer->length = (unsigned int)len; + if (dbuf->base == bdata) { + dbuf->base = isc_mem_get(dbuf->mctx, len); + memmove(dbuf->base, bdata, dbuf->used); + } else { + dbuf->base = isc_mem_reget(dbuf->mctx, dbuf->base, dbuf->length, + len); + } + dbuf->length = (unsigned int)len; return (ISC_R_SUCCESS); } void -isc_buffer_free(isc_buffer_t **dynbuffer) { - isc_buffer_t *dbuf; - isc_mem_t *mctx; +isc_buffer_free(isc_buffer_t **dbufp) { + REQUIRE(dbufp != NULL && ISC_BUFFER_VALID(*dbufp)); + REQUIRE((*dbufp)->mctx != NULL); - REQUIRE(dynbuffer != NULL); - REQUIRE(ISC_BUFFER_VALID(*dynbuffer)); - REQUIRE((*dynbuffer)->mctx != NULL); + isc_buffer_t *dbuf = *dbufp; + isc_mem_t *mctx = dbuf->mctx; + uint8_t *bdata = (uint8_t *)dbuf + sizeof(*dbuf); + unsigned int extra = dbuf->extra; - dbuf = *dynbuffer; - *dynbuffer = NULL; /* destroy external reference */ - mctx = dbuf->mctx; + *dbufp = NULL; /* destroy external reference */ dbuf->mctx = NULL; - isc_mem_put(mctx, dbuf->base, dbuf->length); + if (dbuf->base != bdata) { + isc_mem_put(mctx, dbuf->base, dbuf->length); + } + isc_buffer_invalidate(dbuf); - isc_mem_put(mctx, dbuf, sizeof(isc_buffer_t)); + isc_mem_put(mctx, dbuf, sizeof(*dbuf) + extra); } isc_result_t diff --git a/lib/isc/include/isc/buffer.h b/lib/isc/include/isc/buffer.h index 6ce0ba45ce..0929e6a750 100644 --- a/lib/isc/include/isc/buffer.h +++ b/lib/isc/include/isc/buffer.h @@ -128,7 +128,7 @@ ISC_LANG_BEGINDECLS * space in a buffer, we round the allocated buffer length up to the * nearest * multiple of this value. */ -#define ISC_BUFFER_INCR 2048 +#define ISC_BUFFER_INCR 512 /* * The following macros MUST be used only on valid buffers. It is the @@ -177,6 +177,8 @@ struct isc_buffer { unsigned int used; unsigned int current; unsigned int active; + /*! The extra bytes allocated for dynamic buffer */ + unsigned int extra; /*@}*/ /*! linkable */ ISC_LINK(isc_buffer_t) link; @@ -184,6 +186,8 @@ struct isc_buffer { isc_mem_t *mctx; }; +#define ISC_BUFFER_STATIC_SIZE 512 + /*** *** Functions ***/ @@ -509,10 +513,12 @@ static inline void isc_buffer_init(isc_buffer_t *b, void *base, unsigned int length) { ISC_REQUIRE(b != NULL); - *b = (isc_buffer_t){ .base = base, - .length = length, - .magic = ISC_BUFFER_MAGIC }; - ISC_LINK_INIT(b, link); + *b = (isc_buffer_t){ + .base = base, + .length = length, + .link = ISC_LINK_INITIALIZER, + .magic = ISC_BUFFER_MAGIC, + }; } /*! @@ -521,8 +527,10 @@ isc_buffer_init(isc_buffer_t *b, void *base, unsigned int length) { */ static inline void isc_buffer_initnull(isc_buffer_t *b) { - *b = (isc_buffer_t){ .magic = ISC_BUFFER_MAGIC }; - ISC_LINK_INIT(b, link); + *b = (isc_buffer_t){ + .link = ISC_LINK_INITIALIZER, + .magic = ISC_BUFFER_MAGIC, + }; } /*! @@ -560,12 +568,9 @@ isc_buffer_invalidate(isc_buffer_t *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; + *b = (isc_buffer_t){ + .magic = 0, + }; } /*! diff --git a/tests/isc/buffer_test.c b/tests/isc/buffer_test.c index 94ed6f8017..09e1038e46 100644 --- a/tests/isc/buffer_test.c +++ b/tests/isc/buffer_test.c @@ -42,48 +42,44 @@ ISC_RUN_TEST_IMPL(isc_buffer_reserve) { UNUSED(state); b = NULL; - isc_buffer_allocate(mctx, &b, 1024); - assert_int_equal(b->length, 1024); + isc_buffer_allocate(mctx, &b, ISC_BUFFER_INCR); + assert_int_equal(b->length, ISC_BUFFER_INCR); /* - * 1024 bytes should already be available, so this call does + * 512 bytes should already be available, so this call does * nothing. */ - result = isc_buffer_reserve(b, 1024); + result = isc_buffer_reserve(b, 512); assert_int_equal(result, ISC_R_SUCCESS); - assert_true(ISC_BUFFER_VALID(b)); assert_non_null(b); - assert_int_equal(b->length, 1024); + assert_int_equal(b->length, ISC_BUFFER_INCR); /* - * This call should grow it to 2048 bytes as only 1024 bytes are + * This call should grow it to 1536 bytes as only 1024 bytes are * available in the buffer. */ result = isc_buffer_reserve(b, 1025); assert_int_equal(result, ISC_R_SUCCESS); - assert_true(ISC_BUFFER_VALID(b)); assert_non_null(b); - assert_int_equal(b->length, 2048); + assert_int_equal(b->length, 3 * ISC_BUFFER_INCR); /* - * 2048 bytes should already be available, so this call does + * 1536 bytes should already be available, so this call does * nothing. */ - result = isc_buffer_reserve(b, 2000); + result = isc_buffer_reserve(b, 1500); assert_int_equal(result, ISC_R_SUCCESS); - assert_true(ISC_BUFFER_VALID(b)); assert_non_null(b); - assert_int_equal(b->length, 2048); + assert_int_equal(b->length, 3 * ISC_BUFFER_INCR); /* - * This call should grow it to 4096 bytes as only 2048 bytes are + * This call should grow it to 4096 bytes as only 1536 bytes are * available in the buffer. */ - result = isc_buffer_reserve(b, 3000); + result = isc_buffer_reserve(b, 3585); assert_int_equal(result, ISC_R_SUCCESS); - assert_true(ISC_BUFFER_VALID(b)); assert_non_null(b); - assert_int_equal(b->length, 4096); + assert_int_equal(b->length, 8 * ISC_BUFFER_INCR); /* Consume some of the buffer so we can run the next test. */ isc_buffer_add(b, 4096); @@ -93,9 +89,8 @@ ISC_RUN_TEST_IMPL(isc_buffer_reserve) { */ result = isc_buffer_reserve(b, UINT_MAX); assert_int_equal(result, ISC_R_NOMEMORY); - assert_true(ISC_BUFFER_VALID(b)); assert_non_null(b); - assert_int_equal(b->length, 4096); + assert_int_equal(b->length, 8 * ISC_BUFFER_INCR); isc_buffer_free(&b); } From 302bc503400af26aead30035d842a39533d5743f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 16 Dec 2022 11:11:59 +0100 Subject: [PATCH 07/11] Cleanup the isc_buffer_reserve() usage in put*() helpers There are couple places where we use putstr(), putmem(), ... helpers that tries to reserve space and only if successful puts the data onto the buffer. Cleanup the double reference as it's not needed there. From 8e3a86f6dd568b59673794d3315a47a3bc301d87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 16 Dec 2022 11:17:25 +0100 Subject: [PATCH 08/11] Make the isc_buffer unit header-only The isc_buffer is often used in the hot-path, so make it header-only implementation. --- lib/isc/Makefile.am | 1 - lib/isc/buffer.c | 237 --------------------------- lib/isc/include/isc/buffer.h | 306 +++++++++++++++++++++++++++++------ 3 files changed, 261 insertions(+), 283 deletions(-) delete mode 100644 lib/isc/buffer.c diff --git a/lib/isc/Makefile.am b/lib/isc/Makefile.am index f44a5a00dd..e3ead9b465 100644 --- a/lib/isc/Makefile.am +++ b/lib/isc/Makefile.am @@ -127,7 +127,6 @@ libisc_la_SOURCES = \ base32.c \ base64.c \ bind9.c \ - buffer.c \ commandline.c \ condition.c \ counter.c \ diff --git a/lib/isc/buffer.c b/lib/isc/buffer.c deleted file mode 100644 index 9e2c3bf339..0000000000 --- a/lib/isc/buffer.c +++ /dev/null @@ -1,237 +0,0 @@ -/* - * Copyright (C) Internet Systems Consortium, Inc. ("ISC") - * - * SPDX-License-Identifier: MPL-2.0 - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, you can obtain one at https://mozilla.org/MPL/2.0/. - * - * See the COPYRIGHT file distributed with this work for additional - * information regarding copyright ownership. - */ -/*! \file */ - -#include -#include -#include - -#include -#include -#include -#include -#include -#include - -void -isc_buffer_reinit(isc_buffer_t *b, void *base, unsigned int length) { - /* - * Re-initialize the buffer enough to reconfigure the base of the - * buffer. We will swap in the new buffer, after copying any - * data we contain into the new buffer and adjusting all of our - * internal pointers. - * - * The buffer must not be smaller than the length of the original - * buffer. - */ - REQUIRE(b->length <= length); - REQUIRE(base != NULL); - REQUIRE(b->mctx == NULL); - - if (b->length > 0U) { - (void)memmove(base, b->base, b->length); - } - - b->base = base; - b->length = length; -} - -void -isc_buffer_compact(isc_buffer_t *b) { - unsigned int length; - void *src; - - /* - * Compact the used region by moving the remaining region so it occurs - * at the start of the buffer. The used region is shrunk by the size - * of the consumed region, and the consumed region is then made empty. - */ - - REQUIRE(ISC_BUFFER_VALID(b)); - - src = isc_buffer_current(b); - length = isc_buffer_remaininglength(b); - if (length > 0U) { - (void)memmove(b->base, src, (size_t)length); - } - - if (b->active > b->current) { - b->active -= b->current; - } else { - b->active = 0; - } - b->current = 0; - b->used = length; -} - -isc_result_t -isc_buffer_dup(isc_mem_t *mctx, isc_buffer_t **dstp, const isc_buffer_t *src) { - isc_buffer_t *dst = NULL; - isc_region_t region; - isc_result_t result; - - REQUIRE(dstp != NULL && *dstp == NULL); - REQUIRE(ISC_BUFFER_VALID(src)); - - isc_buffer_usedregion(src, ®ion); - - isc_buffer_allocate(mctx, &dst, region.length); - - result = isc_buffer_copyregion(dst, ®ion); - RUNTIME_CHECK(result == ISC_R_SUCCESS); /* NOSPACE is impossible */ - *dstp = dst; - return (ISC_R_SUCCESS); -} - -isc_result_t -isc_buffer_copyregion(isc_buffer_t *b, const isc_region_t *r) { - isc_result_t result; - - REQUIRE(ISC_BUFFER_VALID(b)); - REQUIRE(r != NULL); - - if (b->mctx) { - result = isc_buffer_reserve(b, r->length); - if (result != ISC_R_SUCCESS) { - return (result); - } - } - - if (r->length > isc_buffer_availablelength(b)) { - return (ISC_R_NOSPACE); - } - - if (r->length > 0U) { - memmove(isc_buffer_used(b), r->base, r->length); - b->used += r->length; - } - - return (ISC_R_SUCCESS); -} - -void -isc_buffer_allocate(isc_mem_t *mctx, isc_buffer_t **dbufp, - unsigned int length) { - REQUIRE(dbufp != NULL && *dbufp == NULL); - - isc_buffer_t *dbuf = isc_mem_get(mctx, sizeof(*dbuf) + length); - uint8_t *bdata = (uint8_t *)dbuf + sizeof(*dbuf); - - isc_buffer_init(dbuf, bdata, length); - dbuf->extra = length; - dbuf->mctx = mctx; - - *dbufp = dbuf; -} - -isc_result_t -isc_buffer_reserve(isc_buffer_t *dbuf, unsigned int size) { - REQUIRE(ISC_BUFFER_VALID(dbuf)); - - size_t len; - uint8_t *bdata = (uint8_t *)dbuf + sizeof(*dbuf); - - len = dbuf->length; - if ((len - dbuf->used) >= size) { - return (ISC_R_SUCCESS); - } - - if (dbuf->mctx == NULL) { - return (ISC_R_NOSPACE); - } - - /* Round to nearest buffer size increment */ - len = size + dbuf->used; - len = ISC_ALIGN(len, ISC_BUFFER_INCR); - - /* Cap at UINT_MAX */ - if (len > UINT_MAX) { - len = UINT_MAX; - } - - if ((len - dbuf->used) < size) { - return (ISC_R_NOMEMORY); - } - - if (dbuf->base == bdata) { - dbuf->base = isc_mem_get(dbuf->mctx, len); - memmove(dbuf->base, bdata, dbuf->used); - } else { - dbuf->base = isc_mem_reget(dbuf->mctx, dbuf->base, dbuf->length, - len); - } - dbuf->length = (unsigned int)len; - - return (ISC_R_SUCCESS); -} - -void -isc_buffer_free(isc_buffer_t **dbufp) { - REQUIRE(dbufp != NULL && ISC_BUFFER_VALID(*dbufp)); - REQUIRE((*dbufp)->mctx != NULL); - - isc_buffer_t *dbuf = *dbufp; - isc_mem_t *mctx = dbuf->mctx; - uint8_t *bdata = (uint8_t *)dbuf + sizeof(*dbuf); - unsigned int extra = dbuf->extra; - - *dbufp = NULL; /* destroy external reference */ - dbuf->mctx = NULL; - - if (dbuf->base != bdata) { - isc_mem_put(mctx, dbuf->base, dbuf->length); - } - - isc_buffer_invalidate(dbuf); - isc_mem_put(mctx, dbuf, sizeof(*dbuf) + extra); -} - -isc_result_t -isc_buffer_printf(isc_buffer_t *b, const char *format, ...) { - va_list ap; - int n; - isc_result_t result; - - REQUIRE(ISC_BUFFER_VALID(b)); - - va_start(ap, format); - n = vsnprintf(NULL, 0, format, ap); - va_end(ap); - - if (n < 0) { - return (ISC_R_FAILURE); - } - - if (b->mctx) { - result = isc_buffer_reserve(b, n + 1); - if (result != ISC_R_SUCCESS) { - return (result); - } - } - - if (isc_buffer_availablelength(b) < (unsigned int)n + 1) { - return (ISC_R_NOSPACE); - } - - va_start(ap, format); - n = vsnprintf(isc_buffer_used(b), n + 1, format, ap); - va_end(ap); - - if (n < 0) { - return (ISC_R_FAILURE); - } - - b->used += n; - - return (ISC_R_SUCCESS); -} diff --git a/lib/isc/include/isc/buffer.h b/lib/isc/include/isc/buffer.h index 0929e6a750..081f26dcdf 100644 --- a/lib/isc/include/isc/buffer.h +++ b/lib/isc/include/isc/buffer.h @@ -109,9 +109,11 @@ #include #include #include +#include #include #include #include +#include ISC_LANG_BEGINDECLS @@ -192,7 +194,7 @@ struct isc_buffer { *** Functions ***/ -void +static inline void isc_buffer_allocate(isc_mem_t *mctx, isc_buffer_t **dynbuffer, unsigned int length); /*!< @@ -208,7 +210,7 @@ isc_buffer_allocate(isc_mem_t *mctx, isc_buffer_t **dynbuffer, *\li Changing the buffer's length field is not permitted. */ -isc_result_t +static inline isc_result_t isc_buffer_reserve(isc_buffer_t *dynbuffer, unsigned int size); /*!< * \brief Make "size" bytes of space available in the buffer. The buffer @@ -222,7 +224,7 @@ isc_buffer_reserve(isc_buffer_t *dynbuffer, unsigned int size); *\li ISC_R_NOMEMORY - no memory available */ -void +static inline void isc_buffer_free(isc_buffer_t **dynbuffer); /*!< * \brief Release resources allocated for a dynamic buffer. @@ -238,10 +240,10 @@ isc_buffer_free(isc_buffer_t **dynbuffer); * isc_buffer_allocate(). */ -void -isc__buffer_initnull(isc_buffer_t *b); +static inline void +isc_buffer_initnull(isc_buffer_t *b); -void +static inline void isc_buffer_reinit(isc_buffer_t *b, void *base, unsigned int length); /*!< * \brief Make 'b' refer to the 'length'-byte region starting at base. @@ -255,7 +257,7 @@ isc_buffer_reinit(isc_buffer_t *b, void *base, unsigned int length); * */ -void +static inline void isc_buffer_compact(isc_buffer_t *b); /*!< * \brief Compact the used region by moving the remaining region so it occurs @@ -412,7 +414,7 @@ isc_buffer_putmem(isc_buffer_t *b, const unsigned char *base, *\li The used pointer in 'b' is advanced by 'length'. */ -isc_result_t +static inline isc_result_t isc_buffer_copyregion(isc_buffer_t *b, const isc_region_t *r); /*!< * \brief Copy the contents of 'r' into 'b'. @@ -432,7 +434,7 @@ isc_buffer_copyregion(isc_buffer_t *b, const isc_region_t *r); * big enough. */ -isc_result_t +static inline isc_result_t isc_buffer_dup(isc_mem_t *mctx, isc_buffer_t **dstp, const isc_buffer_t *src); /*!< * \brief Allocate 'dst' and copy used contents of 'src' into it. @@ -445,7 +447,7 @@ isc_buffer_dup(isc_mem_t *mctx, isc_buffer_t **dstp, const isc_buffer_t *src); *\li ISC_R_SUCCESS */ -isc_result_t +static inline isc_result_t isc_buffer_printf(isc_buffer_t *b, const char *format, ...) ISC_FORMAT_PRINTF(2, 3); /*!< @@ -511,7 +513,7 @@ isc_buffer_printf(isc_buffer_t *b, const char *format, ...) */ static inline void isc_buffer_init(isc_buffer_t *b, void *base, unsigned int length) { - ISC_REQUIRE(b != NULL); + REQUIRE(b != NULL); *b = (isc_buffer_t){ .base = base, @@ -564,9 +566,9 @@ isc_buffer_initnull(isc_buffer_t *b) { */ static inline void isc_buffer_invalidate(isc_buffer_t *b) { - ISC_REQUIRE(ISC_BUFFER_VALID(b)); - ISC_REQUIRE(!ISC_LINK_LINKED(b, link)); - ISC_REQUIRE(b->mctx == NULL); + REQUIRE(ISC_BUFFER_VALID(b)); + REQUIRE(!ISC_LINK_LINKED(b, link)); + REQUIRE(b->mctx == NULL); *b = (isc_buffer_t){ .magic = 0, @@ -584,8 +586,8 @@ isc_buffer_invalidate(isc_buffer_t *b) { */ static inline void isc_buffer_region(isc_buffer_t *b, isc_region_t *r) { - ISC_REQUIRE(ISC_BUFFER_VALID(b)); - ISC_REQUIRE(r != NULL); + REQUIRE(ISC_BUFFER_VALID(b)); + REQUIRE(r != NULL); r->base = b->base; r->length = b->length; @@ -602,8 +604,8 @@ isc_buffer_region(isc_buffer_t *b, isc_region_t *r) { */ static inline void isc_buffer_usedregion(const isc_buffer_t *b, isc_region_t *r) { - ISC_REQUIRE(ISC_BUFFER_VALID(b)); - ISC_REQUIRE(r != NULL); + REQUIRE(ISC_BUFFER_VALID(b)); + REQUIRE(r != NULL); r->base = b->base; r->length = b->used; @@ -620,8 +622,8 @@ isc_buffer_usedregion(const isc_buffer_t *b, isc_region_t *r) { */ static inline void isc_buffer_availableregion(isc_buffer_t *b, isc_region_t *r) { - ISC_REQUIRE(ISC_BUFFER_VALID(b)); - ISC_REQUIRE(r != NULL); + REQUIRE(ISC_BUFFER_VALID(b)); + REQUIRE(r != NULL); r->base = isc_buffer_used(b); r->length = isc_buffer_availablelength(b); @@ -638,8 +640,8 @@ isc_buffer_availableregion(isc_buffer_t *b, isc_region_t *r) { */ static inline void isc_buffer_add(isc_buffer_t *b, unsigned int n) { - ISC_REQUIRE(ISC_BUFFER_VALID(b)); - ISC_REQUIRE(b->used + n <= b->length); + REQUIRE(ISC_BUFFER_VALID(b)); + REQUIRE(b->used + n <= b->length); b->used += n; } @@ -655,8 +657,8 @@ isc_buffer_add(isc_buffer_t *b, unsigned int n) { */ static inline void isc_buffer_subtract(isc_buffer_t *b, unsigned int n) { - ISC_REQUIRE(ISC_BUFFER_VALID(b)); - ISC_REQUIRE(b->used >= n); + REQUIRE(ISC_BUFFER_VALID(b)); + REQUIRE(b->used >= n); b->used -= n; if (b->current > b->used) { @@ -680,7 +682,7 @@ isc_buffer_subtract(isc_buffer_t *b, unsigned int n) { */ static inline void isc_buffer_clear(isc_buffer_t *b) { - ISC_REQUIRE(ISC_BUFFER_VALID(b)); + REQUIRE(ISC_BUFFER_VALID(b)); b->used = 0; b->current = 0; @@ -698,8 +700,8 @@ isc_buffer_clear(isc_buffer_t *b) { */ static inline void isc_buffer_consumedregion(isc_buffer_t *b, isc_region_t *r) { - ISC_REQUIRE(ISC_BUFFER_VALID(b)); - ISC_REQUIRE(r != NULL); + REQUIRE(ISC_BUFFER_VALID(b)); + REQUIRE(r != NULL); r->base = b->base; r->length = b->current; @@ -716,8 +718,8 @@ isc_buffer_consumedregion(isc_buffer_t *b, isc_region_t *r) { */ static inline void isc_buffer_remainingregion(isc_buffer_t *b, isc_region_t *r) { - ISC_REQUIRE(ISC_BUFFER_VALID(b)); - ISC_REQUIRE(r != NULL); + REQUIRE(ISC_BUFFER_VALID(b)); + REQUIRE(r != NULL); r->base = isc_buffer_current(b); r->length = isc_buffer_remaininglength(b); @@ -734,8 +736,8 @@ isc_buffer_remainingregion(isc_buffer_t *b, isc_region_t *r) { */ static inline void isc_buffer_activeregion(isc_buffer_t *b, isc_region_t *r) { - ISC_REQUIRE(ISC_BUFFER_VALID(b)); - ISC_REQUIRE(r != NULL); + REQUIRE(ISC_BUFFER_VALID(b)); + REQUIRE(r != NULL); if (b->current < b->active) { r->base = isc_buffer_current(b); @@ -757,8 +759,8 @@ isc_buffer_activeregion(isc_buffer_t *b, isc_region_t *r) { */ static inline void isc_buffer_setactive(isc_buffer_t *b, unsigned int n) { - ISC_REQUIRE(ISC_BUFFER_VALID(b)); - ISC_REQUIRE(b->current + n <= b->used); + REQUIRE(ISC_BUFFER_VALID(b)); + REQUIRE(b->current + n <= b->used); b->active = b->current + n; } @@ -776,7 +778,7 @@ isc_buffer_setactive(isc_buffer_t *b, unsigned int n) { */ static inline void isc_buffer_first(isc_buffer_t *b) { - ISC_REQUIRE(ISC_BUFFER_VALID(b)); + REQUIRE(ISC_BUFFER_VALID(b)); b->current = 0; } @@ -792,8 +794,8 @@ isc_buffer_first(isc_buffer_t *b) { */ static inline void isc_buffer_forward(isc_buffer_t *b, unsigned int n) { - ISC_REQUIRE(ISC_BUFFER_VALID(b)); - ISC_REQUIRE(b->current + n <= b->used); + REQUIRE(ISC_BUFFER_VALID(b)); + REQUIRE(b->current + n <= b->used); b->current += n; } @@ -809,8 +811,8 @@ isc_buffer_forward(isc_buffer_t *b, unsigned int n) { */ static inline void isc_buffer_back(isc_buffer_t *b, unsigned int n) { - ISC_REQUIRE(ISC_BUFFER_VALID(b)); - ISC_REQUIRE(n <= b->current); + REQUIRE(ISC_BUFFER_VALID(b)); + REQUIRE(n <= b->current); b->current -= n; } @@ -960,14 +962,14 @@ isc_buffer_putuint48(isc_buffer_t *b, uint64_t val) { static inline void isc_buffer_putmem(isc_buffer_t *b, const unsigned char *base, unsigned int length) { - ISC_REQUIRE(ISC_BUFFER_VALID(b)); + REQUIRE(ISC_BUFFER_VALID(b)); if (b->mctx) { isc_result_t result = isc_buffer_reserve(b, length); - ISC_REQUIRE(result == ISC_R_SUCCESS); + REQUIRE(result == ISC_R_SUCCESS); } - ISC_REQUIRE(isc_buffer_availablelength(b) >= (unsigned int)length); + REQUIRE(isc_buffer_availablelength(b) >= (unsigned int)length); if (length > 0U) { memmove(isc_buffer_used(b), base, length); @@ -994,19 +996,233 @@ isc_buffer_putstr(isc_buffer_t *b, const char *source) { unsigned int length; unsigned char *cp; - ISC_REQUIRE(ISC_BUFFER_VALID(b)); - ISC_REQUIRE(source != NULL); + REQUIRE(ISC_BUFFER_VALID(b)); + REQUIRE(source != NULL); length = (unsigned int)strlen(source); if (b->mctx) { isc_result_t result = isc_buffer_reserve(b, length); - ISC_ENSURE(result == ISC_R_SUCCESS); + ENSURE(result == ISC_R_SUCCESS); } - ISC_REQUIRE(isc_buffer_availablelength(b) >= length); + REQUIRE(isc_buffer_availablelength(b) >= length); cp = isc_buffer_used(b); memmove(cp, source, length); b->used += length; } + +static inline void +isc_buffer_reinit(isc_buffer_t *b, void *base, unsigned int length) { + /* + * Re-initialize the buffer enough to reconfigure the base of the + * buffer. We will swap in the new buffer, after copying any + * data we contain into the new buffer and adjusting all of our + * internal pointers. + * + * The buffer must not be smaller than the length of the original + * buffer. + */ + REQUIRE(b->length <= length); + REQUIRE(base != NULL); + REQUIRE(b->mctx == NULL); + + if (b->length > 0U) { + (void)memmove(base, b->base, b->length); + } + + b->base = base; + b->length = length; +} + +static inline void +isc_buffer_compact(isc_buffer_t *b) { + unsigned int length; + void *src; + + /* + * Compact the used region by moving the remaining region so it occurs + * at the start of the buffer. The used region is shrunk by the size + * of the consumed region, and the consumed region is then made empty. + */ + + REQUIRE(ISC_BUFFER_VALID(b)); + + src = isc_buffer_current(b); + length = isc_buffer_remaininglength(b); + if (length > 0U) { + (void)memmove(b->base, src, (size_t)length); + } + + if (b->active > b->current) { + b->active -= b->current; + } else { + b->active = 0; + } + b->current = 0; + b->used = length; +} + +static inline void +isc_buffer_allocate(isc_mem_t *mctx, isc_buffer_t **dbufp, + unsigned int length) { + REQUIRE(dbufp != NULL && *dbufp == NULL); + + isc_buffer_t *dbuf = isc_mem_get(mctx, sizeof(*dbuf) + length); + uint8_t *bdata = (uint8_t *)dbuf + sizeof(*dbuf); + + isc_buffer_init(dbuf, bdata, length); + dbuf->extra = length; + dbuf->mctx = mctx; + + *dbufp = dbuf; +} + +static inline isc_result_t +isc_buffer_reserve(isc_buffer_t *dbuf, unsigned int size) { + REQUIRE(ISC_BUFFER_VALID(dbuf)); + + size_t len; + uint8_t *bdata = (uint8_t *)dbuf + sizeof(*dbuf); + + len = dbuf->length; + if ((len - dbuf->used) >= size) { + return (ISC_R_SUCCESS); + } + + if (dbuf->mctx == NULL) { + return (ISC_R_NOSPACE); + } + + /* Round to nearest buffer size increment */ + len = size + dbuf->used; + len = ISC_ALIGN(len, ISC_BUFFER_INCR); + + /* Cap at UINT_MAX */ + if (len > UINT_MAX) { + len = UINT_MAX; + } + + if ((len - dbuf->used) < size) { + return (ISC_R_NOMEMORY); + } + + if (dbuf->base == bdata) { + dbuf->base = isc_mem_get(dbuf->mctx, len); + memmove(dbuf->base, bdata, dbuf->used); + } else { + dbuf->base = isc_mem_reget(dbuf->mctx, dbuf->base, dbuf->length, + len); + } + dbuf->length = (unsigned int)len; + + return (ISC_R_SUCCESS); +} + +static inline void +isc_buffer_free(isc_buffer_t **dbufp) { + REQUIRE(dbufp != NULL && ISC_BUFFER_VALID(*dbufp)); + REQUIRE((*dbufp)->mctx != NULL); + + isc_buffer_t *dbuf = *dbufp; + isc_mem_t *mctx = dbuf->mctx; + uint8_t *bdata = (uint8_t *)dbuf + sizeof(*dbuf); + unsigned int extra = dbuf->extra; + + *dbufp = NULL; /* destroy external reference */ + dbuf->mctx = NULL; + + if (dbuf->base != bdata) { + isc_mem_put(mctx, dbuf->base, dbuf->length); + } + + isc_buffer_invalidate(dbuf); + isc_mem_put(mctx, dbuf, sizeof(*dbuf) + extra); +} + +static inline isc_result_t +isc_buffer_dup(isc_mem_t *mctx, isc_buffer_t **dstp, const isc_buffer_t *src) { + isc_buffer_t *dst = NULL; + isc_region_t region; + isc_result_t result; + + REQUIRE(dstp != NULL && *dstp == NULL); + REQUIRE(ISC_BUFFER_VALID(src)); + + isc_buffer_usedregion(src, ®ion); + + isc_buffer_allocate(mctx, &dst, region.length); + + result = isc_buffer_copyregion(dst, ®ion); + RUNTIME_CHECK(result == ISC_R_SUCCESS); /* NOSPACE is impossible */ + *dstp = dst; + return (ISC_R_SUCCESS); +} + +static inline isc_result_t +isc_buffer_copyregion(isc_buffer_t *b, const isc_region_t *r) { + isc_result_t result; + + REQUIRE(ISC_BUFFER_VALID(b)); + REQUIRE(r != NULL); + + if (b->mctx) { + result = isc_buffer_reserve(b, r->length); + if (result != ISC_R_SUCCESS) { + return (result); + } + } + + if (r->length > isc_buffer_availablelength(b)) { + return (ISC_R_NOSPACE); + } + + if (r->length > 0U) { + memmove(isc_buffer_used(b), r->base, r->length); + b->used += r->length; + } + + return (ISC_R_SUCCESS); +} + +static inline isc_result_t +isc_buffer_printf(isc_buffer_t *b, const char *format, ...) { + va_list ap; + int n; + isc_result_t result; + + REQUIRE(ISC_BUFFER_VALID(b)); + + va_start(ap, format); + n = vsnprintf(NULL, 0, format, ap); + va_end(ap); + + if (n < 0) { + return (ISC_R_FAILURE); + } + + if (b->mctx) { + result = isc_buffer_reserve(b, n + 1); + if (result != ISC_R_SUCCESS) { + return (result); + } + } + + if (isc_buffer_availablelength(b) < (unsigned int)n + 1) { + return (ISC_R_NOSPACE); + } + + va_start(ap, format); + n = vsnprintf(isc_buffer_used(b), n + 1, format, ap); + va_end(ap); + + if (n < 0) { + return (ISC_R_FAILURE); + } + + b->used += n; + + return (ISC_R_SUCCESS); +} + ISC_LANG_ENDDECLS From e6062ee3ae48d00265c7ebe53e8060fdd644a59b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 16 Dec 2022 11:43:20 +0100 Subject: [PATCH 09/11] Add isc_buffer_setmctx() and isc_buffer_clearmctx() function Add two extra functions needed by StreamDNS: 1. isc_buffer_setmctx() sets the buffer internal memory context, so we can use isc_buffer_reserve() on the buffer. For this, we also need to track whether the .base was dynamically allocated or not. This needs to be called after isc_buffer_init() and before first isc_buffer_reserve() call. 2. isc_buffer_clearmctx() clears the buffer internal memory context, and frees any dynamically allocated buffer. This needs to be called after the last isc_buffer_reserve() call and before calling the isc_buffer_invalidate() --- lib/isc/include/isc/buffer.h | 52 ++++++++++++++++++++++++++---------- 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/lib/isc/include/isc/buffer.h b/lib/isc/include/isc/buffer.h index 081f26dcdf..2877d21179 100644 --- a/lib/isc/include/isc/buffer.h +++ b/lib/isc/include/isc/buffer.h @@ -179,17 +179,16 @@ struct isc_buffer { unsigned int used; unsigned int current; unsigned int active; - /*! The extra bytes allocated for dynamic buffer */ - unsigned int extra; /*@}*/ + /*! The extra bytes allocated for static buffer */ + unsigned int extra; + bool dynamic; /*! linkable */ ISC_LINK(isc_buffer_t) link; /*! private internal elements */ isc_mem_t *mctx; }; -#define ISC_BUFFER_STATIC_SIZE 512 - /*** *** Functions ***/ @@ -210,6 +209,15 @@ isc_buffer_allocate(isc_mem_t *mctx, isc_buffer_t **dynbuffer, *\li Changing the buffer's length field is not permitted. */ +static inline void +isc_buffer_setmctx(isc_buffer_t *b, isc_mem_t *mctx); +static inline void +isc_buffer_clearmctx(isc_buffer_t *b); +/*!< + * \brief Sets/Clears the internal memory context, so isc_buffer_reserve() can + * be used on previously 'static' buffer. + */ + static inline isc_result_t isc_buffer_reserve(isc_buffer_t *dynbuffer, unsigned int size); /*!< @@ -1073,17 +1081,35 @@ isc_buffer_allocate(isc_mem_t *mctx, isc_buffer_t **dbufp, isc_buffer_init(dbuf, bdata, length); dbuf->extra = length; - dbuf->mctx = mctx; + isc_buffer_setmctx(dbuf, mctx); *dbufp = dbuf; } +static inline void +isc_buffer_setmctx(isc_buffer_t *b, isc_mem_t *mctx) { + REQUIRE(ISC_BUFFER_VALID(b)); + + b->mctx = mctx; +} + +static inline void +isc_buffer_clearmctx(isc_buffer_t *b) { + REQUIRE(ISC_BUFFER_VALID(b)); + + if (b->dynamic) { + isc_mem_put(b->mctx, b->base, b->length); + b->dynamic = false; + } + + b->mctx = NULL; +} + static inline isc_result_t isc_buffer_reserve(isc_buffer_t *dbuf, unsigned int size) { REQUIRE(ISC_BUFFER_VALID(dbuf)); - size_t len; - uint8_t *bdata = (uint8_t *)dbuf + sizeof(*dbuf); + size_t len; len = dbuf->length; if ((len - dbuf->used) >= size) { @@ -1107,9 +1133,11 @@ isc_buffer_reserve(isc_buffer_t *dbuf, unsigned int size) { return (ISC_R_NOMEMORY); } - if (dbuf->base == bdata) { + if (!dbuf->dynamic) { + void *old_base = dbuf->base; dbuf->base = isc_mem_get(dbuf->mctx, len); - memmove(dbuf->base, bdata, dbuf->used); + memmove(dbuf->base, old_base, dbuf->used); + dbuf->dynamic = true; } else { dbuf->base = isc_mem_reget(dbuf->mctx, dbuf->base, dbuf->length, len); @@ -1126,15 +1154,11 @@ isc_buffer_free(isc_buffer_t **dbufp) { isc_buffer_t *dbuf = *dbufp; isc_mem_t *mctx = dbuf->mctx; - uint8_t *bdata = (uint8_t *)dbuf + sizeof(*dbuf); unsigned int extra = dbuf->extra; *dbufp = NULL; /* destroy external reference */ - dbuf->mctx = NULL; - if (dbuf->base != bdata) { - isc_mem_put(mctx, dbuf->base, dbuf->length); - } + isc_buffer_clearmctx(dbuf); isc_buffer_invalidate(dbuf); isc_mem_put(mctx, dbuf, sizeof(*dbuf) + extra); From 460afcda183c6348e03ea6f8830821cadf0d3665 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 16 Dec 2022 11:43:20 +0100 Subject: [PATCH 10/11] Add isc_buffer_trycompact() function needed for StreamDNS Add isc_buffer_trycompact() that's an optimization; it will compact the buffer only when the remaining length is smaller than used length. --- lib/isc/include/isc/buffer.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/isc/include/isc/buffer.h b/lib/isc/include/isc/buffer.h index 2877d21179..16df98dc28 100644 --- a/lib/isc/include/isc/buffer.h +++ b/lib/isc/include/isc/buffer.h @@ -265,6 +265,8 @@ isc_buffer_reinit(isc_buffer_t *b, void *base, unsigned int length); * */ +static inline void +isc_buffer_trycompact(isc_buffer_t *b); static inline void isc_buffer_compact(isc_buffer_t *b); /*!< @@ -1043,6 +1045,13 @@ isc_buffer_reinit(isc_buffer_t *b, void *base, unsigned int length) { b->length = length; } +static inline void +isc_buffer_trycompact(isc_buffer_t *b) { + if (isc_buffer_consumedlength(b) >= isc_buffer_remaininglength(b)) { + isc_buffer_compact(b); + } +} + static inline void isc_buffer_compact(isc_buffer_t *b) { unsigned int length; From 94e650ce891c61c62553fcdc84a9c0be51edf717 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Tue, 20 Dec 2022 20:56:22 +0200 Subject: [PATCH 11/11] Use 'restrict' and 'const' for 'isc_buffer_t' The purpose of this commit is to aid compiler in generating better code when working with `isc_buffer_t` objects by using restricted pointers (and, to a lesser extent, 'const' modifier for read-only arguments). This way we, basically, instruct the compiler that the members of structured passed by pointers into the functions can be treated as local variables in the scope of a function. That should reduce the number of load/store operations emitted by compilers when accessing objects (e.g. 'isc_buffer_t') via pointers. --- lib/isc/include/isc/buffer.h | 147 ++++++++++++++++++----------------- 1 file changed, 77 insertions(+), 70 deletions(-) diff --git a/lib/isc/include/isc/buffer.h b/lib/isc/include/isc/buffer.h index 16df98dc28..d9cec44c6b 100644 --- a/lib/isc/include/isc/buffer.h +++ b/lib/isc/include/isc/buffer.h @@ -194,8 +194,8 @@ struct isc_buffer { ***/ static inline void -isc_buffer_allocate(isc_mem_t *mctx, isc_buffer_t **dynbuffer, - unsigned int length); +isc_buffer_allocate(isc_mem_t *mctx, isc_buffer_t **restrict dynbuffer, + const unsigned int length); /*!< * \brief Allocate a dynamic linkable buffer which has "length" bytes in the * data region. @@ -210,16 +210,16 @@ isc_buffer_allocate(isc_mem_t *mctx, isc_buffer_t **dynbuffer, */ static inline void -isc_buffer_setmctx(isc_buffer_t *b, isc_mem_t *mctx); +isc_buffer_setmctx(isc_buffer_t *restrict b, isc_mem_t *mctx); static inline void -isc_buffer_clearmctx(isc_buffer_t *b); +isc_buffer_clearmctx(isc_buffer_t *restrict b); /*!< * \brief Sets/Clears the internal memory context, so isc_buffer_reserve() can * be used on previously 'static' buffer. */ static inline isc_result_t -isc_buffer_reserve(isc_buffer_t *dynbuffer, unsigned int size); +isc_buffer_reserve(isc_buffer_t *restrict dynbuffer, const unsigned int size); /*!< * \brief Make "size" bytes of space available in the buffer. The buffer * pointer may move when you call this function. @@ -233,7 +233,7 @@ isc_buffer_reserve(isc_buffer_t *dynbuffer, unsigned int size); */ static inline void -isc_buffer_free(isc_buffer_t **dynbuffer); +isc_buffer_free(isc_buffer_t **restrict dynbuffer); /*!< * \brief Release resources allocated for a dynamic buffer. * @@ -249,10 +249,11 @@ isc_buffer_free(isc_buffer_t **dynbuffer); */ static inline void -isc_buffer_initnull(isc_buffer_t *b); +isc_buffer_initnull(isc_buffer_t *restrict b); static inline void -isc_buffer_reinit(isc_buffer_t *b, void *base, unsigned int length); +isc_buffer_reinit(isc_buffer_t *restrict b, void *base, + const unsigned int length); /*!< * \brief Make 'b' refer to the 'length'-byte region starting at base. * Any existing data will be copied. @@ -266,9 +267,9 @@ isc_buffer_reinit(isc_buffer_t *b, void *base, unsigned int length); */ static inline void -isc_buffer_trycompact(isc_buffer_t *b); +isc_buffer_trycompact(isc_buffer_t *restrict b); static inline void -isc_buffer_compact(isc_buffer_t *b); +isc_buffer_compact(isc_buffer_t *restrict b); /*!< * \brief Compact the used region by moving the remaining region so it occurs * at the start of the buffer. The used region is shrunk by the size of @@ -288,11 +289,11 @@ isc_buffer_compact(isc_buffer_t *b); */ static inline isc_result_t -isc_buffer_peekuint8(const isc_buffer_t *b, uint8_t *valp); +isc_buffer_peekuint8(const isc_buffer_t *restrict b, uint8_t *valp); static inline uint8_t -isc_buffer_getuint8(isc_buffer_t *b); +isc_buffer_getuint8(isc_buffer_t *restrict b); static inline void -isc_buffer_putuint8(isc_buffer_t *b, uint8_t val); +isc_buffer_putuint8(isc_buffer_t *restrict b, const uint8_t val); /*!< * \brief Peek/Read/Write an unsigned 8-bit integer from/to 'b'. * @@ -317,11 +318,11 @@ isc_buffer_putuint8(isc_buffer_t *b, uint8_t val); */ static inline isc_result_t -isc_buffer_peekuint16(const isc_buffer_t *b, uint16_t *valp); +isc_buffer_peekuint16(const isc_buffer_t *restrict b, uint16_t *valp); static inline uint16_t -isc_buffer_getuint16(isc_buffer_t *b); +isc_buffer_getuint16(isc_buffer_t *restrict b); static inline void -isc_buffer_putuint16(isc_buffer_t *b, uint16_t val); +isc_buffer_putuint16(isc_buffer_t *restrict b, const uint16_t val); /*!< * \brief Peek/Read/Write an unsigned 16-bit integer in network byte order * from/to 'b', convert it to/from host byte order.. @@ -347,11 +348,11 @@ isc_buffer_putuint16(isc_buffer_t *b, uint16_t val); */ static inline isc_result_t -isc_buffer_peekuint32(const isc_buffer_t *b, uint32_t *valp); +isc_buffer_peekuint32(const isc_buffer_t *restrict b, uint32_t *restrict valp); static inline uint32_t -isc_buffer_getuint32(isc_buffer_t *b); +isc_buffer_getuint32(isc_buffer_t *restrict b); static inline void -isc_buffer_putuint32(isc_buffer_t *b, uint32_t val); +isc_buffer_putuint32(isc_buffer_t *restrict b, uint32_t const val); /*!< * \brief Peek/Read/Write an unsigned 32-bit integer in network byte order * from/to 'b', convert it to/from host byte order. @@ -377,11 +378,11 @@ isc_buffer_putuint32(isc_buffer_t *b, uint32_t val); */ static inline isc_result_t -isc_buffer_peekuint48(const isc_buffer_t *b, uint64_t *valp); +isc_buffer_peekuint48(const isc_buffer_t *restrict b, uint64_t *valp); static inline uint64_t -isc_buffer_getuint48(isc_buffer_t *b); +isc_buffer_getuint48(isc_buffer_t *restrict b); static inline void -isc_buffer_putuint48(isc_buffer_t *b, uint64_t val); +isc_buffer_putuint48(isc_buffer_t *restrict b, const uint64_t val); /*!< * \brief Peek/Read/Write an unsigned 48-bit integer in network byte order * from/to 'b', convert it to/from host byte order. @@ -407,8 +408,8 @@ isc_buffer_putuint48(isc_buffer_t *b, uint64_t val); */ static inline void -isc_buffer_putmem(isc_buffer_t *b, const unsigned char *base, - unsigned int length); +isc_buffer_putmem(isc_buffer_t *restrict b, const unsigned char *restrict base, + const unsigned int length); /*!< * \brief Copy 'length' bytes of memory at 'base' into 'b'. * @@ -425,7 +426,7 @@ isc_buffer_putmem(isc_buffer_t *b, const unsigned char *base, */ static inline isc_result_t -isc_buffer_copyregion(isc_buffer_t *b, const isc_region_t *r); +isc_buffer_copyregion(isc_buffer_t *restrict b, const isc_region_t *restrict r); /*!< * \brief Copy the contents of 'r' into 'b'. * @@ -445,7 +446,8 @@ isc_buffer_copyregion(isc_buffer_t *b, const isc_region_t *r); */ static inline isc_result_t -isc_buffer_dup(isc_mem_t *mctx, isc_buffer_t **dstp, const isc_buffer_t *src); +isc_buffer_dup(isc_mem_t *mctx, isc_buffer_t **restrict dstp, + const isc_buffer_t *restrict src); /*!< * \brief Allocate 'dst' and copy used contents of 'src' into it. * @@ -458,7 +460,7 @@ isc_buffer_dup(isc_mem_t *mctx, isc_buffer_t **dstp, const isc_buffer_t *src); */ static inline isc_result_t -isc_buffer_printf(isc_buffer_t *b, const char *format, ...) +isc_buffer_printf(isc_buffer_t *restrict b, const char *restrict format, ...) ISC_FORMAT_PRINTF(2, 3); /*!< * \brief Append a formatted string to the used region of 'b'. @@ -522,7 +524,8 @@ isc_buffer_printf(isc_buffer_t *b, const char *format, ...) *\li 'base' is a pointer to a sequence of 'length' bytes. */ static inline void -isc_buffer_init(isc_buffer_t *b, void *base, unsigned int length) { +isc_buffer_init(isc_buffer_t *restrict b, void *base, + const unsigned int length) { REQUIRE(b != NULL); *b = (isc_buffer_t){ @@ -538,7 +541,7 @@ isc_buffer_init(isc_buffer_t *b, void *base, unsigned int length) { * This can later be grown as needed and swapped in place. */ static inline void -isc_buffer_initnull(isc_buffer_t *b) { +isc_buffer_initnull(isc_buffer_t *restrict b) { *b = (isc_buffer_t){ .link = ISC_LINK_INITIALIZER, .magic = ISC_BUFFER_MAGIC, @@ -575,7 +578,7 @@ isc_buffer_initnull(isc_buffer_t *b) { * it will cause an assertion failure. */ static inline void -isc_buffer_invalidate(isc_buffer_t *b) { +isc_buffer_invalidate(isc_buffer_t *restrict b) { REQUIRE(ISC_BUFFER_VALID(b)); REQUIRE(!ISC_LINK_LINKED(b, link)); REQUIRE(b->mctx == NULL); @@ -595,7 +598,7 @@ isc_buffer_invalidate(isc_buffer_t *b) { *\li 'r' points to a region structure. */ static inline void -isc_buffer_region(isc_buffer_t *b, isc_region_t *r) { +isc_buffer_region(isc_buffer_t *restrict b, isc_region_t *restrict r) { REQUIRE(ISC_BUFFER_VALID(b)); REQUIRE(r != NULL); @@ -613,7 +616,8 @@ isc_buffer_region(isc_buffer_t *b, isc_region_t *r) { *\li 'r' points to a region structure. */ static inline void -isc_buffer_usedregion(const isc_buffer_t *b, isc_region_t *r) { +isc_buffer_usedregion(const isc_buffer_t *restrict b, + isc_region_t *restrict r) { REQUIRE(ISC_BUFFER_VALID(b)); REQUIRE(r != NULL); @@ -631,7 +635,7 @@ isc_buffer_usedregion(const isc_buffer_t *b, isc_region_t *r) { *\li 'r' points to a region structure. */ static inline void -isc_buffer_availableregion(isc_buffer_t *b, isc_region_t *r) { +isc_buffer_availableregion(isc_buffer_t *restrict b, isc_region_t *restrict r) { REQUIRE(ISC_BUFFER_VALID(b)); REQUIRE(r != NULL); @@ -649,7 +653,7 @@ isc_buffer_availableregion(isc_buffer_t *b, isc_region_t *r) { *\li used + n <= length */ static inline void -isc_buffer_add(isc_buffer_t *b, unsigned int n) { +isc_buffer_add(isc_buffer_t *restrict b, const unsigned int n) { REQUIRE(ISC_BUFFER_VALID(b)); REQUIRE(b->used + n <= b->length); @@ -666,7 +670,7 @@ isc_buffer_add(isc_buffer_t *b, unsigned int n) { *\li used >= n */ static inline void -isc_buffer_subtract(isc_buffer_t *b, unsigned int n) { +isc_buffer_subtract(isc_buffer_t *restrict b, const unsigned int n) { REQUIRE(ISC_BUFFER_VALID(b)); REQUIRE(b->used >= n); @@ -691,7 +695,7 @@ isc_buffer_subtract(isc_buffer_t *b, unsigned int n) { *\li used = 0 */ static inline void -isc_buffer_clear(isc_buffer_t *b) { +isc_buffer_clear(isc_buffer_t *restrict b) { REQUIRE(ISC_BUFFER_VALID(b)); b->used = 0; @@ -709,7 +713,7 @@ isc_buffer_clear(isc_buffer_t *b) { *\li 'r' points to a region structure. */ static inline void -isc_buffer_consumedregion(isc_buffer_t *b, isc_region_t *r) { +isc_buffer_consumedregion(isc_buffer_t *restrict b, isc_region_t *restrict r) { REQUIRE(ISC_BUFFER_VALID(b)); REQUIRE(r != NULL); @@ -727,7 +731,7 @@ isc_buffer_consumedregion(isc_buffer_t *b, isc_region_t *r) { *\li 'r' points to a region structure. */ static inline void -isc_buffer_remainingregion(isc_buffer_t *b, isc_region_t *r) { +isc_buffer_remainingregion(isc_buffer_t *restrict b, isc_region_t *restrict r) { REQUIRE(ISC_BUFFER_VALID(b)); REQUIRE(r != NULL); @@ -745,7 +749,7 @@ isc_buffer_remainingregion(isc_buffer_t *b, isc_region_t *r) { *\li 'r' points to a region structure. */ static inline void -isc_buffer_activeregion(isc_buffer_t *b, isc_region_t *r) { +isc_buffer_activeregion(isc_buffer_t *restrict b, isc_region_t *restrict r) { REQUIRE(ISC_BUFFER_VALID(b)); REQUIRE(r != NULL); @@ -768,7 +772,7 @@ isc_buffer_activeregion(isc_buffer_t *b, isc_region_t *r) { *\li current + n <= used */ static inline void -isc_buffer_setactive(isc_buffer_t *b, unsigned int n) { +isc_buffer_setactive(isc_buffer_t *restrict b, const unsigned int n) { REQUIRE(ISC_BUFFER_VALID(b)); REQUIRE(b->current + n <= b->used); @@ -787,7 +791,7 @@ isc_buffer_setactive(isc_buffer_t *b, unsigned int n) { *\li current == 0 */ static inline void -isc_buffer_first(isc_buffer_t *b) { +isc_buffer_first(isc_buffer_t *restrict b) { REQUIRE(ISC_BUFFER_VALID(b)); b->current = 0; @@ -803,7 +807,7 @@ isc_buffer_first(isc_buffer_t *b) { *\li current + n <= used */ static inline void -isc_buffer_forward(isc_buffer_t *b, unsigned int n) { +isc_buffer_forward(isc_buffer_t *restrict b, const unsigned int n) { REQUIRE(ISC_BUFFER_VALID(b)); REQUIRE(b->current + n <= b->used); @@ -820,7 +824,7 @@ isc_buffer_forward(isc_buffer_t *b, unsigned int n) { *\li n <= current */ static inline void -isc_buffer_back(isc_buffer_t *b, unsigned int n) { +isc_buffer_back(isc_buffer_t *restrict b, const unsigned int n) { REQUIRE(ISC_BUFFER_VALID(b)); REQUIRE(n <= b->current); @@ -836,7 +840,7 @@ isc_buffer_back(isc_buffer_t *b, unsigned int n) { } static inline isc_result_t -isc_buffer_peekuint8(const isc_buffer_t *b, uint8_t *valp) { +isc_buffer_peekuint8(const isc_buffer_t *restrict b, uint8_t *valp) { ISC_BUFFER_PEEK_CHECK(b, sizeof(*valp)); uint8_t *cp = isc_buffer_current(b); @@ -847,7 +851,7 @@ isc_buffer_peekuint8(const isc_buffer_t *b, uint8_t *valp) { } static inline uint8_t -isc_buffer_getuint8(isc_buffer_t *b) { +isc_buffer_getuint8(isc_buffer_t *restrict b) { uint8_t val = 0; isc_result_t result = isc_buffer_peekuint8(b, &val); ENSURE(result == ISC_R_SUCCESS); @@ -869,7 +873,7 @@ isc_buffer_getuint8(isc_buffer_t *b) { } static inline void -isc_buffer_putuint8(isc_buffer_t *b, uint8_t val) { +isc_buffer_putuint8(isc_buffer_t *restrict b, const uint8_t val) { ISC_BUFFER_PUT_RESERVE(b, val); uint8_t *cp = isc_buffer_used(b); @@ -878,7 +882,7 @@ isc_buffer_putuint8(isc_buffer_t *b, uint8_t val) { } static inline isc_result_t -isc_buffer_peekuint16(const isc_buffer_t *b, uint16_t *valp) { +isc_buffer_peekuint16(const isc_buffer_t *restrict b, uint16_t *valp) { ISC_BUFFER_PEEK_CHECK(b, sizeof(*valp)); uint8_t *cp = isc_buffer_current(b); @@ -890,7 +894,7 @@ isc_buffer_peekuint16(const isc_buffer_t *b, uint16_t *valp) { } static inline uint16_t -isc_buffer_getuint16(isc_buffer_t *b) { +isc_buffer_getuint16(isc_buffer_t *restrict b) { uint16_t val = 0; isc_result_t result = isc_buffer_peekuint16(b, &val); ENSURE(result == ISC_R_SUCCESS); @@ -899,7 +903,7 @@ isc_buffer_getuint16(isc_buffer_t *b) { } static inline void -isc_buffer_putuint16(isc_buffer_t *b, uint16_t val) { +isc_buffer_putuint16(isc_buffer_t *restrict b, const uint16_t val) { ISC_BUFFER_PUT_RESERVE(b, val); uint8_t *cp = isc_buffer_used(b); @@ -908,7 +912,7 @@ isc_buffer_putuint16(isc_buffer_t *b, uint16_t val) { } static inline isc_result_t -isc_buffer_peekuint32(const isc_buffer_t *b, uint32_t *valp) { +isc_buffer_peekuint32(const isc_buffer_t *restrict b, uint32_t *valp) { ISC_BUFFER_PEEK_CHECK(b, sizeof(*valp)); uint8_t *cp = isc_buffer_current(b); @@ -920,7 +924,7 @@ isc_buffer_peekuint32(const isc_buffer_t *b, uint32_t *valp) { } uint32_t -isc_buffer_getuint32(isc_buffer_t *b) { +isc_buffer_getuint32(isc_buffer_t *restrict b) { uint32_t val = 0; isc_result_t result = isc_buffer_peekuint32(b, &val); ENSURE(result == ISC_R_SUCCESS); @@ -929,7 +933,7 @@ isc_buffer_getuint32(isc_buffer_t *b) { } static inline void -isc_buffer_putuint32(isc_buffer_t *b, uint32_t val) { +isc_buffer_putuint32(isc_buffer_t *restrict b, const uint32_t val) { ISC_BUFFER_PUT_RESERVE(b, val); uint8_t *cp = isc_buffer_used(b); @@ -939,7 +943,7 @@ isc_buffer_putuint32(isc_buffer_t *b, uint32_t val) { } static inline isc_result_t -isc_buffer_peekuint48(const isc_buffer_t *b, uint64_t *valp) { +isc_buffer_peekuint48(const isc_buffer_t *restrict b, uint64_t *valp) { ISC_BUFFER_PEEK_CHECK(b, 6); /* 48-bits */ uint8_t *cp = isc_buffer_current(b); @@ -951,7 +955,7 @@ isc_buffer_peekuint48(const isc_buffer_t *b, uint64_t *valp) { } static inline uint64_t -isc_buffer_getuint48(isc_buffer_t *b) { +isc_buffer_getuint48(isc_buffer_t *restrict b) { uint64_t val = 0; isc_result_t result = isc_buffer_peekuint48(b, &val); ENSURE(result == ISC_R_SUCCESS); @@ -960,7 +964,7 @@ isc_buffer_getuint48(isc_buffer_t *b) { } static inline void -isc_buffer_putuint48(isc_buffer_t *b, uint64_t val) { +isc_buffer_putuint48(isc_buffer_t *restrict b, const uint64_t val) { ISC_BUFFER_PUT_RESERVE(b, val); uint8_t *cp = isc_buffer_used(b); @@ -970,8 +974,8 @@ isc_buffer_putuint48(isc_buffer_t *b, uint64_t val) { } static inline void -isc_buffer_putmem(isc_buffer_t *b, const unsigned char *base, - unsigned int length) { +isc_buffer_putmem(isc_buffer_t *restrict b, const unsigned char *restrict base, + const unsigned int length) { REQUIRE(ISC_BUFFER_VALID(b)); if (b->mctx) { @@ -1002,7 +1006,7 @@ isc_buffer_putmem(isc_buffer_t *b, const unsigned char *base, *\li The used pointer in 'b' is advanced by strlen('source'). */ static inline void -isc_buffer_putstr(isc_buffer_t *b, const char *source) { +isc_buffer_putstr(isc_buffer_t *restrict b, const char *restrict source) { unsigned int length; unsigned char *cp; @@ -1023,7 +1027,8 @@ isc_buffer_putstr(isc_buffer_t *b, const char *source) { } static inline void -isc_buffer_reinit(isc_buffer_t *b, void *base, unsigned int length) { +isc_buffer_reinit(isc_buffer_t *restrict b, void *base, + const unsigned int length) { /* * Re-initialize the buffer enough to reconfigure the base of the * buffer. We will swap in the new buffer, after copying any @@ -1046,14 +1051,14 @@ isc_buffer_reinit(isc_buffer_t *b, void *base, unsigned int length) { } static inline void -isc_buffer_trycompact(isc_buffer_t *b) { +isc_buffer_trycompact(isc_buffer_t *restrict b) { if (isc_buffer_consumedlength(b) >= isc_buffer_remaininglength(b)) { isc_buffer_compact(b); } } static inline void -isc_buffer_compact(isc_buffer_t *b) { +isc_buffer_compact(isc_buffer_t *restrict b) { unsigned int length; void *src; @@ -1081,8 +1086,8 @@ isc_buffer_compact(isc_buffer_t *b) { } static inline void -isc_buffer_allocate(isc_mem_t *mctx, isc_buffer_t **dbufp, - unsigned int length) { +isc_buffer_allocate(isc_mem_t *mctx, isc_buffer_t **restrict dbufp, + const unsigned int length) { REQUIRE(dbufp != NULL && *dbufp == NULL); isc_buffer_t *dbuf = isc_mem_get(mctx, sizeof(*dbuf) + length); @@ -1096,14 +1101,14 @@ isc_buffer_allocate(isc_mem_t *mctx, isc_buffer_t **dbufp, } static inline void -isc_buffer_setmctx(isc_buffer_t *b, isc_mem_t *mctx) { +isc_buffer_setmctx(isc_buffer_t *restrict b, isc_mem_t *mctx) { REQUIRE(ISC_BUFFER_VALID(b)); b->mctx = mctx; } static inline void -isc_buffer_clearmctx(isc_buffer_t *b) { +isc_buffer_clearmctx(isc_buffer_t *restrict b) { REQUIRE(ISC_BUFFER_VALID(b)); if (b->dynamic) { @@ -1115,7 +1120,7 @@ isc_buffer_clearmctx(isc_buffer_t *b) { } static inline isc_result_t -isc_buffer_reserve(isc_buffer_t *dbuf, unsigned int size) { +isc_buffer_reserve(isc_buffer_t *restrict dbuf, const unsigned int size) { REQUIRE(ISC_BUFFER_VALID(dbuf)); size_t len; @@ -1157,7 +1162,7 @@ isc_buffer_reserve(isc_buffer_t *dbuf, unsigned int size) { } static inline void -isc_buffer_free(isc_buffer_t **dbufp) { +isc_buffer_free(isc_buffer_t **restrict dbufp) { REQUIRE(dbufp != NULL && ISC_BUFFER_VALID(*dbufp)); REQUIRE((*dbufp)->mctx != NULL); @@ -1174,7 +1179,8 @@ isc_buffer_free(isc_buffer_t **dbufp) { } static inline isc_result_t -isc_buffer_dup(isc_mem_t *mctx, isc_buffer_t **dstp, const isc_buffer_t *src) { +isc_buffer_dup(isc_mem_t *mctx, isc_buffer_t **restrict dstp, + const isc_buffer_t *restrict src) { isc_buffer_t *dst = NULL; isc_region_t region; isc_result_t result; @@ -1193,7 +1199,8 @@ isc_buffer_dup(isc_mem_t *mctx, isc_buffer_t **dstp, const isc_buffer_t *src) { } static inline isc_result_t -isc_buffer_copyregion(isc_buffer_t *b, const isc_region_t *r) { +isc_buffer_copyregion(isc_buffer_t *restrict b, + const isc_region_t *restrict r) { isc_result_t result; REQUIRE(ISC_BUFFER_VALID(b)); @@ -1219,7 +1226,7 @@ isc_buffer_copyregion(isc_buffer_t *b, const isc_region_t *r) { } static inline isc_result_t -isc_buffer_printf(isc_buffer_t *b, const char *format, ...) { +isc_buffer_printf(isc_buffer_t *restrict b, const char *restrict format, ...) { va_list ap; int n; isc_result_t result;