Refactor the common buffer manipulation in rdataslab.c in macros

The rdataslab.c was full of code like this:

        length = raw[0] * 256 + raw[1];

and

        count2 = *current2++ * 256;
        count2 += *current2++;

Refactor code like this into peek_uint16() and get_uint16 macros
to prevent code repetition and possible mistakes when copy and
pasting the same code over and over.

As a side note for an entertainment of a careful reader of the commit
messages: The byte manipulation was changed from multiplication and
addition to shift with or.

The difference in the assembly looks like this:

MUL and ADD:

	movzx   eax, BYTE PTR [rdi]
        movzx   edi, BYTE PTR [rdi+1]
        sal     eax, 8
        or      edi, eax

SHIFT and OR:

        movzx   edi, WORD PTR [rdi]
        rol     di, 8
        movzx   edi, di

If the result and/or buffer is then being used after the macro call,
there's more differences in favor of the SHIFT+OR solution.

diff --git a/lib/dns/rdataslab.c b/lib/dns/rdataslab.c
index 941e60a3181285c9e2e25ad14fc6d12f59bc2085..6063c1be9ce555094e96a2a88de9f8ba0435e095 100644 (file)
--- a/lib/dns/rdataslab.c
+++ b/lib/dns/rdataslab.c
@@ -89,6 +89,15 @@ struct xrdata {
 #endif /* if DNS_RDATASET_FIXED */
 };
 
+#define peek_uint16(buffer) \
+       ({ ((uint16_t) * (buffer) << 8) | *((buffer) + 1); })
+#define get_uint16(buffer)                            \
+       ({                                            \
+               uint16_t __ret = peek_uint16(buffer); \
+               buffer += sizeof(uint16_t);           \
+               __ret;                                \
+       })
+
 static void
 rdataset_disassociate(dns_rdataset_t *rdataset DNS__DB_FLARG);
 static isc_result_t
@@ -381,25 +390,20 @@ free_rdatas:
 
 unsigned int
 dns_rdataslab_size(unsigned char *slab, unsigned int reservelen) {
-       unsigned int count, length;
-       unsigned char *current = NULL;
-
        REQUIRE(slab != NULL);
 
-       current = slab + reservelen;
-       count = *current++ * 256;
-       count += *current++;
+       unsigned char *current = slab + reservelen;
+       uint16_t count = get_uint16(current);
+
 #if DNS_RDATASET_FIXED
        current += (4 * count);
 #endif /* if DNS_RDATASET_FIXED */
-       while (count > 0) {
-               count--;
-               length = *current++ * 256;
-               length += *current++;
-#if DNS_RDATASET_FIXED
-               current += length + 2;
-#else  /* if DNS_RDATASET_FIXED */
+
+       while (count-- > 0) {
+               uint16_t length = get_uint16(current);
                current += length;
+#if DNS_RDATASET_FIXED
+               current += 2;
 #endif /* if DNS_RDATASET_FIXED */
        }
 
@@ -408,26 +412,22 @@ dns_rdataslab_size(unsigned char *slab, unsigned int reservelen) {
 
 unsigned int
 dns_rdataslab_rdatasize(unsigned char *slab, unsigned int reservelen) {
-       unsigned int count, length, rdatalen = 0;
-       unsigned char *current = NULL;
-
        REQUIRE(slab != NULL);
 
-       current = slab + reservelen;
-       count = *current++ * 256;
-       count += *current++;
+       uint16_t rdatalen = 0;
+       unsigned char *current = slab + reservelen;
+       uint16_t count = get_uint16(current);
+
 #if DNS_RDATASET_FIXED
        current += (4 * count);
 #endif /* if DNS_RDATASET_FIXED */
-       while (count > 0) {
-               count--;
-               length = *current++ * 256;
-               length += *current++;
+
+       while (count-- > 0) {
+               uint16_t length = get_uint16(current);
                rdatalen += length;
-#if DNS_RDATASET_FIXED
-               current += length + 2;
-#else  /* if DNS_RDATASET_FIXED */
                current += length;
+#if DNS_RDATASET_FIXED
+               current += 2;
 #endif /* if DNS_RDATASET_FIXED */
        }
 
@@ -436,14 +436,11 @@ dns_rdataslab_rdatasize(unsigned char *slab, unsigned int reservelen) {
 
 unsigned int
 dns_rdataslab_count(unsigned char *slab, unsigned int reservelen) {
-       unsigned int count;
-       unsigned char *current = NULL;
-
        REQUIRE(slab != NULL);
 
-       current = slab + reservelen;
-       count = *current++ * 256;
-       count += *current++;
+       unsigned char *current = slab + reservelen;
+       uint16_t count = get_uint16(current);
+
        return (count);
 }
 
@@ -458,11 +455,8 @@ rdata_from_slab(unsigned char **current, dns_rdataclass_t rdclass,
                dns_rdatatype_t type, dns_rdata_t *rdata) {
        unsigned char *tcurrent = *current;
        isc_region_t region;
-       unsigned int length;
        bool offline = false;
-
-       length = *tcurrent++ * 256;
-       length += *tcurrent++;
+       uint16_t length = get_uint16(tcurrent);
 
        if (type == dns_rdatatype_rrsig) {
                if ((*tcurrent & DNS_RDATASLAB_OFFLINE) != 0) {
@@ -493,23 +487,19 @@ static bool
 rdata_in_slab(unsigned char *slab, unsigned int reservelen,
              dns_rdataclass_t rdclass, dns_rdatatype_t type,
              dns_rdata_t *rdata) {
-       unsigned int count, i;
-       unsigned char *current = NULL;
-       dns_rdata_t trdata = DNS_RDATA_INIT;
-       int n;
+       unsigned char *current = slab + reservelen;
 
-       current = slab + reservelen;
-       count = *current++ * 256;
-       count += *current++;
+       uint16_t count = get_uint16(current);
 
 #if DNS_RDATASET_FIXED
        current += (4 * count);
 #endif /* if DNS_RDATASET_FIXED */
 
-       for (i = 0; i < count; i++) {
+       for (size_t i = 0; i < count; i++) {
+               dns_rdata_t trdata = DNS_RDATA_INIT;
                rdata_from_slab(&current, rdclass, type, &trdata);
 
-               n = dns_rdata_compare(&trdata, rdata);
+               int n = dns_rdata_compare(&trdata, rdata);
                if (n == 0) {
                        return (true);
                }
@@ -552,15 +542,13 @@ dns_rdataslab_merge(unsigned char *oslab, unsigned char *nslab,
        REQUIRE(oslab != NULL && nslab != NULL);
 
        ocurrent = oslab + reservelen;
-       ocount = *ocurrent++ * 256;
-       ocount += *ocurrent++;
+       ocount = get_uint16(ocurrent);
 #if DNS_RDATASET_FIXED
        ocurrent += (4 * ocount);
 #endif /* if DNS_RDATASET_FIXED */
        ostart = ocurrent;
        ncurrent = nslab + reservelen;
-       ncount = *ncurrent++ * 256;
-       ncount += *ncurrent++;
+       ncount = get_uint16(ncurrent);
 #if DNS_RDATASET_FIXED
        ncurrent += (4 * ncount);
 #endif /* if DNS_RDATASET_FIXED */
@@ -579,8 +567,7 @@ dns_rdataslab_merge(unsigned char *oslab, unsigned char *nslab,
         */
        olength = 0;
        for (count = 0; count < ocount; count++) {
-               length = *ocurrent++ * 256;
-               length += *ocurrent++;
+               length = get_uint16(ocurrent);
 #if DNS_RDATASET_FIXED
                olength += length + 8;
                ocurrent += length + 2;
@@ -679,7 +666,7 @@ dns_rdataslab_merge(unsigned char *oslab, unsigned char *nslab,
        ocurrent = ostart;
        INSIST(ocount != 0);
 #if DNS_RDATASET_FIXED
-       oorder = ocurrent[2] * 256 + ocurrent[3];
+       oorder = peek_uint16(&ocurrent[2]);
        INSIST(oorder < ocount);
 #endif /* if DNS_RDATASET_FIXED */
        rdata_from_slab(&ocurrent, rdclass, type, &ordata);
@@ -693,7 +680,7 @@ dns_rdataslab_merge(unsigned char *oslab, unsigned char *nslab,
                do {
                        dns_rdata_reset(&nrdata);
 #if DNS_RDATASET_FIXED
-                       norder = ncurrent[2] * 256 + ncurrent[3];
+                       norder = peek_uint16(&ncurrent[2]);
 
                        INSIST(norder < oncount);
 #endif /* if DNS_RDATASET_FIXED */
@@ -732,7 +719,7 @@ dns_rdataslab_merge(unsigned char *oslab, unsigned char *nslab,
                        if (oadded < ocount) {
                                dns_rdata_reset(&ordata);
 #if DNS_RDATASET_FIXED
-                               oorder = ocurrent[2] * 256 + ocurrent[3];
+                               oorder = peek_uint16(&ocurrent[2]);
                                INSIST(oorder < ocount);
 #endif /* if DNS_RDATASET_FIXED */
                                rdata_from_slab(&ocurrent, rdclass, type,
@@ -760,8 +747,7 @@ dns_rdataslab_merge(unsigned char *oslab, unsigned char *nslab,
                                do {
                                        dns_rdata_reset(&nrdata);
 #if DNS_RDATASET_FIXED
-                                       norder = ncurrent[2] * 256 +
-                                                ncurrent[3];
+                                       norder = peek_uint16(&ncurrent[2]);
                                        INSIST(norder < oncount);
 #endif /* if DNS_RDATASET_FIXED */
                                        rdata_from_slab(&ncurrent, rdclass,
@@ -806,11 +792,9 @@ dns_rdataslab_subtract(unsigned char *mslab, unsigned char *sslab,
        REQUIRE(mslab != NULL && sslab != NULL);
 
        mcurrent = mslab + reservelen;
-       mcount = *mcurrent++ * 256;
-       mcount += *mcurrent++;
+       mcount = get_uint16(mcurrent);
        scurrent = sslab + reservelen;
-       scount = *scurrent++ * 256;
-       scount += *scurrent++;
+       scount = get_uint16(scurrent);
        INSIST(mcount > 0 && scount > 0);
 
        /*
@@ -910,15 +894,14 @@ dns_rdataslab_subtract(unsigned char *mslab, unsigned char *sslab,
         * Copy the parts of mslab not in sslab.
         */
        mcurrent = mslab + reservelen;
-       mcount = *mcurrent++ * 256;
-       mcount += *mcurrent++;
+       mcount = get_uint16(mcurrent);
 #if DNS_RDATASET_FIXED
        mcurrent += (4 * mcount);
 #endif /* if DNS_RDATASET_FIXED */
        for (i = 0; i < mcount; i++) {
                unsigned char *mrdatabegin = mcurrent;
 #if DNS_RDATASET_FIXED
-               order = mcurrent[2] * 256 + mcurrent[3];
+               order = peek_uint16(&mcurrent[2]);
                INSIST(order < mcount);
 #endif /* if DNS_RDATASET_FIXED */
                rdata_from_slab(&mcurrent, rdclass, type, &mrdata);
@@ -967,12 +950,10 @@ dns_rdataslab_equal(unsigned char *slab1, unsigned char *slab2,
        unsigned int length1, length2;
 
        current1 = slab1 + reservelen;
-       count1 = *current1++ * 256;
-       count1 += *current1++;
+       count1 = get_uint16(current1);
 
        current2 = slab2 + reservelen;
-       count2 = *current2++ * 256;
-       count2 += *current2++;
+       count2 = get_uint16(current2);
 
        if (count1 != count2) {
                return (false);
@@ -983,12 +964,9 @@ dns_rdataslab_equal(unsigned char *slab1, unsigned char *slab2,
        current2 += (4 * count2);
 #endif /* if DNS_RDATASET_FIXED */
 
-       while (count1 > 0) {
-               length1 = *current1++ * 256;
-               length1 += *current1++;
-
-               length2 = *current2++ * 256;
-               length2 += *current2++;
+       while (count1-- > 0) {
+               length1 = get_uint16(current1);
+               length2 = get_uint16(current2);
 
 #if DNS_RDATASET_FIXED
                current1 += 2;
@@ -1003,8 +981,6 @@ dns_rdataslab_equal(unsigned char *slab1, unsigned char *slab2,
 
                current1 += length1;
                current2 += length1;
-
-               count1--;
        }
        return (true);
 }
@@ -1019,12 +995,10 @@ dns_rdataslab_equalx(unsigned char *slab1, unsigned char *slab2,
        dns_rdata_t rdata2 = DNS_RDATA_INIT;
 
        current1 = slab1 + reservelen;
-       count1 = *current1++ * 256;
-       count1 += *current1++;
+       count1 = get_uint16(current1);
 
        current2 = slab2 + reservelen;
-       count2 = *current2++ * 256;
-       count2 += *current2++;
+       count2 = get_uint16(current2);
 
        if (count1 != count2) {
                return (false);
@@ -1200,11 +1174,8 @@ rdataset_disassociate(dns_rdataset_t *rdataset DNS__DB_FLARG) {
 
 static isc_result_t
 rdataset_first(dns_rdataset_t *rdataset) {
-       unsigned char *raw = NULL;
-       unsigned int count;
-
-       raw = rdataset->slab.raw;
-       count = raw[0] * 256 + raw[1];
+       unsigned char *raw = rdataset->slab.raw;
+       uint16_t count = peek_uint16(raw);
        if (count == 0) {
                rdataset->slab.iter_pos = NULL;
                rdataset->slab.iter_count = 0;
@@ -1232,11 +1203,7 @@ rdataset_first(dns_rdataset_t *rdataset) {
 
 static isc_result_t
 rdataset_next(dns_rdataset_t *rdataset) {
-       unsigned int count;
-       unsigned int length;
-       unsigned char *raw = NULL;
-
-       count = rdataset->slab.iter_count;
+       uint16_t count = rdataset->slab.iter_count;
        if (count == 0) {
                rdataset->slab.iter_pos = NULL;
                return (ISC_R_NOMORE);
@@ -1246,12 +1213,12 @@ rdataset_next(dns_rdataset_t *rdataset) {
        /*
         * Skip forward one record (length + 4) or one offset (4).
         */
-       raw = rdataset->slab.iter_pos;
+       unsigned char *raw = rdataset->slab.iter_pos;
 #if DNS_RDATASET_FIXED
        if ((rdataset->attributes & DNS_RDATASETATTR_LOADORDER) == 0)
 #endif /* DNS_RDATASET_FIXED */
        {
-               length = raw[0] * 256 + raw[1];
+               uint16_t length = peek_uint16(raw);
                raw += length;
        }
        rdataset->slab.iter_pos = raw + DNS_RDATASET_ORDER +
@@ -1284,7 +1251,7 @@ rdataset_current(dns_rdataset_t *rdataset, dns_rdata_t *rdata) {
        }
 #endif /* if DNS_RDATASET_FIXED */
 
-       length = raw[0] * 256 + raw[1];
+       length = peek_uint16(raw);
 
        raw += DNS_RDATASET_ORDER + DNS_RDATASET_LENGTH;
 
@@ -1322,7 +1289,7 @@ rdataset_count(dns_rdataset_t *rdataset) {
        unsigned int count;
 
        raw = rdataset->slab.raw;
-       count = raw[0] * 256 + raw[1];
+       count = get_uint16(raw);
 
        return (count);
 }