From: Alan T. DeKok <aland@freeradius.org>
Date: Mon, 4 Sep 2023 12:17:39 +0000 (-0400)
Subject: we have talloc destructors, so fewer explicit free()s are necessary
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1f40232e5338530baccee1bcc0875e5d390e3a75;p=thirdparty%2Ffreeradius-server.git

we have talloc destructors, so fewer explicit free()s are necessary

and rearrange some of the code in the unlang initialization
routines, so that everything is cleaned up on error, or on clean
exit.
---

diff --git a/src/lib/unlang/base.c b/src/lib/unlang/base.c
index d7cb6458a5f..d673aab14e4 100644
--- a/src/lib/unlang/base.c
+++ b/src/lib/unlang/base.c
@@ -69,6 +69,8 @@ void unlang_register(int type, unlang_op_t *op)
 	memcpy(&unlang_ops[type], op, sizeof(unlang_ops[type]));
 }
 
+static TALLOC_CTX *unlang_ctx = NULL;
+
 int unlang_init_global(void)
 {
 	if (instance_count > 0) {
@@ -76,17 +78,35 @@ int unlang_init_global(void)
 		return 0;
 	}
 
+	unlang_ctx = talloc_init("unlang");
+	if (!unlang_ctx) return NULL;
+
 	/*
 	 *	Explicitly initialise the xlat tree, and perform dictionary lookups.
 	 */
-	if (xlat_init() < 0) return -1;
+	if (xlat_init(unlang_ctx) < 0) {
+	fail:
+		TALLOC_FREE(unlang_ctx);
+
+		memset(unlang_ops, 0, sizeof(unlang_ops));
+		return -1;
+	}
 
-	unlang_interpret_init_global();
+	unlang_interpret_init_global(unlang_ctx);
 
-	/* Register operations for the default keywords */
-	unlang_compile_init();
+	/*
+	 *	Operations which can fail, and which require cleanup.
+	 */
+	if (unlang_subrequest_op_init() < 0) goto fail;
+
+	/*
+	 *	Register operations for the default keywords.  The
+	 *	operations listed below cannot fail, and do not
+	 *	require cleanup.
+	 */
+	unlang_compile_init(unlang_ctx);
 	unlang_condition_init();
-	unlang_foreach_init();
+	unlang_foreach_init(unlang_ctx);
 	unlang_function_init();
 	unlang_group_init();
 	unlang_load_balance_init();
@@ -94,7 +114,6 @@ int unlang_init_global(void)
 	unlang_module_init();
 	unlang_parallel_init();
 	unlang_return_init();
-	if (unlang_subrequest_op_init() < 0) return -1;
 	unlang_detach_init();
 	unlang_switch_init();
 	unlang_call_init();
@@ -114,8 +133,10 @@ void unlang_free_global(void)
 {
 	if (--instance_count > 0) return;
 
-	unlang_compile_free();
-	unlang_foreach_free();
 	unlang_subrequest_op_free();
+	TALLOC_FREE(unlang_ctx);
+
+	memset(unlang_ops, 0, sizeof(unlang_ops));
+
 	xlat_free();
 }
diff --git a/src/lib/unlang/call_env.c b/src/lib/unlang/call_env.c
index e304bc53282..3c74fc4c40e 100644
--- a/src/lib/unlang/call_env.c
+++ b/src/lib/unlang/call_env.c
@@ -271,11 +271,12 @@ static unlang_action_t call_env_expand_start(UNUSED rlm_rcode_t *p_result, UNUSE
 {
 	call_env_ctx_t	*call_env_ctx = talloc_get_type_abort(uctx, call_env_ctx_t);
 	TALLOC_CTX	*ctx;
-	call_env_parsed_t const	*env;
+	call_env_parsed_t const	*env = NULL;
 	void		**out;
 
 	while ((call_env_ctx->last_expanded = call_env_parsed_next(call_env_ctx->parsed, call_env_ctx->last_expanded))) {
 		env = call_env_ctx->last_expanded;
+		fr_assert(env != NULL);
 
 		if (!env->tmpl_only) break;
 
@@ -293,6 +294,8 @@ static unlang_action_t call_env_expand_start(UNUSED rlm_rcode_t *p_result, UNUSE
 
 	ctx = *call_env_ctx->data;
 
+	fr_assert(env != NULL);
+
 	/*
 	 *	Multi pair options should allocate boxes in the context of the array
 	 */
diff --git a/src/lib/unlang/compile.c b/src/lib/unlang/compile.c
index 069e6c020c9..f32dee679d0 100644
--- a/src/lib/unlang/compile.c
+++ b/src/lib/unlang/compile.c
@@ -4745,14 +4745,9 @@ static int8_t instruction_cmp(void const *one, void const *two)
 }
 
 
-void unlang_compile_init(void)
+void unlang_compile_init(TALLOC_CTX *ctx)
 {
-	unlang_instruction_tree = fr_rb_alloc(NULL, instruction_cmp, NULL);
-}
-
-void unlang_compile_free(void)
-{
-	TALLOC_FREE(unlang_instruction_tree);
+	unlang_instruction_tree = fr_rb_alloc(ctx, instruction_cmp, NULL);
 }
 
 
diff --git a/src/lib/unlang/compile.h b/src/lib/unlang/compile.h
index e584f903112..9dd417cb3eb 100644
--- a/src/lib/unlang/compile.h
+++ b/src/lib/unlang/compile.h
@@ -38,9 +38,7 @@ typedef struct {
 	fr_retry_config_t	retry;
 } unlang_actions_t;
 
-void		unlang_compile_init(void);
-
-void		unlang_compile_free(void);
+void		unlang_compile_init(TALLOC_CTX *ctx);
 
 int		unlang_compile(CONF_SECTION *cs, rlm_components_t component, tmpl_rules_t const *rules, void **instruction);
 
diff --git a/src/lib/unlang/foreach.c b/src/lib/unlang/foreach.c
index 15630b79db6..68938e1d12b 100644
--- a/src/lib/unlang/foreach.c
+++ b/src/lib/unlang/foreach.c
@@ -268,14 +268,14 @@ static xlat_action_t unlang_foreach_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out,
 	return XLAT_ACTION_DONE;
 }
 
-void unlang_foreach_init(void)
+void unlang_foreach_init(TALLOC_CTX *ctx)
 {
 	size_t	i;
 
 	for (i = 0; i < NUM_ELEMENTS(xlat_foreach_names); i++) {
 		xlat_t *x;
 
-		x = xlat_func_register(NULL, xlat_foreach_names[i],
+		x = xlat_func_register(ctx, xlat_foreach_names[i],
 				  unlang_foreach_xlat, FR_TYPE_VOID);
 		fr_assert(x);
 		xlat_func_flags_set(x, XLAT_FUNC_FLAG_INTERNAL);
@@ -295,12 +295,3 @@ void unlang_foreach_init(void)
 				.interpret = unlang_break,
 			   });
 }
-
-void unlang_foreach_free(void)
-{
-	size_t	i;
-
-	for (i = 0; i < NUM_ELEMENTS(xlat_foreach_names); i++) {
-		xlat_func_unregister(xlat_foreach_names[i]);
-	}
-}
diff --git a/src/lib/unlang/interpret.c b/src/lib/unlang/interpret.c
index d4534544c0d..da7ce7d316d 100644
--- a/src/lib/unlang/interpret.c
+++ b/src/lib/unlang/interpret.c
@@ -1688,17 +1688,17 @@ unlang_interpret_t *unlang_interpret_get_thread_default(void)
 	return talloc_get_type_abort(intp_thread_default, unlang_interpret_t);
 }
 
-int unlang_interpret_init_global(void)
+int unlang_interpret_init_global(TALLOC_CTX *ctx)
 {
 	xlat_t	*xlat;
 	/*
 	 *  Should be void, but someone decided not to register multiple xlats
 	 *  breaking the convention we use everywhere else in the server...
 	 */
-	if (unlikely((xlat = xlat_func_register(NULL, "interpreter", unlang_interpret_xlat, FR_TYPE_VOID)) == NULL)) return -1;
+	if (unlikely((xlat = xlat_func_register(ctx, "interpreter", unlang_interpret_xlat, FR_TYPE_VOID)) == NULL)) return -1;
 	xlat_func_args_set(xlat, unlang_interpret_xlat_args);
 
-	if (unlikely((xlat = xlat_func_register(NULL, "cancel", unlang_cancel_xlat, FR_TYPE_VOID)) == NULL)) return -1;
+	if (unlikely((xlat = xlat_func_register(ctx, "cancel", unlang_cancel_xlat, FR_TYPE_VOID)) == NULL)) return -1;
 	xlat_func_args_set(xlat, unlang_cancel_xlat_args);
 
 	return 0;
diff --git a/src/lib/unlang/interpret.h b/src/lib/unlang/interpret.h
index ee57de2b3a7..7d0ca3b49d5 100644
--- a/src/lib/unlang/interpret.h
+++ b/src/lib/unlang/interpret.h
@@ -168,7 +168,7 @@ void			unlang_interpret_stack_result_set(request_t *request, rlm_rcode_t code);
 
 TALLOC_CTX		*unlang_interpret_frame_talloc_ctx(request_t *request);
 
-int			unlang_interpret_init_global(void);
+int			unlang_interpret_init_global(TALLOC_CTX *ctx);
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/lib/unlang/unlang_priv.h b/src/lib/unlang/unlang_priv.h
index 61a8ea98a5e..ddb34fd2a77 100644
--- a/src/lib/unlang/unlang_priv.h
+++ b/src/lib/unlang/unlang_priv.h
@@ -589,9 +589,7 @@ void		unlang_caller_init(void);
 
 void		unlang_condition_init(void);
 
-void		unlang_foreach_init(void);
-
-void		unlang_foreach_free(void);
+void		unlang_foreach_init(TALLOC_CTX *ctx);
 
 void		unlang_function_init(void);
 
diff --git a/src/lib/unlang/xlat.h b/src/lib/unlang/xlat.h
index 785f77fb6f2..eea0b348385 100644
--- a/src/lib/unlang/xlat.h
+++ b/src/lib/unlang/xlat.h
@@ -503,7 +503,7 @@ xlat_action_t	unlang_xlat_yield(request_t *request,
  *	xlat_builtin.c
  */
 int		xlat_protocols_register(void);
-int		xlat_init(void);
+int		xlat_init(TALLOC_CTX *ctx);
 void		xlat_free(void);
 
 #ifdef __cplusplus
diff --git a/src/lib/unlang/xlat_builtin.c b/src/lib/unlang/xlat_builtin.c
index 65ea494a22e..76ce33a072f 100644
--- a/src/lib/unlang/xlat_builtin.c
+++ b/src/lib/unlang/xlat_builtin.c
@@ -3339,7 +3339,7 @@ int xlat_protocols_register(void)
  *
  * @hidecallgraph
  */
-int xlat_init(void)
+int xlat_init(TALLOC_CTX *ctx)
 {
 	xlat_t *xlat;
 
@@ -3360,7 +3360,7 @@ int xlat_init(void)
 	 */
 #define XLAT_REGISTER_ARGS(_xlat, _func, _return_type, _args) \
 do { \
-	if (unlikely((xlat = xlat_func_register(NULL, _xlat, _func, _return_type)) == NULL)) return -1; \
+	if (unlikely((xlat = xlat_func_register(ctx, _xlat, _func, _return_type)) == NULL)) return -1; \
 	xlat_func_args_set(xlat, _args); \
 	xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_PURE | XLAT_FUNC_FLAG_INTERNAL); \
 } while (0)
@@ -3383,7 +3383,7 @@ do { \
 #undef XLAT_REGISTER_ARGS
 #define XLAT_REGISTER_ARGS(_xlat, _func, _return_type, _args) \
 do { \
-	if (unlikely((xlat = xlat_func_register(NULL, _xlat, _func, _return_type)) == NULL)) return -1; \
+	if (unlikely((xlat = xlat_func_register(ctx, _xlat, _func, _return_type)) == NULL)) return -1; \
 	xlat_func_args_set(xlat, _args); \
 	xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_INTERNAL); \
 } while (0)
@@ -3403,10 +3403,10 @@ do { \
 	XLAT_REGISTER_ARGS("flatten", xlat_func_flatten, FR_TYPE_NULL, xlat_func_debug_attr_args); /* takes an attribute reference */
 	XLAT_REGISTER_ARGS("unflatten", xlat_func_unflatten, FR_TYPE_NULL, xlat_func_debug_attr_args); /* takes an attribute reference */
 
-	if (unlikely((xlat = xlat_func_register(NULL, "untaint", xlat_func_untaint, FR_TYPE_VOID)) == NULL)) return -1;
+	if (unlikely((xlat = xlat_func_register(ctx, "untaint", xlat_func_untaint, FR_TYPE_VOID)) == NULL)) return -1;
 	xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_INTERNAL);
 
-	if (unlikely((xlat = xlat_func_register(NULL, "taint", xlat_func_taint, FR_TYPE_VOID)) == NULL)) return -1;
+	if (unlikely((xlat = xlat_func_register(ctx, "taint", xlat_func_taint, FR_TYPE_VOID)) == NULL)) return -1;
 	xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_INTERNAL);
 
 	/*
@@ -3414,7 +3414,7 @@ do { \
 	 */
 #define XLAT_REGISTER_MONO(_xlat, _func, _return_type, _arg) \
 do { \
-	if (unlikely((xlat = xlat_func_register(NULL, _xlat, _func, _return_type)) == NULL)) return -1; \
+	if (unlikely((xlat = xlat_func_register(ctx, _xlat, _func, _return_type)) == NULL)) return -1; \
 	xlat_func_mono_set(xlat, _arg); \
 	xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_PURE | XLAT_FUNC_FLAG_INTERNAL); \
 } while (0)
@@ -3428,7 +3428,7 @@ do { \
 	XLAT_REGISTER_MONO("md5", xlat_func_md5, FR_TYPE_OCTETS, xlat_func_md5_arg);
 	XLAT_REGISTER_MONO("pack", xlat_func_pack, FR_TYPE_OCTETS, xlat_func_pack_arg);
 #if defined(HAVE_REGEX_PCRE) || defined(HAVE_REGEX_PCRE2)
-	if (unlikely((xlat = xlat_func_register(NULL, "regex", xlat_func_regex, FR_TYPE_STRING)) == NULL)) return -1;
+	if (unlikely((xlat = xlat_func_register(ctx, "regex", xlat_func_regex, FR_TYPE_STRING)) == NULL)) return -1;
 	xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_INTERNAL);
 #endif
 	XLAT_REGISTER_MONO("sha1", xlat_func_sha1, FR_TYPE_OCTETS, xlat_func_sha_arg);
@@ -3462,7 +3462,7 @@ do { \
 #undef XLAT_REGISTER_MONO
 #define XLAT_REGISTER_MONO(_xlat, _func, _return_type, _arg) \
 do { \
-	if (unlikely((xlat = xlat_func_register(NULL, _xlat, _func, _return_type)) == NULL)) return -1; \
+	if (unlikely((xlat = xlat_func_register(ctx, _xlat, _func, _return_type)) == NULL)) return -1; \
 	xlat_func_mono_set(xlat, _arg); \
 	xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_INTERNAL); \
 } while (0)
@@ -3470,7 +3470,7 @@ do { \
 	XLAT_REGISTER_MONO("rand", xlat_func_rand, FR_TYPE_UINT64, xlat_func_rand_arg);
 	XLAT_REGISTER_MONO("randstr", xlat_func_randstr, FR_TYPE_STRING, xlat_func_randstr_arg);
 
-	if (unlikely((xlat = xlat_func_register(NULL, "module", xlat_func_module, FR_TYPE_STRING)) == NULL)) return -1;
+	if (unlikely((xlat = xlat_func_register(ctx, "module", xlat_func_module, FR_TYPE_STRING)) == NULL)) return -1;
 	xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_INTERNAL);
 
 	return xlat_register_expressions();