Remove qpzone.c:first_existing_header_indirect() which had bugs

The first_existing_header_indirect() static function is used only
in one place and it has bugs:

1. It doesn't advance the pointer and can cause an infinite loop
   if it doesn't break out from the loop on the first iteration.
2. It doesn't check if the header EXISTS, though its name indicates
   that it should.
3. Even if the infinite loop bug is fixed, it would eventually
   return the last checked header's pointer even if all the
   candidates do not match the criteria of the selection.

Instead of fixing it, remove the function and use simpler code in
the place where it was being called.

diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c
index e6d3618a9617c9f54107faac40d493528b484a3b..b01d84b17b765e775fa08fd8ae46ca9109ec5afd 100644 (file)
--- a/lib/dns/qpzone.c
+++ b/lib/dns/qpzone.c
@@ -820,22 +820,6 @@ first_existing_header(dns_vectop_t *top, uint32_t serial) {
        return NULL;
 }
 
-static dns_vecheader_t **
-first_existing_header_indirect(dns_vectop_t *top, uint32_t serial) {
-       REQUIRE(top != NULL);
-       dns_vecheader_t **result = &ISC_SLIST_HEAD(top->headers);
-
-       ISC_SLIST_FOREACH_PTR(p, &ISC_SLIST_HEAD(top->headers)) {
-               dns_vecheader_t *header = *p;
-               result = p;
-
-               if (header->serial <= serial && !IGNORE(header)) {
-                       break;
-               }
-       }
-       return result;
-}
-
 static void
 clean_multiple_headers(dns_vectop_t *top) {
        uint32_t parent_serial = UINT32_MAX;
@@ -1814,9 +1798,14 @@ add(qpzonedb_t *qpdb, qpznode_t *node, const dns_name_t *nodename,
        dns_vecheader_t **header_p = NULL;
        dns_vecheader_t *header = NULL;
        if (foundtop != NULL) {
-               header_p = first_existing_header_indirect(foundtop, UINT32_MAX);
-               INSIST(header_p != NULL);
-               header = EXISTS(*header_p) ? *header_p : NULL;
+               ISC_SLIST_FOREACH_PTR(p, &ISC_SLIST_HEAD(foundtop->headers)) {
+                       if (!IGNORE(*p)) {
+                               header_p = p;
+                               header = *p;
+                               break;
+                       }
+                       ISC_SLIST_PTR_ADVANCE(p, next_header);
+               }
        }
 
        if (header != NULL) {
@@ -1901,7 +1890,7 @@ add(qpzonedb_t *qpdb, qpznode_t *node, const dns_name_t *nodename,
                         * Since we don't generate changed records when
                         * loading, we MUST clean up 'header' now.
                         */
-                       *header_p = ISC_SLIST_NEXT(header, next_header);
+                       ISC_SLIST_PTR_REMOVE(header_p, header, next_header);
                        ISC_SLIST_PREPEND(foundtop->headers, newheader,
                                          next_header);
                        maybe_update_recordsandsize(false, version, header,