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.
FreeRADIUS · Sep 4, 2023 · 1f40232 · 1f40232
1 parent e0c27e9
commit 1f40232
Show file tree

Hide file tree

Showing 10 changed files with 53 additions and 47 deletions.
diff --git a/src/lib/unlang/base.c b/src/lib/unlang/base.c
@@ -69,32 +69,51 @@ 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) {
 		instance_count++;
 		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();
 	unlang_map_init();
 	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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,18 +3403,18 @@ 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);
 
 	/*
 	 *	All of these functions are pure.
 	 */
 #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,15 +3462,15 @@ 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)
 
 	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();