From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Thu, 3 Dec 2020 15:56:20 +0000 (-0700)
Subject: Remove duplicate hash table free function
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b72466883d8b008a7f0a56f27269df80ac631fbd;p=thirdparty%2Ffreeradius-server.git

Remove duplicate hash table free function

Fix thread safety issues in rlm_detail
---

diff --git a/src/lib/util/hash.c b/src/lib/util/hash.c
index ec2009f5d64..f3849979b83 100644
--- a/src/lib/util/hash.c
+++ b/src/lib/util/hash.c
@@ -255,7 +255,22 @@ static int list_delete(fr_hash_table_t *ht,
 
 static int _fr_hash_table_free(fr_hash_table_t *ht)
 {
-	talloc_free_children(ht);
+	int i;
+	fr_hash_entry_t *node, *next;
+
+	if (ht->free) {
+		for (i = 0; i < ht->num_buckets; i++) {
+			if (ht->buckets[i]) for (node = ht->buckets[i];
+						 node != &ht->null;
+						 node = next) {
+				next = node->next;
+				if (!node->data) continue; /* dummy entry */
+
+				ht->free(node->data);
+			}
+		}
+	}
+
 	return 0;
 }
 
@@ -551,40 +566,6 @@ int fr_hash_table_delete(fr_hash_table_t *ht, void const *data)
 	return 1;
 }
 
-
-/*
- *	Free a hash table
- */
-void fr_hash_table_free(fr_hash_table_t *ht)
-{
-	int i;
-	fr_hash_entry_t *node, *next;
-
-	if (!ht) return;
-
-	/*
-	 *	Walk over the buckets, freeing them all.
-	 */
-	if (ht->free) {
-		for (i = 0; i < ht->num_buckets; i++) {
-			if (ht->buckets[i]) for (node = ht->buckets[i];
-						 node != &ht->null;
-						 node = next) {
-				next = node->next;
-				if (!node->data) continue; /* dummy entry */
-
-				ht->free(node->data);
-			}
-		}
-	}
-
-	/*
-	 *	Also frees nodes and buckets
-	 */
-	talloc_free(ht);
-}
-
-
 /*
  *	Count number of elements
  */
diff --git a/src/lib/util/hash.h b/src/lib/util/hash.h
index bbff2b32d5b..f594b86dc25 100644
--- a/src/lib/util/hash.h
+++ b/src/lib/util/hash.h
@@ -64,8 +64,6 @@ fr_hash_table_t *fr_hash_table_create(TALLOC_CTX *ctx,
 				      fr_hash_table_cmp_t cmpNode,
 				      fr_hash_table_free_t freeNode);
 
-void		fr_hash_table_free(fr_hash_table_t *ht);
-
 int		fr_hash_table_insert(fr_hash_table_t *ht, void const *data);
 
 int		fr_hash_table_delete(fr_hash_table_t *ht, void const *data);
diff --git a/src/modules/rlm_detail/rlm_detail.c b/src/modules/rlm_detail/rlm_detail.c
index adfe492b288..bdb972a7359 100644
--- a/src/modules/rlm_detail/rlm_detail.c
+++ b/src/modules/rlm_detail/rlm_detail.c
@@ -116,18 +116,6 @@ fr_dict_attr_autoload_t rlm_detail_dict_attr[] = {
 	{ NULL }
 };
 
-/*
- *	Clean up.
- */
-static int mod_detach(void *instance)
-{
-	rlm_detail_t *inst = instance;
-
-	if (inst->ht) fr_hash_table_free(inst->ht);
-	return 0;
-}
-
-
 static uint32_t detail_hash(void const *data)
 {
 	fr_dict_attr_t const *da = data;
@@ -172,7 +160,7 @@ static int mod_instantiate(void *instance, CONF_SECTION *conf)
 	if (cs) {
 		CONF_ITEM	*ci;
 
-		inst->ht = fr_hash_table_create(NULL, detail_hash, detail_cmp, NULL);
+		inst->ht = fr_hash_table_create(inst, detail_hash, detail_cmp, NULL);
 
 		for (ci = cf_item_next(cs, NULL);
 		     ci != NULL;
@@ -212,8 +200,9 @@ static int mod_instantiate(void *instance, CONF_SECTION *conf)
 		 *	If we didn't suppress anything, delete the hash table.
 		 */
 		if (fr_hash_table_num_elements(inst->ht) == 0) {
-			fr_hash_table_free(inst->ht);
-			inst->ht = NULL;
+			TALLOC_FREE(inst->ht);
+		} else {
+			fr_hash_table_fill(inst->ht);
 		}
 	}
 
@@ -484,7 +473,6 @@ module_t rlm_detail = {
 	.inst_size	= sizeof(rlm_detail_t),
 	.config		= module_config,
 	.instantiate	= mod_instantiate,
-	.detach		= mod_detach,
 	.methods = {
 		[MOD_AUTHORIZE]		= mod_authorize,
 		[MOD_PREACCT]		= mod_accounting,