Make isc_heap_create() and isc_heap_insert() return void

Previously, the function(s) in the commit subject could fail for various
reasons - mostly allocation failures, or other functions returning
different return code than ISC_R_SUCCESS.  Now, the aforementioned
function(s) cannot ever fail and they would always return ISC_R_SUCCESS.

Change the function(s) to return void and remove the extra checks in
the code that uses them.

(cherry picked from commit bbb4cdb92df6113413610814b83f2e65adfadad9)

diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c
index f99d8a6c126e9f77f3dd3d3994a5c120c8ac87a6..e086510461a6e7d06b19bc57e94a4c8ef7d633ad 100644 (file)
--- a/lib/dns/rbtdb.c
+++ b/lib/dns/rbtdb.c
@@ -603,7 +603,7 @@ expire_header(dns_rbtdb_t *rbtdb, rdatasetheader_t *header, bool tree_locked,
 static void
 overmem_purge(dns_rbtdb_t *rbtdb, unsigned int locknum_start, isc_stdtime_t now,
              bool tree_locked);
-static isc_result_t
+static void
 resign_insert(dns_rbtdb_t *rbtdb, int idx, rdatasetheader_t *newheader);
 static void
 resign_delete(dns_rbtdb_t *rbtdb, rbtdb_version_t *version,
@@ -2693,17 +2693,7 @@ closeversion(dns_db_t *db, dns_dbversion_t **versionp, bool commit) {
                lock = &rbtdb->node_locks[header->node->locknum].lock;
                NODE_LOCK(lock, isc_rwlocktype_write);
                if (rollback && !IGNORE(header)) {
-                       isc_result_t result;
-                       result = resign_insert(rbtdb, header->node->locknum,
-                                              header);
-                       if (result != ISC_R_SUCCESS) {
-                               isc_log_write(dns_lctx,
-                                             DNS_LOGCATEGORY_DATABASE,
-                                             DNS_LOGMODULE_ZONE, ISC_LOG_ERROR,
-                                             "Unable to reinsert header to "
-                                             "re-signing heap: %s",
-                                             dns_result_totext(result));
-                       }
+                       resign_insert(rbtdb, header->node->locknum, header);
                }
                decrement_reference(rbtdb, header->node, least_serial,
                                    isc_rwlocktype_write, isc_rwlocktype_none,
@@ -6076,16 +6066,13 @@ cname_and_other_data(dns_rbtnode_t *node, rbtdb_serial_t serial) {
        return (false);
 }
 
-static isc_result_t
+static void
 resign_insert(dns_rbtdb_t *rbtdb, int idx, rdatasetheader_t *newheader) {
-       isc_result_t result;
-
        INSIST(!IS_CACHE(rbtdb));
        INSIST(newheader->heap_index == 0);
        INSIST(!ISC_LINK_LINKED(newheader, link));
 
-       result = isc_heap_insert(rbtdb->heaps[idx], newheader);
-       return (result);
+       isc_heap_insert(rbtdb->heaps[idx], newheader);
 }
 
 /*
@@ -6508,20 +6495,9 @@ find_header:
                                                         newheader, link);
                                }
                                INSIST(rbtdb->heaps != NULL);
-                               result = isc_heap_insert(rbtdb->heaps[idx],
-                                                        newheader);
-                               if (result != ISC_R_SUCCESS) {
-                                       free_rdataset(rbtdb, rbtdb->common.mctx,
-                                                     newheader);
-                                       return (result);
-                               }
+                               isc_heap_insert(rbtdb->heaps[idx], newheader);
                        } else if (RESIGN(newheader)) {
-                               result = resign_insert(rbtdb, idx, newheader);
-                               if (result != ISC_R_SUCCESS) {
-                                       free_rdataset(rbtdb, rbtdb->common.mctx,
-                                                     newheader);
-                                       return (result);
-                               }
+                               resign_insert(rbtdb, idx, newheader);
                                /*
                                 * Don't call resign_delete as we don't need
                                 * to reverse the delete.  The free_rdataset
@@ -6551,13 +6527,7 @@ find_header:
                        idx = newheader->node->locknum;
                        if (IS_CACHE(rbtdb)) {
                                INSIST(rbtdb->heaps != NULL);
-                               result = isc_heap_insert(rbtdb->heaps[idx],
-                                                        newheader);
-                               if (result != ISC_R_SUCCESS) {
-                                       free_rdataset(rbtdb, rbtdb->common.mctx,
-                                                     newheader);
-                                       return (result);
-                               }
+                               isc_heap_insert(rbtdb->heaps[idx], newheader);
                                if (ZEROTTL(newheader)) {
                                        ISC_LIST_APPEND(rbtdb->rdatasets[idx],
                                                        newheader, link);
@@ -6566,12 +6536,7 @@ find_header:
                                                         newheader, link);
                                }
                        } else if (RESIGN(newheader)) {
-                               result = resign_insert(rbtdb, idx, newheader);
-                               if (result != ISC_R_SUCCESS) {
-                                       free_rdataset(rbtdb, rbtdb->common.mctx,
-                                                     newheader);
-                                       return (result);
-                               }
+                               resign_insert(rbtdb, idx, newheader);
                                resign_delete(rbtdb, rbtversion, header);
                        }
                        if (topheader_prev != NULL) {
@@ -6616,12 +6581,7 @@ find_header:
 
                idx = newheader->node->locknum;
                if (IS_CACHE(rbtdb)) {
-                       result = isc_heap_insert(rbtdb->heaps[idx], newheader);
-                       if (result != ISC_R_SUCCESS) {
-                               free_rdataset(rbtdb, rbtdb->common.mctx,
-                                             newheader);
-                               return (result);
-                       }
+                       isc_heap_insert(rbtdb->heaps[idx], newheader);
                        if (ZEROTTL(newheader)) {
                                ISC_LIST_APPEND(rbtdb->rdatasets[idx],
                                                newheader, link);
@@ -6630,12 +6590,7 @@ find_header:
                                                 newheader, link);
                        }
                } else if (RESIGN(newheader)) {
-                       result = resign_insert(rbtdb, idx, newheader);
-                       if (result != ISC_R_SUCCESS) {
-                               free_rdataset(rbtdb, rbtdb->common.mctx,
-                                             newheader);
-                               return (result);
-                       }
+                       resign_insert(rbtdb, idx, newheader);
                        resign_delete(rbtdb, rbtversion, header);
                }
 
@@ -7162,13 +7117,8 @@ subtractrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
                                                  RDATASET_ATTR_RESIGN);
                                newheader->resign = header->resign;
                                newheader->resign_lsb = header->resign_lsb;
-                               result = resign_insert(rbtdb, rbtnode->locknum,
-                                                      newheader);
-                               if (result != ISC_R_SUCCESS) {
-                                       free_rdataset(rbtdb, rbtdb->common.mctx,
-                                                     newheader);
-                                       goto unlock;
-                               }
+                               resign_insert(rbtdb, rbtnode->locknum,
+                                             newheader);
                        }
                        /*
                         * We have to set the serial since the rdataslab
@@ -7535,7 +7485,6 @@ loading_addrdataset(void *arg, const dns_name_t *name,
 static isc_result_t
 rbt_datafixer(dns_rbtnode_t *rbtnode, void *base, size_t filesize, void *arg,
              uint64_t *crc) {
-       isc_result_t result;
        dns_rbtdb_t *rbtdb = (dns_rbtdb_t *)arg;
        rdatasetheader_t *header;
        unsigned char *limit = ((unsigned char *)base) + filesize;
@@ -7563,10 +7512,7 @@ rbt_datafixer(dns_rbtnode_t *rbtnode, void *base, size_t filesize, void *arg,
                if (RESIGN(header) &&
                    (header->resign != 0 || header->resign_lsb != 0)) {
                        int idx = header->node->locknum;
-                       result = isc_heap_insert(rbtdb->heaps[idx], header);
-                       if (result != ISC_R_SUCCESS) {
-                               return (result);
-                       }
+                       isc_heap_insert(rbtdb->heaps[idx], header);
                }
 
                if (header->next != NULL) {
@@ -8238,7 +8184,6 @@ getsize(dns_db_t *db, dns_dbversion_t *version, uint64_t *records,
 static isc_result_t
 setsigningtime(dns_db_t *db, dns_rdataset_t *rdataset, isc_stdtime_t resign) {
        dns_rbtdb_t *rbtdb = (dns_rbtdb_t *)db;
-       isc_result_t result = ISC_R_SUCCESS;
        rdatasetheader_t *header, oldheader;
 
        REQUIRE(VALID_RBTDB(rbtdb));
@@ -8277,11 +8222,11 @@ setsigningtime(dns_db_t *db, dns_rdataset_t *rdataset, isc_stdtime_t resign) {
                }
        } else if (resign != 0) {
                RDATASET_ATTR_SET(header, RDATASET_ATTR_RESIGN);
-               result = resign_insert(rbtdb, header->node->locknum, header);
+               resign_insert(rbtdb, header->node->locknum, header);
        }
        NODE_UNLOCK(&rbtdb->node_locks[header->node->locknum].lock,
                    isc_rwlocktype_write);
-       return (result);
+       return (ISC_R_SUCCESS);
 }
 
 static isc_result_t
@@ -8693,11 +8638,7 @@ dns_rbtdb_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type,
        }
        sooner = IS_CACHE(rbtdb) ? ttl_sooner : resign_sooner;
        for (i = 0; i < (int)rbtdb->node_lock_count; i++) {
-               result = isc_heap_create(hmctx, sooner, set_index, 0,
-                                        &rbtdb->heaps[i]);
-               if (result != ISC_R_SUCCESS) {
-                       goto cleanup_heaps;
-               }
+               isc_heap_create(hmctx, sooner, set_index, 0, &rbtdb->heaps[i]);
        }
 
        /*
@@ -8852,26 +8793,6 @@ dns_rbtdb_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type,
 
        return (ISC_R_SUCCESS);
 
-cleanup_heaps:
-       if (rbtdb->heaps != NULL) {
-               for (i = 0; i < (int)rbtdb->node_lock_count; i++) {
-                       if (rbtdb->heaps[i] != NULL) {
-                               isc_heap_destroy(&rbtdb->heaps[i]);
-                       }
-               }
-               isc_mem_put(hmctx, rbtdb->heaps,
-                           rbtdb->node_lock_count * sizeof(isc_heap_t *));
-       }
-
-       if (rbtdb->rdatasets != NULL) {
-               isc_mem_put(mctx, rbtdb->rdatasets,
-                           rbtdb->node_lock_count *
-                                   sizeof(rdatasetheaderlist_t));
-       }
-       if (rbtdb->rrsetstats != NULL) {
-               dns_stats_detach(&rbtdb->rrsetstats);
-       }
-
 cleanup_node_locks:
        isc_mem_put(mctx, rbtdb->node_locks,
                    rbtdb->node_lock_count * sizeof(rbtdb_nodelock_t));

diff --git a/lib/dns/zoneverify.c b/lib/dns/zoneverify.c
index e2fe6a8b44069f50aa6ea8326d121967e33e8f85..e60f6c6c20cda8b1a15e558192f0a89a2597485c 100644 (file)
--- a/lib/dns/zoneverify.c
+++ b/lib/dns/zoneverify.c
@@ -396,13 +396,12 @@ chain_equal(const struct nsec3_chain_fixed *e1,
        return (memcmp(e1 + 1, e2 + 1, data_length) == 0);
 }
 
-static isc_result_t
+static void
 record_nsec3(const vctx_t *vctx, const unsigned char *rawhash,
             const dns_rdata_nsec3_t *nsec3, isc_heap_t *chains) {
-       struct nsec3_chain_fixed *element;
+       struct nsec3_chain_fixed *element = NULL;
+       unsigned char *cp = NULL;
        size_t len;
-       unsigned char *cp;
-       isc_result_t result;
 
        len = sizeof(*element) + nsec3->next_length * 2 + nsec3->salt_length;
 
@@ -418,13 +417,7 @@ record_nsec3(const vctx_t *vctx, const unsigned char *rawhash,
        memmove(cp, rawhash, nsec3->next_length);
        cp += nsec3->next_length;
        memmove(cp, nsec3->next, nsec3->next_length);
-       result = isc_heap_insert(chains, element);
-       if (result != ISC_R_SUCCESS) {
-               zoneverify_log_error(vctx, "isc_heap_insert failed: %s",
-                                    isc_result_totext(result));
-               isc_mem_put(vctx->mctx, element, len);
-       }
-       return (result);
+       isc_heap_insert(chains, element);
 }
 
 /*
@@ -499,12 +492,7 @@ match_nsec3(const vctx_t *vctx, const dns_name_t *name,
        /*
         * Record chain.
         */
-       result = record_nsec3(vctx, rawhash, &nsec3, vctx->expected_chains);
-       if (result != ISC_R_SUCCESS) {
-               zoneverify_log_error(vctx, "record_nsec3(): %s",
-                                    isc_result_totext(result));
-               return (result);
-       }
+       record_nsec3(vctx, rawhash, &nsec3, vctx->expected_chains);
 
        /*
         * Make sure there is only one NSEC3 record with this set of
@@ -608,6 +596,7 @@ record_found(const vctx_t *vctx, const dns_name_t *name, dns_dbnode_t *node,
                if (nsec3.next_length != isc_buffer_usedlength(&b)) {
                        continue;
                }
+
                /*
                 * We only care about NSEC3 records that match a NSEC3PARAM
                 * record.
@@ -619,12 +608,7 @@ record_found(const vctx_t *vctx, const dns_name_t *name, dns_dbnode_t *node,
                /*
                 * Record chain.
                 */
-               result = record_nsec3(vctx, owner, &nsec3, vctx->found_chains);
-               if (result != ISC_R_SUCCESS) {
-                       zoneverify_log_error(vctx, "record_nsec3(): %s",
-                                            isc_result_totext(result));
-                       goto cleanup;
-               }
+               record_nsec3(vctx, owner, &nsec3, vctx->found_chains);
        }
        result = ISC_R_SUCCESS;
 
@@ -1285,11 +1269,9 @@ verifyemptynodes(const vctx_t *vctx, const dns_name_t *name,
        return (ISC_R_SUCCESS);
 }
 
-static isc_result_t
+static void
 vctx_init(vctx_t *vctx, isc_mem_t *mctx, dns_zone_t *zone, dns_db_t *db,
          dns_dbversion_t *ver, dns_name_t *origin, dns_keytable_t *secroots) {
-       isc_result_t result;
-
        memset(vctx, 0, sizeof(*vctx));
 
        vctx->mctx = mctx;
@@ -1311,21 +1293,11 @@ vctx_init(vctx_t *vctx, isc_mem_t *mctx, dns_zone_t *zone, dns_db_t *db,
        dns_rdataset_init(&vctx->nsec3paramsigs);
 
        vctx->expected_chains = NULL;
-       result = isc_heap_create(mctx, chain_compare, NULL, 1024,
-                                &vctx->expected_chains);
-       if (result != ISC_R_SUCCESS) {
-               return (result);
-       }
+       isc_heap_create(mctx, chain_compare, NULL, 1024,
+                       &vctx->expected_chains);
 
        vctx->found_chains = NULL;
-       result = isc_heap_create(mctx, chain_compare, NULL, 1024,
-                                &vctx->found_chains);
-       if (result != ISC_R_SUCCESS) {
-               isc_heap_destroy(&vctx->expected_chains);
-               return (result);
-       }
-
-       return (result);
+       isc_heap_create(mctx, chain_compare, NULL, 1024, &vctx->found_chains);
 }
 
 static void
@@ -1994,10 +1966,7 @@ dns_zoneverify_dnssec(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver,
        isc_result_t result, vresult = ISC_R_UNSET;
        vctx_t vctx;
 
-       result = vctx_init(&vctx, mctx, zone, db, ver, origin, secroots);
-       if (result != ISC_R_SUCCESS) {
-               return (result);
-       }
+       vctx_init(&vctx, mctx, zone, db, ver, origin, secroots);
 
        result = check_apex_rrsets(&vctx);
        if (result != ISC_R_SUCCESS) {

diff --git a/lib/isc/heap.c b/lib/isc/heap.c
index 25b813c0c42947be9b7fb64fd992b787b0d147d4..0f7de62c86d0af20e23cf08b0d967b22875b0eab 100644 (file)
--- a/lib/isc/heap.c
+++ b/lib/isc/heap.c
@@ -78,7 +78,7 @@ heap_check(isc_heap_t *heap) {
 #define heap_check(x) (void)0
 #endif /* ifdef ISC_HEAP_CHECK */
 
-isc_result_t
+void
 isc_heap_create(isc_mem_t *mctx, isc_heapcompare_t compare, isc_heapindex_t idx,
                unsigned int size_increment, isc_heap_t **heapp) {
        isc_heap_t *heap;
@@ -102,8 +102,6 @@ isc_heap_create(isc_mem_t *mctx, isc_heapcompare_t compare, isc_heapindex_t idx,
        heap->index = idx;
 
        *heapp = heap;
-
-       return (ISC_R_SUCCESS);
 }
 
 void
@@ -123,7 +121,7 @@ isc_heap_destroy(isc_heap_t **heapp) {
        isc_mem_putanddetach(&heap->mctx, heap, sizeof(*heap));
 }
 
-static bool
+static void
 resize(isc_heap_t *heap) {
        void **new_array;
        unsigned int new_size;
@@ -139,8 +137,6 @@ resize(isc_heap_t *heap) {
        }
        heap->size = new_size;
        heap->array = new_array;
-
-       return (true);
 }
 
 static void
@@ -194,7 +190,7 @@ sink_down(isc_heap_t *heap, unsigned int i, void *elt) {
        heap_check(heap);
 }
 
-isc_result_t
+void
 isc_heap_insert(isc_heap_t *heap, void *elt) {
        unsigned int new_last;
 
@@ -203,14 +199,12 @@ isc_heap_insert(isc_heap_t *heap, void *elt) {
        heap_check(heap);
        new_last = heap->last + 1;
        RUNTIME_CHECK(new_last > 0); /* overflow check */
-       if (new_last >= heap->size && !resize(heap)) {
-               return (ISC_R_NOMEMORY);
+       if (new_last >= heap->size) {
+               resize(heap);
        }
        heap->last = new_last;
 
        float_up(heap, new_last, elt);
-
-       return (ISC_R_SUCCESS);
 }
 
 void

diff --git a/lib/isc/include/isc/heap.h b/lib/isc/include/isc/heap.h
index 3cba2f6434bb224af7e4da325dfeb38740daaee0..d492c60cec3e23e70a5bf808cc008edeb9971ef4 100644 (file)
--- a/lib/isc/include/isc/heap.h
+++ b/lib/isc/include/isc/heap.h
@@ -47,7 +47,7 @@ typedef void (*isc_heapaction_t)(void *, void *);
 
 typedef struct isc_heap isc_heap_t;
 
-isc_result_t
+void
 isc_heap_create(isc_mem_t *mctx, isc_heapcompare_t compare,
                isc_heapindex_t index, unsigned int size_increment,
                isc_heap_t **heapp);
@@ -72,10 +72,6 @@ isc_heap_create(isc_mem_t *mctx, isc_heapcompare_t compare,
  *     used, which is currently 1024, allowing space for an additional 1024
  *     heap elements to be inserted before adding more space.
  *\li  "heapp" is not NULL, and "*heap" is NULL.
- *
- * Returns:
- *\li  ISC_R_SUCCESS           - success
- *\li  ISC_R_NOMEMORY          - insufficient memory
  */
 
 void
@@ -87,7 +83,7 @@ isc_heap_destroy(isc_heap_t **heapp);
  *\li  "heapp" is not NULL and "*heap" points to a valid isc_heap_t.
  */
 
-isc_result_t
+void
 isc_heap_insert(isc_heap_t *heap, void *elt);
 /*!<
  * \brief Inserts a new element into a heap.

diff --git a/lib/isc/tests/heap_test.c b/lib/isc/tests/heap_test.c
index 7dac7d6ca1425fb3f597052fe91473431db168d1..5c41b20d32c17f0c4ab2e2ae5a739473468e8f60 100644 (file)
--- a/lib/isc/tests/heap_test.c
+++ b/lib/isc/tests/heap_test.c
@@ -76,17 +76,14 @@ idx(void *p, unsigned int i) {
 static void
 isc_heap_delete_test(void **state) {
        isc_heap_t *heap = NULL;
-       isc_result_t result;
        struct e e1 = { 100, 0 };
 
        UNUSED(state);
 
-       result = isc_heap_create(test_mctx, compare, idx, 0, &heap);
-       assert_int_equal(result, ISC_R_SUCCESS);
+       isc_heap_create(test_mctx, compare, idx, 0, &heap);
        assert_non_null(heap);
 
        isc_heap_insert(heap, &e1);
-       assert_int_equal(result, ISC_R_SUCCESS);
        assert_int_equal(e1.index, 1);
 
        isc_heap_delete(heap, e1.index);

diff --git a/lib/isc/timer.c b/lib/isc/timer.c
index 4c2fb3742429b24a77c7c1dcb45f4f17ee0fbdec..82d02dbe672361ee5562bfa1e8abe41c94b898f3 100644 (file)
--- a/lib/isc/timer.c
+++ b/lib/isc/timer.c
@@ -100,7 +100,6 @@ isc_timermgr_poke(isc_timermgr_t *manager0);
 
 static inline isc_result_t
 schedule(isc_timer_t *timer, isc_time_t *now, bool signal_ok) {
-       isc_result_t result;
        isc_timermgr_t *manager;
        isc_time_t due;
        int cmp;
@@ -117,7 +116,7 @@ schedule(isc_timer_t *timer, isc_time_t *now, bool signal_ok) {
         * Compute the new due time.
         */
        if (timer->type != isc_timertype_once) {
-               result = isc_time_add(now, &timer->interval, &due);
+               isc_result_t result = isc_time_add(now, &timer->interval, &due);
                if (result != ISC_R_SUCCESS) {
                        return (result);
                }
@@ -162,11 +161,7 @@ schedule(isc_timer_t *timer, isc_time_t *now, bool signal_ok) {
                }
        } else {
                timer->due = due;
-               result = isc_heap_insert(manager->heap, timer);
-               if (result != ISC_R_SUCCESS) {
-                       INSIST(result == ISC_R_NOMEMORY);
-                       return (ISC_R_NOMEMORY);
-               }
+               isc_heap_insert(manager->heap, timer);
                manager->nscheduled++;
        }
 
@@ -671,7 +666,6 @@ set_index(void *what, unsigned int index) {
 isc_result_t
 isc_timermgr_create(isc_mem_t *mctx, isc_timermgr_t **managerp) {
        isc_timermgr_t *manager;
-       isc_result_t result;
 
        /*
         * Create a timer manager.
@@ -688,12 +682,7 @@ isc_timermgr_create(isc_mem_t *mctx, isc_timermgr_t **managerp) {
        manager->nscheduled = 0;
        isc_time_settoepoch(&manager->due);
        manager->heap = NULL;
-       result = isc_heap_create(mctx, sooner, set_index, 0, &manager->heap);
-       if (result != ISC_R_SUCCESS) {
-               INSIST(result == ISC_R_NOMEMORY);
-               isc_mem_put(mctx, manager, sizeof(*manager));
-               return (ISC_R_NOMEMORY);
-       }
+       isc_heap_create(mctx, sooner, set_index, 0, &manager->heap);
        isc_mutex_init(&manager->lock);
        isc_mem_attach(mctx, &manager->mctx);
        isc_condition_init(&manager->wakeup);