From: Arran Cudbard-Bell Date: Wed, 15 Nov 2023 23:34:15 +0000 (-0600) Subject: Remove duplicate code for setting up call_envs X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=20832938eea2ce1babaaca57289469ce5e3d6d59;p=thirdparty%2Ffreeradius-server.git Remove duplicate code for setting up call_envs Wrap the ctx, tmpls and method_env in a single structure, and just pass that around... Make the count and parsing functions private, because we're essentially going to do the same work anywhere call envs are used. --- diff --git a/src/lib/unlang/call_env.c b/src/lib/unlang/call_env.c index 843de196745..a0b05932771 100644 --- a/src/lib/unlang/call_env.c +++ b/src/lib/unlang/call_env.c @@ -28,6 +28,7 @@ RCSID("$Id$") #include #include #include +#include #include "call_env.h" @@ -93,7 +94,7 @@ call_env_result_t call_env_value_parse(TALLOC_CTX *ctx, request_t *request, void */ typedef struct { call_env_result_t *result; //!< Where to write the return code of callenv expansion. - call_env_parsed_head_t const *parsed; //!< Head of the parsed list of tmpls to expand. + call_env_t const *call_env; //!< Call env being expanded. call_env_parsed_t const *last_expanded; //!< The last expanded tmpl. fr_value_box_list_t tmpl_expanded; //!< List to write value boxes to as tmpls are expanded. void **data; //!< Final destination structure for value boxes. @@ -111,7 +112,7 @@ static unlang_action_t call_env_expand_start(UNUSED rlm_rcode_t *p_result, UNUSE 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))) { + while ((call_env_ctx->last_expanded = call_env_parsed_next(&call_env_ctx->call_env->pairs, call_env_ctx->last_expanded))) { env = call_env_ctx->last_expanded; fr_assert(env != NULL); @@ -196,7 +197,7 @@ tmpl_only: return UNLANG_ACTION_FAIL; } - if (!call_env_parsed_next(call_env_ctx->parsed, env)) { + if (!call_env_parsed_next(&call_env_ctx->call_env->pairs, env)) { if (call_env_ctx->result) *call_env_ctx->result = CALL_ENV_SUCCESS; return UNLANG_ACTION_CALCULATE_RESULT; } @@ -212,20 +213,19 @@ tmpl_only: * @param[out] env_result Where to write the result of the callenv expansion. May be NULL * @param[in,out] env_data Where the destination structure should be created. * @param[in] call_env Call environment being expanded. - * @param[in] call_env_parsed Parsed tmpls for the call environment. */ -unlang_action_t call_env_expand(TALLOC_CTX *ctx, request_t *request, call_env_result_t *env_result, void **env_data, call_env_method_t const *call_env, - call_env_parsed_head_t const *call_env_parsed) +unlang_action_t call_env_expand(TALLOC_CTX *ctx, request_t *request, call_env_result_t *env_result, void **env_data, + call_env_t const *call_env) { call_env_ctx_t *call_env_ctx; MEM(call_env_ctx = talloc_zero(ctx, call_env_ctx_t)); - MEM(*env_data = talloc_zero_array(ctx, uint8_t, call_env->inst_size)); - talloc_set_name_const(*env_data, call_env->inst_type); + MEM(*env_data = talloc_zero_array(ctx, uint8_t, call_env->method->inst_size)); + talloc_set_name_const(*env_data, call_env->method->inst_type); call_env_ctx->result = env_result; if (env_result) *env_result = CALL_ENV_INVALID; /* Make sure we ran to completion*/ call_env_ctx->data = env_data; - call_env_ctx->parsed = call_env_parsed; + call_env_ctx->call_env = call_env; fr_value_box_list_init(&call_env_ctx->tmpl_expanded); return unlang_function_push(request, call_env_expand_start, call_env_expand_repeat, NULL, 0, UNLANG_SUB_FRAME, @@ -247,6 +247,7 @@ unlang_action_t call_env_expand(TALLOC_CTX *ctx, request_t *request, call_env_re * - 0 on success; * - <0 on failure; */ + static inline CC_HINT(always_inline) int call_env_parse(TALLOC_CTX *ctx, call_env_parsed_head_t *parsed, char const *name, fr_dict_t const *dict_def, CONF_SECTION const *cs, call_env_parser_t const *call_env) { CONF_PAIR const *cp, *next; @@ -367,11 +368,14 @@ int call_env_parse(TALLOC_CTX *ctx, call_env_parsed_head_t *parsed, char const * * @param[in] call_env to parse. * @return Number of parsed_call_env expected to be required. */ +static inline CC_HINT(always_inline) size_t call_env_count(size_t *names_len, CONF_SECTION const *cs, call_env_parser_t const *call_env) { size_t pair_count, tmpl_count = 0; CONF_PAIR const *cp; + *names_len = 0; + while (call_env->name) { if (FR_BASE_TYPE(call_env->type) == FR_TYPE_SUBSECTION) { CONF_SECTION const *subcs; @@ -398,3 +402,53 @@ size_t call_env_count(size_t *names_len, CONF_SECTION const *cs, call_env_parser return tmpl_count; } + +/** Given a call_env_method, parse all call_env_pair_t in the context of a specific call to an xlat or module method + * + * @param[in] ctx to allocate the call_env_t in. + * @param[in] name Module name for error messages. + * @param[in] call_env_method containing the call_env_pair_t to evaluate against the specified CONF_SECTION. + * @param[in] namespace to pass to the tmpl tokenizer when parsing pairs from the specified CONF_SECTION. + * @param[in] cs to parse in the context of the call. + * @return + * - A new call_env_t on success. + * - NULL on failure. + */ +call_env_t *call_env_alloc(TALLOC_CTX *ctx, char const *name, call_env_method_t const *call_env_method, + fr_dict_t const *namespace, CONF_SECTION *cs) +{ + unsigned int count; + size_t names_len; + call_env_t *call_env; + + /* + * Only used if caller doesn't use a more specific assert + */ + fr_assert_msg(call_env_method->inst_size, "inst_size 0 for %s, method_env (%p)", name, call_env_method); + + /* + * Firstly assess how many parsed env there will be and create a talloc pool to hold them. + * The pool size is a rough estimate based on each tmpl also allocating at least two children, + * for which we allow twice the length of the value to be parsed. + */ + count = call_env_count(&names_len, cs, call_env_method->env); + + /* + * Pre-allocated headers: + * 1 header for the call_env_pair_parsed_t, 1 header for the tmpl_t, 1 header for the name, + * one header for the value. + * + * Pre-allocated memory: + * ((sizeof(call_env_pair_parsed_t) + sizeof(tmpl_t)) * count) + (names of tmpls * 2)... Not sure what + * the * 2 is for, maybe for slop? + */ + MEM(call_env = talloc_pooled_object(ctx, call_env_t, count * 4, (sizeof(call_env_parser_t) + sizeof(tmpl_t)) * count + names_len * 2)); + call_env->method = call_env_method; + call_env_parsed_init(&call_env->pairs); + if (call_env_parse(call_env, &call_env->pairs, name, namespace, cs, call_env_method->env) < 0) { + talloc_free(call_env); + return NULL; + } + + return call_env; +} diff --git a/src/lib/unlang/call_env.h b/src/lib/unlang/call_env.h index b069b151661..d84645240fb 100644 --- a/src/lib/unlang/call_env.h +++ b/src/lib/unlang/call_env.h @@ -34,6 +34,7 @@ extern "C" { typedef struct call_env_parser_s call_env_parser_t; typedef struct call_env_parsed_s call_env_parsed_t; typedef struct call_env_method_s call_env_method_t; +typedef struct call_env_s call_env_t; FR_DLIST_TYPES(call_env_parsed) FR_DLIST_TYPEDEFS(call_env_parsed, call_env_parsed_head_t, call_env_parsed_entry_t) @@ -119,9 +120,16 @@ FR_DLIST_FUNCS(call_env_parsed, call_env_parsed_t, entry) .inst_type = STRINGIFY(_inst) \ struct call_env_method_s { - size_t inst_size; //!< Size of per call env. - char const *inst_type; //!< Type of per call env. - call_env_parser_t const *env; //!< Parsing rules for call method env. + size_t inst_size; //!< Size of per call env. + char const *inst_type; //!< Type of per call env. + call_env_parser_t const *env; //!< Parsing rules for call method env. +}; + +/** Structure containing both a talloc pool, a list of parsed call_env_pairs + */ +struct call_env_s { + call_env_parsed_head_t pairs; //!< The per call parsed call environment. + call_env_method_t const *method; //!< The method this call env is for. }; /** Derive whether tmpl can only emit a single box. @@ -237,13 +245,10 @@ _Generic((((_s *)NULL)->_f), \ .required = _required \ } -unlang_action_t call_env_expand(TALLOC_CTX *ctx, request_t *request, call_env_result_t *result, void **env_data, call_env_method_t const *call_env, - call_env_parsed_head_t const *call_env_parsed); - -int call_env_parse(TALLOC_CTX *ctx, call_env_parsed_head_t *parsed, char const *name, fr_dict_t const *dict_def, - CONF_SECTION const *cs, call_env_parser_t const *call_env) CC_HINT(nonnull); +unlang_action_t call_env_expand(TALLOC_CTX *ctx, request_t *request, call_env_result_t *result, void **env_data, call_env_t const *call_env); -size_t call_env_count(size_t *names_len, CONF_SECTION const *cs, call_env_parser_t const *call_env); +call_env_t *call_env_alloc(TALLOC_CTX *ctx, char const *name, call_env_method_t const *call_env_method, + fr_dict_t const *namespace, CONF_SECTION *cs) CC_HINT(nonnull(3,4,5)); #ifdef __cplusplus } diff --git a/src/lib/unlang/compile.c b/src/lib/unlang/compile.c index d91a3fe6c00..c5689ce5886 100644 --- a/src/lib/unlang/compile.c +++ b/src/lib/unlang/compile.c @@ -4399,8 +4399,6 @@ static unlang_t *compile_module(unlang_t *parent, unlang_compile_t *unlang_ctx, MEM(single = talloc_zero(parent, unlang_module_t)); single->instance = inst; single->method = method; - single->method_env = method_env; - c = unlang_module_to_generic(single); c->parent = parent; c->next = NULL; @@ -4424,38 +4422,18 @@ static unlang_t *compile_module(unlang_t *parent, unlang_compile_t *unlang_ctx, * Parse the method environment for this module / method */ if (method_env) { - size_t count, names_len = 0; - - /* - * Firstly assess how many parsed env there will be and create a talloc pool to hold them. - * The pool size is a rough estimate based on each tmpl also allocating at least two children, - * for which we allow twice the length of the value to be parsed. - */ - count = call_env_count(&names_len, inst->dl_inst->conf, method_env->env); - - /* - * Pre-allocated headers: - * 1 header for the call_env_parsed_t, 1 header for the tmpl_t, 1 header for the name, - * one header for the value. - * - * Pre-allocated memory: - * ((sizeof(call_env_parsed_t) + sizeof(tmpl_t)) * count) + (names of tmpls * 2)... Not sure what - * the * 2 is for, maybe for slop? - */ - MEM(single->call_env_ctx = _talloc_pooled_object(single, 0, "call_env_ctx", count * 4, - ((sizeof(call_env_parsed_t) + sizeof(tmpl_t)) * count) + (names_len * 2))); + fr_assert_msg(method_env->inst_size, "Method environment for module %s, method %s %s declared, " + "but no inst_size set", + inst->module->name, unlang_ctx->section_name1, unlang_ctx->section_name2); - call_env_parsed_init(&single->call_env_parsed); - if (call_env_parse(single->call_env_ctx, &single->call_env_parsed, single->self.name, - unlang_ctx->rules ? unlang_ctx->rules->attr.dict_def : fr_dict_internal(), - inst->dl_inst->conf, method_env->env) < 0) { + single->call_env = call_env_alloc(single, single->self.name, method_env, + unlang_ctx->rules ? unlang_ctx->rules->attr.dict_def : fr_dict_internal(), + inst->dl_inst->conf); + if (!single->call_env) { error: talloc_free(c); return NULL; } - fr_assert_msg(method_env->inst_size, "Method environment for module %s, method %s %s declared, " - "but no inst_size set", - inst->module->name, unlang_ctx->section_name1, unlang_ctx->section_name2); } /* diff --git a/src/lib/unlang/module.c b/src/lib/unlang/module.c index 521544c9bb6..a082420ec38 100644 --- a/src/lib/unlang/module.c +++ b/src/lib/unlang/module.c @@ -911,9 +911,9 @@ static unlang_action_t unlang_module(rlm_rcode_t *p_result, request_t *request, goto done; } - if (mc->method_env) { + if (mc->call_env) { if (!state->env_data) { - ua = call_env_expand(state, request, &state->env_result, &state->env_data, mc->method_env, &mc->call_env_parsed); + ua = call_env_expand(state, request, &state->env_result, &state->env_data, mc->call_env); switch (ua) { case UNLANG_ACTION_FAIL: goto fail; diff --git a/src/lib/unlang/module_priv.h b/src/lib/unlang/module_priv.h index 64fe63f2dd5..0b3e9532c1e 100644 --- a/src/lib/unlang/module_priv.h +++ b/src/lib/unlang/module_priv.h @@ -36,10 +36,7 @@ typedef struct { unlang_t self; //!< Common fields in all #unlang_t tree nodes. module_instance_t *instance; //!< Global instance of the module we're calling. module_method_t method; //!< The entry point into the module. - call_env_method_t const *method_env; //!< Call environment for this method. - call_env_parsed_head_t call_env_parsed; //!< The per call parsed call environment. - TALLOC_CTX *call_env_ctx; //!< A talloc pooled object for parsed call env - ///< to be allocated from. + call_env_t const *call_env; //!< The per call parsed call environment. } unlang_module_t; /** A module stack entry diff --git a/src/lib/unlang/xlat.c b/src/lib/unlang/xlat.c index f915e2151af..53fa3d7d446 100644 --- a/src/lib/unlang/xlat.c +++ b/src/lib/unlang/xlat.c @@ -306,10 +306,9 @@ static unlang_action_t unlang_xlat_repeat(rlm_rcode_t *p_result, request_t *requ /* * If the xlat is a function with a method_env, expand it before calling the function. */ - if ((state->exp->type == XLAT_FUNC) && state->exp->call.func->call_env && !state->env_data) { + if ((state->exp->type == XLAT_FUNC) && state->exp->call.inst->call_env && !state->env_data) { unlang_action_t ua = call_env_expand(state, request, NULL, &state->env_data, - state->exp->call.func->call_env, - &state->exp->call.inst->call_env_parsed); + state->exp->call.inst->call_env); switch (ua) { case UNLANG_ACTION_FAIL: goto fail; diff --git a/src/lib/unlang/xlat.h b/src/lib/unlang/xlat.h index 9aa8d432592..c2a812c1504 100644 --- a/src/lib/unlang/xlat.h +++ b/src/lib/unlang/xlat.h @@ -48,8 +48,8 @@ typedef enum { XLAT_INPUT_ARGS //!< Ingests a number of arguments } xlat_input_type_t; -typedef struct xlat_inst xlat_inst_t; -typedef struct xlat_thread_inst xlat_thread_inst_t; +typedef struct xlat_inst_s xlat_inst_t; +typedef struct xlat_thread_inst_s xlat_thread_inst_t; #include @@ -71,7 +71,7 @@ typedef size_t (*xlat_escape_legacy_t)(request_t *request, char *out, size_t out /** Instance data for an xlat expansion node * */ -struct xlat_inst { +struct xlat_inst_s { fr_heap_index_t idx; //!< Entry in heap of xlat instances. ///< Identical instances are used for ///< global instance data and thread-specific @@ -79,15 +79,13 @@ struct xlat_inst { xlat_exp_t *node; //!< Node this data relates to. void *data; //!< xlat node specific instance data. - call_env_parsed_head_t call_env_parsed; //!< The per call parsed environment. - TALLOC_CTX *call_env_ctx; //!< A talloc pooled object for parsed call env - ///< to be allocated from. + call_env_t const *call_env; //!< Per call environment. }; /** Thread specific instance data for xlat expansion node * */ -struct xlat_thread_inst { +struct xlat_thread_inst_s { fr_heap_index_t idx; //!< Entry in heap of xlat thread instances. ///< Identical instances are used for ///< global instance data and thread-specific diff --git a/src/lib/unlang/xlat_func.c b/src/lib/unlang/xlat_func.c index c30f65762fb..8746f519d6a 100644 --- a/src/lib/unlang/xlat_func.c +++ b/src/lib/unlang/xlat_func.c @@ -391,11 +391,11 @@ int xlat_func_mono_set(xlat_t *x, xlat_arg_parser_t const args[]) /** Register call environment of an xlat * * @param[in,out] x to have it's module method env registered. - * @param[in] env to be registered. + * @param[in] env_method to be registered. */ -void xlat_func_call_env_set(xlat_t *x, call_env_method_t const *env) +void xlat_func_call_env_set(xlat_t *x, call_env_method_t const *env_method) { - x->call_env = env; + x->call_env_method = env_method; } /** Specify flags that alter the xlat's behaviour diff --git a/src/lib/unlang/xlat_inst.c b/src/lib/unlang/xlat_inst.c index 0a1d60e58bc..31734300d0b 100644 --- a/src/lib/unlang/xlat_inst.c +++ b/src/lib/unlang/xlat_inst.c @@ -248,22 +248,17 @@ static xlat_inst_t *xlat_inst_alloc(xlat_exp_t *node) /* * If the xlat has a call env defined, parse it. */ - if (call->func->call_env) { - size_t count, names_len = 0; - CONF_SECTION *cs = call->func->mctx->inst->conf; - call_env_method_t const *call_env = call->func->call_env; - - count = call_env_count(&names_len, cs, call_env->env); - MEM(xi->call_env_ctx = _talloc_pooled_object(xi, 0, "call_env_ctx", count * 4, - (sizeof(call_env_parsed_t) + sizeof(tmpl_t)) * count + names_len * 2)); - call_env_parsed_init(&xi->call_env_parsed); - if (call_env_parse(xi->call_env_ctx, &xi->call_env_parsed, call->func->mctx->inst->name, - call->dict, call->func->mctx->inst->conf, call_env->env) < 0) { + if (call->func->call_env_method) { + fr_assert_msg(call->func->call_env_method->inst_size, + "Method environment for module %s, xlat %s declared, " + "but no inst_size set", call->func->mctx->inst->name, call->func->name); + + xi->call_env = call_env_alloc(xi, call->func->name, call->func->call_env_method, + call->dict, call->func->mctx->inst->conf); + if (!xi->call_env) { talloc_free(xi); return NULL; } - fr_assert_msg(call_env->inst_size, "Method environment for module %s, xlat %s declared, " - "but no inst_size set", call->func->mctx->inst->name, call->func->name); } return xi; diff --git a/src/lib/unlang/xlat_priv.h b/src/lib/unlang/xlat_priv.h index 93458810550..47055089117 100644 --- a/src/lib/unlang/xlat_priv.h +++ b/src/lib/unlang/xlat_priv.h @@ -88,7 +88,7 @@ typedef struct xlat_s { xlat_input_type_t input_type; //!< Type of input used. xlat_arg_parser_t const *args; //!< Definition of args consumed. - call_env_method_t const *call_env; //!< Optional tmpl expansions performed before calling the + call_env_method_t const *call_env_method; //!< Optional tmpl expansions performed before calling the ///< xlat. Typically used for xlats which refer to tmpls ///< in their module config.