Merge branch 'ondrej/lib/isc/heap.c-cleanup' into 'main'

Make isc_heap_create() and isc_heap_insert() return void

See merge request isc-projects/bind9!5756

lib/dns/rbtdb.c

@@ -558,7 +558,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,
@@ -2587,17 +2587,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",
-					      isc_result_totext(result));
-			}
+			resign_insert(rbtdb, header->node->locknum, header);
 		}
 		decrement_reference(rbtdb, header->node, least_serial,
 				    isc_rwlocktype_write, isc_rwlocktype_none,
@@ -6025,16 +6015,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);
 }
 
 /*
@@ -6457,20 +6444,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
@@ -6500,13 +6476,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);
@@ -6515,12 +6485,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) {
@@ -6565,12 +6530,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);
@@ -6579,12 +6539,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);
 		}
 
@@ -7111,13 +7066,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
@@ -7785,7 +7735,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));
@@ -7824,11 +7773,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
@@ -8236,11 +8185,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]);
 	}
 
 	/*
@@ -8395,26 +8340,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));

lib/dns/zoneverify.c

@@ -395,13 +395,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;
 
@@ -417,13 +416,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);
 }
 
 /*
@@ -498,12 +491,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
@@ -607,6 +595,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.
@@ -618,12 +607,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;
 
@@ -1284,11 +1268,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;
@@ -1310,21 +1292,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
@@ -1993,10 +1965,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) {

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

lib/isc/include/isc/heap.h

@@ -46,7 +46,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);
@@ -71,10 +71,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
@@ -86,7 +82,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.

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);

lib/isc/timer.c

@@ -97,7 +97,6 @@ isc_timermgr_poke(isc_timermgr_t *manager);
 
 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;
 
@@ -113,7 +112,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);
 		}
@@ -158,11 +157,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++;
 	}
 
@@ -657,7 +652,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.
@@ -674,12 +668,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);