Remove redundant function 'newchain'

The removed function 'newchain(a, b)' was almost the same as calling
!chain_equal(a, b), varying only in the amount of data compared
in the non-fixed-length data portion of given chain nodes.

A third argument 'data_size' has been introduced into 'chain_equal'
function in order to allow it to know how many bytes to compare in the
variable-length data portion of the chain nodes.

A helper function 'chain_length(e)' has been introduced to allow
easy calculation of the total length of the non-fixed-length data part
of chain nodes.

Check the thread below for more details:
https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/291#note_12184

diff --git a/lib/dns/zoneverify.c b/lib/dns/zoneverify.c
index 335f9ec51a7c989acb70ced50e1ae8ba313b621d..7ab2b51002600f6719b67f436b748a2355a10c70 100644 (file)
--- a/lib/dns/zoneverify.c
+++ b/lib/dns/zoneverify.c
@@ -95,6 +95,15 @@ struct nsec3_chain_fixed {
         */
 };
 
+/*
+ * Helper function used to calculate length of variable-length
+ * data section in object pointed to by 'chain'.
+ */
+static inline size_t
+chain_length(struct nsec3_chain_fixed *chain) {
+       return (chain->salt_length + 2 * chain->next_length);
+}
+
 /*%
  * Log a zone verification error described by 'fmt' and the variable arguments
  * following it.  Either use dns_zone_logv() or print to stderr, depending on
@@ -333,8 +342,6 @@ check_no_rrsig(const vctx_t *vctx, const dns_rdataset_t *rdataset,
 static bool
 chain_compare(void *arg1, void *arg2) {
        struct nsec3_chain_fixed *e1 = arg1, *e2 = arg2;
-       size_t len;
-
        /*
         * Do each element in turn to get a stable sort.
         */
@@ -362,8 +369,7 @@ chain_compare(void *arg1, void *arg2) {
        if (e1->next_length > e2->next_length) {
                return (false);
        }
-       len = e1->salt_length + 2 * e1->next_length;
-       if (memcmp(e1 + 1, e2 + 1, len) < 0) {
+       if (memcmp(e1 + 1, e2 + 1, chain_length(e1)) < 0) {
                return (true);
        }
        return (false);
@@ -371,9 +377,7 @@ chain_compare(void *arg1, void *arg2) {
 
 static bool
 chain_equal(const struct nsec3_chain_fixed *e1,
-           const struct nsec3_chain_fixed *e2) {
-       size_t len;
-
+           const struct nsec3_chain_fixed *e2, size_t data_length) {
        if (e1->hash != e2->hash) {
                return (false);
        }
@@ -386,11 +390,8 @@ chain_equal(const struct nsec3_chain_fixed *e1,
        if (e1->next_length != e2->next_length) {
                return (false);
        }
-       len = e1->salt_length + 2 * e1->next_length;
-       if (memcmp(e1 + 1, e2 + 1, len) != 0) {
-               return (false);
-       }
-       return (true);
+
+       return (memcmp(e1 + 1, e2 + 1, data_length) == 0);
 }
 
 static isc_result_t
@@ -1062,19 +1063,6 @@ check_no_nsec(const vctx_t *vctx, const dns_name_t *name, dns_dbnode_t *node) {
        return (nsec_exists ? ISC_R_FAILURE : ISC_R_SUCCESS);
 }
 
-static bool
-newchain(const struct nsec3_chain_fixed *first,
-        const struct nsec3_chain_fixed *e) {
-       if (first->hash != e->hash || first->iterations != e->iterations ||
-           first->salt_length != e->salt_length ||
-           first->next_length != e->next_length ||
-           memcmp(first + 1, e + 1, first->salt_length) != 0)
-       {
-               return (true);
-       }
-       return (false);
-}
-
 static void
 free_element(isc_mem_t *mctx, struct nsec3_chain_fixed *e) {
        size_t len;
@@ -1173,7 +1161,7 @@ verify_nsec3_chains(const vctx_t *vctx, isc_mem_t *mctx) {
                        /*
                         * Check that they match.
                         */
-                       if (chain_equal(e, f)) {
+                       if (chain_equal(e, f, chain_length(e))) {
                                free_element(mctx, f);
                                f = NULL;
                        } else {
@@ -1196,7 +1184,9 @@ verify_nsec3_chains(const vctx_t *vctx, isc_mem_t *mctx) {
                                                isc_heap_delete(
                                                        vctx->found_chains, 1);
                                        }
-                                       if (f != NULL && chain_equal(e, f)) {
+                                       if (f != NULL &&
+                                           chain_equal(e, f, chain_length(e)))
+                                       {
                                                free_element(mctx, f);
                                                f = NULL;
                                                break;
@@ -1212,7 +1202,7 @@ verify_nsec3_chains(const vctx_t *vctx, isc_mem_t *mctx) {
 
                if (first == NULL) {
                        prev = first = e;
-               } else if (newchain(first, e)) {
+               } else if (!chain_equal(first, e, first->salt_length)) {
                        if (!checklast(mctx, vctx, first, prev)) {
                                result = ISC_R_FAILURE;
                        }