Code cleanups (#41656)

diff --git a/bin/named/query.c b/bin/named/query.c
index 3b0ef962e0c6fad654c3a41117d8c488ef1fa4c2..fd6d65cef6b0d2650def836b1712ab1e31eba7f3 100644 (file)
--- a/bin/named/query.c
+++ b/bin/named/query.c
@@ -517,7 +517,7 @@ query_getnamebuf(ns_client_t *client) {
        dbuf = ISC_LIST_TAIL(client->query.namebufs);
        INSIST(dbuf != NULL);
        isc_buffer_availableregion(dbuf, &r);
-       if (r.length < 255) {
+       if (r.length < DNS_NAME_MAXWIRE) {
                result = query_newnamebuf(client);
                if (result != ISC_R_SUCCESS) {
                    CTRACE(ISC_LOG_DEBUG(3),
@@ -1184,12 +1184,9 @@ query_getdb(ns_client_t *client, dns_name_t *name, dns_rdatatype_t qtype,
        unsigned int namelabels;
        unsigned int zonelabels;
        dns_zone_t *zone = NULL;
-       dns_db_t *tdbp;
 
        REQUIRE(zonep != NULL && *zonep == NULL);
 
-       tdbp = NULL;
-
        /* Calculate how many labels are in name. */
        namelabels = dns_name_countlabels(name);
        zonelabels = 0;
@@ -1206,15 +1203,17 @@ query_getdb(ns_client_t *client, dns_name_t *name, dns_rdatatype_t qtype,
         * If # zone labels < # name labels, try to find an even better match
         * Only try if DLZ drivers are loaded for this view
         */
-       if (zonelabels < namelabels &&
-           !ISC_LIST_EMPTY(client->view->dlz_searched))
+       if (ISC_UNLIKELY(zonelabels < namelabels &&
+                        !ISC_LIST_EMPTY(client->view->dlz_searched)))
        {
                dns_clientinfomethods_t cm;
                dns_clientinfo_t ci;
+               dns_db_t *tdbp;
 
                dns_clientinfomethods_init(&cm, ns_client_sourceip);
                dns_clientinfo_init(&ci, client, NULL);
 
+               tdbp = NULL;
                tresult = dns_view_searchdlz(client->view, name,
                                             zonelabels, &cm, &ci, &tdbp);
                 /* If we successful, we found a better match. */
@@ -6800,11 +6799,14 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                options |= DNS_GETDB_NOEXACT;
        result = query_getdb(client, client->query.qname, qtype, options,
                             &zone, &db, &version, &is_zone);
-       if ((result != ISC_R_SUCCESS || !is_zone) && !RECURSIONOK(client) &&
-           (options & DNS_GETDB_NOEXACT) != 0 && qtype == dns_rdatatype_ds) {
+       if (ISC_UNLIKELY((result != ISC_R_SUCCESS || !is_zone) &&
+                        qtype == dns_rdatatype_ds &&
+                        !RECURSIONOK(client) &&
+                        (options & DNS_GETDB_NOEXACT) != 0))
+       {
                /*
-                * Look to see if we are authoritative for the
-                * child zone if the query type is DS.
+                * If the query type is DS, look to see if we are
+                * authoritative for the child zone.
                 */
                dns_db_t *tdb = NULL;
                dns_zone_t *tzone = NULL;
@@ -6883,7 +6885,7 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
         * We'll need some resources...
         */
        dbuf = query_getnamebuf(client);
-       if (dbuf == NULL) {
+       if (ISC_UNLIKELY(dbuf == NULL)) {
                CTRACE(ISC_LOG_ERROR,
                       "query_find: query_getnamebuf failed (2)");
                QUERY_ERROR(DNS_R_SERVFAIL);
@@ -6891,7 +6893,7 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
        }
        fname = query_newname(client, dbuf, &b);
        rdataset = query_newrdataset(client);
-       if (fname == NULL || rdataset == NULL) {
+       if (ISC_UNLIKELY(fname == NULL || rdataset == NULL)) {
                CTRACE(ISC_LOG_ERROR,
                       "query_find: query_newname failed (2)");
                QUERY_ERROR(DNS_R_SERVFAIL);
@@ -6914,7 +6916,7 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                                client->query.dboptions, client->now,
                                &node, fname, &cm, &ci, rdataset, sigrdataset);
 
-       if (db == client->view->cachedb)
+       if (!is_zone)
                dns_cache_updatestats(client->view->cache, result);
 
  resume:
@@ -8958,6 +8960,14 @@ ns_query_start(ns_client_t *client) {
                client->attributes |= NS_CLIENTATTR_NOSETFC;
        }
 
+       /*
+        * Check for multiple question queries, since edns1 is dead.
+        */
+       if (message->counts[DNS_SECTION_QUESTION] > 1) {
+               query_error(client, DNS_R_FORMERR, __LINE__);
+               return;
+       }
+
        /*
         * Get the question name.
         */
@@ -8985,14 +8995,6 @@ ns_query_start(ns_client_t *client) {
        if (ns_g_server->log_queries)
                log_query(client, saved_flags, saved_extflags);
 
-       /*
-        * Check for multiple question queries, since edns1 is dead.
-        */
-       if (message->counts[DNS_SECTION_QUESTION] > 1) {
-               query_error(client, DNS_R_FORMERR, __LINE__);
-               return;
-       }
-
        /*
         * Check for meta-queries like IXFR and AXFR.
         */

diff --git a/configure b/configure
index f565d64e8582ce271e77a76930eecc21f287d58a..43e2389b35c571f57f3e1f9924409e7d8f588d15 100755 (executable)
--- a/configure
+++ b/configure
@@ -19662,8 +19662,6 @@ ISC_PLATFORM_USEGCCASM="#undef ISC_PLATFORM_USEGCCASM"
 ISC_PLATFORM_USESTDASM="#undef ISC_PLATFORM_USESTDASM"
 ISC_PLATFORM_USEMACASM="#undef ISC_PLATFORM_USEMACASM"
 if test "$use_atomic" = "yes"; then
-       { $as_echo "$as_me:${as_lineno-$LINENO}: checking architecture type for atomic operations" >&5
-$as_echo_n "checking architecture type for atomic operations... " >&6; }
        have_atomic=yes         # set default
        case "$host" in
        i[3456]86-*)
@@ -19767,6 +19765,8 @@ _ACEOF
                arch=noatomic
        ;;
        esac
+       { $as_echo "$as_me:${as_lineno-$LINENO}: checking architecture type for atomic operations" >&5
+$as_echo_n "checking architecture type for atomic operations... " >&6; }
        { $as_echo "$as_me:${as_lineno-$LINENO}: result: $arch" >&5
 $as_echo "$arch" >&6; }
 fi
@@ -21265,6 +21265,8 @@ else
 fi
 
 
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to enable query trace logging" >&5
+$as_echo_n "checking whether to enable query trace logging... " >&6; }
 case "$want_querytrace" in
 yes)
        { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5

diff --git a/configure.in b/configure.in
index 5acc87e11a7fe6e96f7a40754683fea0b390260a..4d5199ef752e94ea58bba7981e03b8385f75fcdd 100644 (file)
--- a/configure.in
+++ b/configure.in
@@ -3745,7 +3745,6 @@ ISC_PLATFORM_USEGCCASM="#undef ISC_PLATFORM_USEGCCASM"
 ISC_PLATFORM_USESTDASM="#undef ISC_PLATFORM_USESTDASM"
 ISC_PLATFORM_USEMACASM="#undef ISC_PLATFORM_USEMACASM"
 if test "$use_atomic" = "yes"; then
-       AC_MSG_CHECKING([architecture type for atomic operations])
        have_atomic=yes         # set default
        case "$host" in
        [i[3456]86-*])
@@ -3785,6 +3784,7 @@ if test "$use_atomic" = "yes"; then
                arch=noatomic
        ;;
        esac
+       AC_MSG_CHECKING([architecture type for atomic operations])
        AC_MSG_RESULT($arch)
 fi
 
@@ -4358,6 +4358,7 @@ AC_ARG_ENABLE(querytrace,
        [  --enable-querytrace     enable very verbose query trace logging [[default=no]]],
        want_querytrace="$enableval", want_querytrace="no")
 
+AC_MSG_CHECKING([whether to enable query trace logging])
 case "$want_querytrace" in
 yes)
        AC_MSG_RESULT(yes)

diff --git a/lib/dns/badcache.c b/lib/dns/badcache.c
index 3e6d22c4d8c8f3573ab9ee09e353bdf3bc4d9546..8751ddb194ddc16c2287f943c0f77a4caa5bc790 100644 (file)
--- a/lib/dns/badcache.c
+++ b/lib/dns/badcache.c
@@ -242,6 +242,18 @@ dns_badcache_find(dns_badcache_t *bc, dns_name_t *name,
 
        LOCK(&bc->lock);
 
+       /*
+        * XXXMUKS: dns_name_equal() is expensive as it does a
+        * octet-by-octet comparison, and it can be made better in two
+        * ways here. First, lowercase the names (use
+        * dns_name_downcase() instead of dns_name_copy() in
+        * dns_badcache_add()) so that dns_name_caseequal() can be used
+        * which the compiler will emit as SIMD instructions. Second,
+        * don't put multiple copies of the same name in the chain (or
+        * multiple names will have to be matched for equality), but use
+        * name->link to store the type specific part.
+        */
+
        if (bc->count == 0)
                goto skip;
 

diff --git a/lib/dns/message.c b/lib/dns/message.c
index 7a7aa7b921f59e5fec53ccbccd7640858ac4ee67..e5e90e444cae86698cf6fd7f7817f412a32c7072 100644 (file)
--- a/lib/dns/message.c
+++ b/lib/dns/message.c
@@ -840,9 +840,8 @@ dns_message_find(dns_name_t *name, dns_rdataclass_t rdclass,
 {
        dns_rdataset_t *curr;
 
-       if (rdataset != NULL) {
-               REQUIRE(*rdataset == NULL);
-       }
+       REQUIRE(name != NULL);
+       REQUIRE(rdataset == NULL || *rdataset == NULL);
 
        for (curr = ISC_LIST_TAIL(name->list);
             curr != NULL;
@@ -865,15 +864,13 @@ dns_message_findtype(dns_name_t *name, dns_rdatatype_t type,
        dns_rdataset_t *curr;
 
        REQUIRE(name != NULL);
-       if (rdataset != NULL) {
-               REQUIRE(*rdataset == NULL);
-       }
+       REQUIRE(rdataset == NULL || *rdataset == NULL);
 
        for (curr = ISC_LIST_TAIL(name->list);
             curr != NULL;
             curr = ISC_LIST_PREV(curr, link)) {
                if (curr->type == type && curr->covers == covers) {
-                       if (rdataset != NULL)
+                       if (ISC_UNLIKELY(rdataset != NULL))
                                *rdataset = curr;
                        return (ISC_R_SUCCESS);
                }
@@ -2348,13 +2345,12 @@ dns_message_findname(dns_message_t *msg, dns_section_t section,
        REQUIRE(msg != NULL);
        REQUIRE(VALID_SECTION(section));
        REQUIRE(target != NULL);
-       if (name != NULL)
-               REQUIRE(*name == NULL);
+       REQUIRE(name == NULL || *name == NULL);
+
        if (type == dns_rdatatype_any) {
                REQUIRE(rdataset == NULL);
        } else {
-               if (rdataset != NULL)
-                       REQUIRE(*rdataset == NULL);
+               REQUIRE(rdataset == NULL || *rdataset == NULL);
        }
 
        result = findname(&foundname, target,
@@ -2371,7 +2367,7 @@ dns_message_findname(dns_message_t *msg, dns_section_t section,
        /*
         * And now look for the type.
         */
-       if (type == dns_rdatatype_any)
+       if (ISC_UNLIKELY(type == dns_rdatatype_any))
                return (ISC_R_SUCCESS);
 
        result = dns_message_findtype(foundname, type, covers, rdataset);

diff --git a/lib/isc/hash.c b/lib/isc/hash.c
index da8c4487d30420abed00539545941c881b5604aa..85cf6c38c4e06b9886fa29f6613c6d727e9e5775 100644 (file)
--- a/lib/isc/hash.c
+++ b/lib/isc/hash.c
@@ -404,7 +404,7 @@ isc__hash_setvec(const isc_uint16_t *vec) {
        }
 }
 
-static unsigned int fnv_offset_basis;
+static isc_uint32_t fnv_offset_basis;
 static isc_once_t fnv_once = ISC_ONCE_INIT;
 
 static void
@@ -439,19 +439,20 @@ isc_hash_set_initializer(const void *initializer) {
        fnv_offset_basis = *((const unsigned int *) initializer);
 }
 
-unsigned int
+isc_uint32_t
 isc_hash_function(const void *data, size_t length,
                  isc_boolean_t case_sensitive,
-                 unsigned int *previous_hashp)
+                 const isc_uint32_t *previous_hashp)
 {
-       unsigned int hval;
+       isc_uint32_t hval;
        const unsigned char *bp;
        const unsigned char *be;
 
        INSIST(data == NULL || length > 0);
        RUNTIME_CHECK(isc_once_do(&fnv_once, fnv_initialize) == ISC_R_SUCCESS);
 
-       hval = previous_hashp != NULL ? *previous_hashp : fnv_offset_basis;
+       hval = ISC_UNLIKELY(previous_hashp != NULL) ?
+               *previous_hashp : fnv_offset_basis;
 
        if (length == 0)
                return (hval);
@@ -470,34 +471,34 @@ isc_hash_function(const void *data, size_t length,
 
        if (case_sensitive) {
                while (bp < be - 4) {
-                       hval ^= (unsigned int) bp[0];
+                       hval ^= (isc_uint32_t) bp[0];
                        hval *= 16777619;
-                       hval ^= (unsigned int) bp[1];
+                       hval ^= (isc_uint32_t) bp[1];
                        hval *= 16777619;
-                       hval ^= (unsigned int) bp[2];
+                       hval ^= (isc_uint32_t) bp[2];
                        hval *= 16777619;
-                       hval ^= (unsigned int) bp[3];
+                       hval ^= (isc_uint32_t) bp[3];
                        hval *= 16777619;
                        bp += 4;
                }
                while (bp < be) {
-                       hval ^= (unsigned int) *bp++;
+                       hval ^= (isc_uint32_t) *bp++;
                        hval *= 16777619;
                }
        } else {
                while (bp < be - 4) {
-                       hval ^= (unsigned int) maptolower[bp[0]];
+                       hval ^= (isc_uint32_t) maptolower[bp[0]];
                        hval *= 16777619;
-                       hval ^= (unsigned int) maptolower[bp[1]];
+                       hval ^= (isc_uint32_t) maptolower[bp[1]];
                        hval *= 16777619;
-                       hval ^= (unsigned int) maptolower[bp[2]];
+                       hval ^= (isc_uint32_t) maptolower[bp[2]];
                        hval *= 16777619;
-                       hval ^= (unsigned int) maptolower[bp[3]];
+                       hval ^= (isc_uint32_t) maptolower[bp[3]];
                        hval *= 16777619;
                        bp += 4;
                }
                while (bp < be) {
-                       hval ^= (unsigned int) maptolower[*bp++];
+                       hval ^= (isc_uint32_t) maptolower[*bp++];
                        hval *= 16777619;
                }
        }
@@ -505,19 +506,20 @@ isc_hash_function(const void *data, size_t length,
        return (hval);
 }
 
-unsigned int
+isc_uint32_t
 isc_hash_function_reverse(const void *data, size_t length,
                          isc_boolean_t case_sensitive,
-                         unsigned int *previous_hashp)
+                         const isc_uint32_t *previous_hashp)
 {
-       unsigned int hval;
+       isc_uint32_t hval;
        const unsigned char *bp;
        const unsigned char *be;
 
        INSIST(data == NULL || length > 0);
        RUNTIME_CHECK(isc_once_do(&fnv_once, fnv_initialize) == ISC_R_SUCCESS);
 
-       hval = previous_hashp != NULL ? *previous_hashp : fnv_offset_basis;
+       hval = ISC_UNLIKELY(previous_hashp != NULL) ?
+               *previous_hashp : fnv_offset_basis;
 
        if (length == 0)
                return (hval);
@@ -537,33 +539,33 @@ isc_hash_function_reverse(const void *data, size_t length,
        if (case_sensitive) {
                while (be >= bp + 4) {
                        be -= 4;
-                       hval ^= (unsigned int) be[3];
+                       hval ^= (isc_uint32_t) be[3];
                        hval *= 16777619;
-                       hval ^= (unsigned int) be[2];
+                       hval ^= (isc_uint32_t) be[2];
                        hval *= 16777619;
-                       hval ^= (unsigned int) be[1];
+                       hval ^= (isc_uint32_t) be[1];
                        hval *= 16777619;
-                       hval ^= (unsigned int) be[0];
+                       hval ^= (isc_uint32_t) be[0];
                        hval *= 16777619;
                }
                while (--be >= bp) {
-                       hval ^= (unsigned int) *be;
+                       hval ^= (isc_uint32_t) *be;
                        hval *= 16777619;
                }
        } else {
                while (be >= bp + 4) {
                        be -= 4;
-                       hval ^= (unsigned int) maptolower[be[3]];
+                       hval ^= (isc_uint32_t) maptolower[be[3]];
                        hval *= 16777619;
-                       hval ^= (unsigned int) maptolower[be[2]];
+                       hval ^= (isc_uint32_t) maptolower[be[2]];
                        hval *= 16777619;
-                       hval ^= (unsigned int) maptolower[be[1]];
+                       hval ^= (isc_uint32_t) maptolower[be[1]];
                        hval *= 16777619;
-                       hval ^= (unsigned int) maptolower[be[0]];
+                       hval ^= (isc_uint32_t) maptolower[be[0]];
                        hval *= 16777619;
                }
                while (--be >= bp) {
-                       hval ^= (unsigned int) maptolower[*be];
+                       hval ^= (isc_uint32_t) maptolower[*be];
                        hval *= 16777619;
                }
        }

diff --git a/lib/isc/include/isc/hash.h b/lib/isc/include/isc/hash.h
index 5334c655e71ef2f07dab10e72f50ed828419a987..571e74fd5c3460257a5c96f6396ca6c51c3711e4 100644 (file)
--- a/lib/isc/include/isc/hash.h
+++ b/lib/isc/include/isc/hash.h
@@ -204,14 +204,14 @@ isc_hash_get_initializer(void);
 void
 isc_hash_set_initializer(const void *initializer);
 
-unsigned int
+isc_uint32_t
 isc_hash_function(const void *data, size_t length,
                  isc_boolean_t case_sensitive,
-                 unsigned int *previous_hashp);
-unsigned int
+                 const isc_uint32_t *previous_hashp);
+isc_uint32_t
 isc_hash_function_reverse(const void *data, size_t length,
                          isc_boolean_t case_sensitive,
-                         unsigned int *previous_hashp);
+                         const isc_uint32_t *previous_hashp);
 /*!<
  * \brief Calculate a hash over data.
  *

diff --git a/lib/isc/mem.c b/lib/isc/mem.c
index 6c49d4b38f65fada4540ae982a204b1fce854b59..743b3d3517751f016b03038b8098c68a83c914cf 100644 (file)
--- a/lib/isc/mem.c
+++ b/lib/isc/mem.c
@@ -1995,53 +1995,43 @@ isc___mempool_get(isc_mempool_t *mpctx0 FLARG) {
        /*
         * Don't let the caller go over quota
         */
-       if (mpctx->allocated >= mpctx->maxalloc) {
+       if (ISC_UNLIKELY(mpctx->allocated >= mpctx->maxalloc)) {
                item = NULL;
                goto out;
        }
 
-       /*
-        * if we have a free list item, return the first here
-        */
-       item = mpctx->items;
-       if (item != NULL) {
-               mpctx->items = item->next;
-               INSIST(mpctx->freecount > 0);
-               mpctx->freecount--;
-               mpctx->gets++;
-               mpctx->allocated++;
-               goto out;
-       }
-
-       /*
-        * We need to dip into the well.  Lock the memory context here and
-        * fill up our free list.
-        */
-       MCTXLOCK(mctx, &mctx->lock);
-       for (i = 0; i < mpctx->fillcount; i++) {
-               if ((mctx->flags & ISC_MEMFLAG_INTERNAL) != 0) {
-                       item = mem_getunlocked(mctx, mpctx->size);
-               } else {
-                       item = mem_get(mctx, mpctx->size);
-                       if (item != NULL)
-                               mem_getstats(mctx, mpctx->size);
+       if (ISC_UNLIKELY(mpctx->items == NULL)) {
+               /*
+                * We need to dip into the well.  Lock the memory context here and
+                * fill up our free list.
+                */
+               MCTXLOCK(mctx, &mctx->lock);
+               for (i = 0; i < mpctx->fillcount; i++) {
+                       if ((mctx->flags & ISC_MEMFLAG_INTERNAL) != 0) {
+                               item = mem_getunlocked(mctx, mpctx->size);
+                       } else {
+                               item = mem_get(mctx, mpctx->size);
+                               if (item != NULL)
+                                       mem_getstats(mctx, mpctx->size);
+                       }
+                       if (ISC_UNLIKELY(item == NULL))
+                               break;
+                       item->next = mpctx->items;
+                       mpctx->items = item;
+                       mpctx->freecount++;
                }
-               if (item == NULL)
-                       break;
-               item->next = mpctx->items;
-               mpctx->items = item;
-               mpctx->freecount++;
+               MCTXUNLOCK(mctx, &mctx->lock);
        }
-       MCTXUNLOCK(mctx, &mctx->lock);
 
        /*
         * If we didn't get any items, return NULL.
         */
        item = mpctx->items;
-       if (item == NULL)
+       if (ISC_UNLIKELY(item == NULL))
                goto out;
 
        mpctx->items = item->next;
+       INSIST(mpctx->freecount > 0);
        mpctx->freecount--;
        mpctx->gets++;
        mpctx->allocated++;