From: Alan T. DeKok <aland@freeradius.org>
Date: Thu, 19 May 2022 23:01:03 +0000 (-0400)
Subject: only set destructors if they're going to do work.
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ec69d6487bf2c748bfe8fea84087789945209cbd;p=thirdparty%2Ffreeradius-server.git

only set destructors if they're going to do work.

This papers over certain issues with pure functions, and purifying
them.  But arguably pure functions should use no instance data,
and should take no destructors.  If they do, they're not pure.
---

diff --git a/src/lib/unlang/xlat_inst.c b/src/lib/unlang/xlat_inst.c
index 47ecc6154d1..b6c6cf994f6 100644
--- a/src/lib/unlang/xlat_inst.c
+++ b/src/lib/unlang/xlat_inst.c
@@ -81,12 +81,12 @@ static int _xlat_thread_inst_detach(xlat_thread_inst_t *xt)
 
 	DEBUG4("Cleaning up xlat thread instance (%p/%p)", xt, xt->data);
 
-	if (call->func->thread_detach) {
-		(void) call->func->thread_detach(XLAT_THREAD_INST_CTX(call->inst->data,
-								      xt->data, xt->node, xt->mctx,
-								      xt->el,
-								      call->func->thread_uctx));
-	}
+	fr_assert(call->func->thread_detach);
+
+	(void) call->func->thread_detach(XLAT_THREAD_INST_CTX(call->inst->data,
+							      xt->data, xt->node, xt->mctx,
+							      xt->el,
+							      call->func->thread_uctx));
 
 	return 0;
 }
@@ -135,7 +135,7 @@ static xlat_thread_inst_t *xlat_thread_inst_alloc(TALLOC_CTX *ctx, fr_event_list
 
 	fr_assert(xi->node->type == XLAT_FUNC);
 
-	talloc_set_destructor(xt, _xlat_thread_inst_detach);
+	if (call->func->thread_detach) talloc_set_destructor(xt, _xlat_thread_inst_detach);
 
 	if (call->func->thread_inst_size) {
 		MEM(xt->data = talloc_zero_array(xt, uint8_t, call->func->thread_inst_size));
@@ -178,11 +178,15 @@ static xlat_thread_inst_t *xlat_thread_inst_alloc(TALLOC_CTX *ctx, fr_event_list
  */
 static int _xlat_inst_detach(xlat_inst_t *xi)
 {
-	xlat_call_t const *call = &((xlat_exp_t const *)talloc_get_type_abort_const(xi->node, xlat_exp_t))->call;
+	xlat_call_t const *call;
 
 	fr_assert(xlat_inst_tree);		/* xlat_inst_init must have been called */
+
+	talloc_get_type_abort_const(xi->node, xlat_exp_t);
 	fr_assert(xi->node->type == XLAT_FUNC);
 
+	call = &xi->node->call;
+
 	/*
 	 *	Remove permanent data from the instance tree
 	 *	and auto-free the tree when the last xlat is
@@ -228,7 +232,10 @@ static xlat_inst_t *xlat_inst_alloc(xlat_exp_t *node)
 	 *	Instance data is freed when the
 	 *	node is freed.
 	 */
-	talloc_set_destructor(xi, _xlat_inst_detach);
+	if (call->func->detach || !call->ephemeral) {
+		talloc_set_destructor(xi, _xlat_inst_detach);
+	}
+
 	if (call->func->inst_size) {
 		MEM(xi->data = talloc_zero_array(xi, uint8_t, call->func->inst_size));
 		if (call->func->inst_type) {
@@ -260,6 +267,12 @@ static int _xlat_instantiate_ephemeral_walker(xlat_exp_t *node, void *uctx)
 
 	fr_assert(!call->inst && !call->thread_inst);
 
+	/*
+	 *	Mark this up as an ephemeral node, so the destructors
+	 *	don't search for it in the xlat_inst_tree.
+	 */
+	call->ephemeral = true;
+
 	xi = call->inst = xlat_inst_alloc(node);
 	if (!xi) return -1;
 
@@ -291,12 +304,6 @@ static int _xlat_instantiate_ephemeral_walker(xlat_exp_t *node, void *uctx)
 	    					 		 el,
 	    					 		 call->func->thread_uctx)) < 0)) goto error;
 
-	/*
-	 *	Mark this up as an ephemeral node, so the destructors
-	 *	don't search for it in the xlat_inst_tree.
-	 */
-	call->ephemeral = true;
-
 	return 0;
 }