From: Arran Cudbard-Bell Date: Sun, 6 Jun 2021 00:27:24 +0000 (-0500) Subject: Add UNLANG_ACTION_FAIL to make it easier to deal with common unlang functions failing X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=15acec4aadfb16ddd46b4f669f9530c4f07cbbe8;p=thirdparty%2Ffreeradius-server.git Add UNLANG_ACTION_FAIL to make it easier to deal with common unlang functions failing It lets us return an unlang_action_t from push functions, which results in less boilerplate. --- diff --git a/src/lib/eap/tls.c b/src/lib/eap/tls.c index e4ac338d4f9..dc53b5ade09 100644 --- a/src/lib/eap/tls.c +++ b/src/lib/eap/tls.c @@ -957,10 +957,10 @@ static unlang_action_t eap_tls_handshake(rlm_rcode_t *p_result, int *priority, */ static inline CC_HINT(always_inline) unlang_action_t eap_tls_handshake_push(request_t *request, eap_session_t *eap_session) { - if (unlang_interpret_push_function(request, - eap_tls_handshake, - eap_tls_handshake_resume, - NULL, UNLANG_SUB_FRAME, eap_session) < 0) return UNLANG_ACTION_STOP_PROCESSING; + if (unlang_function_push(request, + eap_tls_handshake, + eap_tls_handshake_resume, + NULL, UNLANG_SUB_FRAME, eap_session) < 0) return UNLANG_ACTION_STOP_PROCESSING; return UNLANG_ACTION_PUSHED_CHILD; } diff --git a/src/lib/server/trigger.c b/src/lib/server/trigger.c index 743ed59347f..a0ce6d8cdca 100644 --- a/src/lib/server/trigger.c +++ b/src/lib/server/trigger.c @@ -434,7 +434,7 @@ int trigger_exec(unlang_interpret_t *intp, request_t *request, } } - if (unlang_interpret_push_function(child, trigger_run, trigger_resume, + if (unlang_function_push(child, trigger_run, trigger_resume, NULL, UNLANG_TOP_FRAME, trigger) < 0) goto error; if (!intp) { diff --git a/src/lib/unlang/action.h b/src/lib/unlang/action.h index 93e3057d46f..6bacd0ccd0a 100644 --- a/src/lib/unlang/action.h +++ b/src/lib/unlang/action.h @@ -33,6 +33,7 @@ extern "C" { * These deal exclusively with control flow. */ typedef enum { + UNLANG_ACTION_FAIL = -1, //!< Encountered an unexpected error. UNLANG_ACTION_CALCULATE_RESULT = 1, //!< Calculate a new section #rlm_rcode_t value. UNLANG_ACTION_EXECUTE_NEXT, //!< Execute the next #unlang_t. UNLANG_ACTION_PUSHED_CHILD, //!< #unlang_t pushed a new child onto the stack, diff --git a/src/lib/unlang/function.c b/src/lib/unlang/function.c index 18b874b444f..c1986c3839b 100644 --- a/src/lib/unlang/function.c +++ b/src/lib/unlang/function.c @@ -80,8 +80,6 @@ static unlang_action_t unlang_function_call_repeat(rlm_rcode_t *p_result, reques unlang_frame_state_func_t *state = talloc_get_type_abort(frame->state, unlang_frame_state_func_t); char const *caller; - *p_result = RLM_MODULE_NOOP; /* Avoid asserts */ - /* * Don't let the callback mess with the current * module permanently. @@ -107,8 +105,6 @@ static unlang_action_t unlang_function_call(rlm_rcode_t *p_result, request_t *re unlang_frame_state_func_t *state = talloc_get_type_abort(frame->state, unlang_frame_state_func_t); char const *caller; - *p_result = RLM_MODULE_NOOP; /* Avoid asserts */ - /* * Don't let the callback mess with the current * module permanently. @@ -149,6 +145,68 @@ static unlang_action_t unlang_function_call(rlm_rcode_t *p_result, request_t *re return ua; } +/** Clear pending repeat function calls, and remove the signal handler. + * + * The function frame being modified must be at the top of the stack. + * + * @param[in] request The current request. + * @return + * - 0 on success. + * - -1 on failure. + */ +int unlang_function_clear(request_t *request) +{ + unlang_stack_t *stack = request->stack; + unlang_stack_frame_t *frame = &stack->frame[stack->depth]; + unlang_frame_state_func_t *state; + + if (frame->instruction->type != UNLANG_TYPE_FUNCTION) { + RERROR("Can't clear function on non-function frame"); + return -1; + } + + state = talloc_get_type_abort(frame->state, unlang_frame_state_func_t); + state->repeat = NULL; + state->signal = NULL; + + repeatable_clear(frame); + + return 0; +} + +/** Set a new repeat function for an existing function frame + * + * The function frame being modified must be at the top of the stack. + * + * @param[in] request The current request. + * @param[in] repeat the repeat function to set. + * @return + * - 0 on success. + * - -1 on failure. + */ +int unlang_function_repeat(request_t *request, unlang_function_t repeat) +{ + unlang_stack_t *stack = request->stack; + unlang_stack_frame_t *frame = &stack->frame[stack->depth]; + unlang_frame_state_func_t *state; + + if (frame->instruction->type != UNLANG_TYPE_FUNCTION) { + RERROR("Can't set repeat function on non-function frame"); + return -1; + } + + state = talloc_get_type_abort(frame->state, unlang_frame_state_func_t); + + /* + * If we're inside unlang_function_call, + * it'll pickup state->repeat and do the right thing + * once the current function returns. + */ + state->repeat = repeat; + + return 0; +} + /** Push a generic function onto the unlang stack * * These can be pushed by any other type of unlang op to allow a submodule or function @@ -164,8 +222,8 @@ static unlang_action_t unlang_function_call(rlm_rcode_t *p_result, request_t *re * - 0 on success. * - -1 on failure. */ -int unlang_interpret_push_function(request_t *request, unlang_function_t func, unlang_function_t repeat, - unlang_function_signal_t signal, bool top_frame, void *uctx) +unlang_action_t unlang_function_push(request_t *request, unlang_function_t func, unlang_function_t repeat, + unlang_function_signal_t signal, bool top_frame, void *uctx) { unlang_stack_t *stack = request->stack; unlang_stack_frame_t *frame; @@ -175,7 +233,7 @@ int unlang_interpret_push_function(request_t *request, unlang_function_t func, u * Push module's function */ if (unlang_interpret_push(request, &function_instruction, - RLM_MODULE_UNKNOWN, UNLANG_NEXT_STOP, top_frame) < 0) return -1; + RLM_MODULE_NOOP, UNLANG_NEXT_STOP, top_frame) < 0) return UNLANG_ACTION_FAIL; frame = &stack->frame[stack->depth]; @@ -199,7 +257,7 @@ int unlang_interpret_push_function(request_t *request, unlang_function_t func, u */ if (!func && repeat) frame->process = unlang_function_call_repeat; - return 0; + return UNLANG_ACTION_PUSHED_CHILD; } void unlang_function_init(void) diff --git a/src/lib/unlang/function.h b/src/lib/unlang/function.h index a46e5997047..1bc0b327735 100644 --- a/src/lib/unlang/function.h +++ b/src/lib/unlang/function.h @@ -21,6 +21,10 @@ * @file unlang/function.h * @brief Declarations for generic unlang functions. * + * These are a useful alternative to module methods for library code. + * They're more light weight, and don't require instance data lookups + * to function. + * * @copyright 2021 The FreeRADIUS server project */ @@ -52,10 +56,14 @@ typedef unlang_action_t (*unlang_function_t)(rlm_rcode_t *p_result, int *priorit */ typedef void (*unlang_function_signal_t)(request_t *request, fr_state_signal_t action, void *uctx); -int unlang_interpret_push_function(request_t *request, - unlang_function_t func, unlang_function_t repeat, - unlang_function_signal_t signal, bool top_frame, void *uctx) - CC_HINT(warn_unused_result); +int unlang_function_clear(request_t *request) CC_HINT(warn_unused_result); + +int unlang_function_repeat(request_t *request, unlang_function_t repeat) CC_HINT(warn_unused_result); + +unlang_action_t unlang_function_push(request_t *request, + unlang_function_t func, unlang_function_t repeat, + unlang_function_signal_t signal, bool top_frame, void *uctx) + CC_HINT(warn_unused_result); #ifdef __cplusplus } diff --git a/src/lib/unlang/interpret.c b/src/lib/unlang/interpret.c index af0871cc732..97dae3e2bf0 100644 --- a/src/lib/unlang/interpret.c +++ b/src/lib/unlang/interpret.c @@ -327,7 +327,7 @@ unlang_frame_action_t frame_eval(request_t *request, unlang_stack_frame_t *frame */ while (frame->instruction) { unlang_t const *instruction = frame->instruction; - unlang_action_t action = UNLANG_ACTION_UNWIND; + unlang_action_t ua = UNLANG_ACTION_UNWIND; DUMP_STACK; @@ -385,15 +385,15 @@ unlang_frame_action_t frame_eval(request_t *request, unlang_stack_frame_t *frame * should be evaluated again. */ repeatable_clear(frame); - action = frame->process(result, request, frame); + ua = frame->process(result, request, frame); RDEBUG4("** [%i] %s << %s (%d)", stack->depth, __FUNCTION__, - fr_table_str_by_value(unlang_action_table, action, ""), *priority); + fr_table_str_by_value(unlang_action_table, ua, ""), *priority); fr_assert(*priority >= -1); fr_assert(*priority <= MOD_PRIORITY_MAX); - switch (action) { + switch (ua) { /* * The request is now defunct, and we should not * continue processing it. @@ -435,6 +435,14 @@ unlang_frame_action_t frame_eval(request_t *request, unlang_stack_frame_t *frame DUMP_STACK; return UNLANG_FRAME_ACTION_YIELD; + /* + * This action is intended to be returned by library + * functions. It reduces the boilerplate. + */ + case UNLANG_ACTION_FAIL: + *result = RLM_MODULE_FAIL; + FALL_THROUGH; + /* * Instruction finished execution, * check to see what we need to do next, and update @@ -470,7 +478,7 @@ unlang_frame_action_t frame_eval(request_t *request, unlang_stack_frame_t *frame * Execute the next instruction in this frame */ case UNLANG_ACTION_EXECUTE_NEXT: - if ((action == UNLANG_ACTION_EXECUTE_NEXT) && unlang_ops[instruction->type].debug_braces) { + if ((ua == UNLANG_ACTION_EXECUTE_NEXT) && unlang_ops[instruction->type].debug_braces) { REXDENT(); RDEBUG2("}"); } diff --git a/src/lib/unlang/module.c b/src/lib/unlang/module.c index 53bb27c4199..c0324abd4d7 100644 --- a/src/lib/unlang/module.c +++ b/src/lib/unlang/module.c @@ -407,19 +407,6 @@ int unlang_module_push(rlm_rcode_t *p_result, request_t *request, return 0; } -/** Allocate a subrequest to run through a virtual server at some point in the future - * - * @param[in] parent to hang sub request off of. - * @param[in] namespace the child will operate in. - * @return - * - A new child request. - * - NULL on failure. - */ -request_t *unlang_module_subrequest_alloc(request_t *parent, fr_dict_t const *namespace) -{ - return unlang_io_subrequest_alloc(parent, namespace, UNLANG_NORMAL_CHILD); -} - /** Push a pre-compiled xlat and resumption state onto the stack for evaluation * * In order to use the async unlang processor the calling module needs to establish @@ -798,6 +785,10 @@ static unlang_action_t unlang_module_resume(rlm_rcode_t *p_result, request_t *re case UNLANG_ACTION_UNWIND: break; + case UNLANG_ACTION_FAIL: + *p_result = RLM_MODULE_FAIL; + break; + case UNLANG_ACTION_EXECUTE_NEXT: /* Not valid */ fr_assert(0); *p_result = RLM_MODULE_FAIL; @@ -940,6 +931,10 @@ static unlang_action_t unlang_module(rlm_rcode_t *p_result, request_t *request, case UNLANG_ACTION_UNWIND: break; + case UNLANG_ACTION_FAIL: + *p_result = RLM_MODULE_FAIL; + break; + case UNLANG_ACTION_EXECUTE_NEXT: fr_assert(0); *p_result = RLM_MODULE_FAIL; diff --git a/src/lib/unlang/module.h b/src/lib/unlang/module.h index 103ecb41991..ec6ece1eb91 100644 --- a/src/lib/unlang/module.h +++ b/src/lib/unlang/module.h @@ -112,8 +112,6 @@ int unlang_module_push(rlm_rcode_t *p_result, request_t *request, module_instance_t *module_instance, module_method_t method, bool top_frame) CC_HINT(warn_unused_result); -request_t *unlang_module_subrequest_alloc(request_t *parent, fr_dict_t const *namespace); - unlang_action_t unlang_module_yield_to_subrequest(rlm_rcode_t *p_result, request_t *child, unlang_module_resume_t resume, unlang_module_signal_t signal, diff --git a/src/lib/unlang/parallel.c b/src/lib/unlang/parallel.c index 958988ca33d..8b04ec41664 100644 --- a/src/lib/unlang/parallel.c +++ b/src/lib/unlang/parallel.c @@ -365,12 +365,12 @@ static unlang_action_t unlang_parallel_process(rlm_rcode_t *p_result, request_t } else { unlang_stack_frame_t *child_frame; - if (unlang_interpret_push_function(child, - NULL, - unlang_parallel_child_done, - unlang_parallel_child_signal, - UNLANG_TOP_FRAME, - &state->children[i]) < 0) goto error; + if (unlang_function_push(child, + NULL, + unlang_parallel_child_done, + unlang_parallel_child_signal, + UNLANG_TOP_FRAME, + &state->children[i]) < 0) goto error; child_frame = frame_current(child); return_point_set(child_frame); /* Don't unwind this frame */ diff --git a/src/lib/unlang/subrequest.c b/src/lib/unlang/subrequest.c index c393b82998e..bd8785714a9 100644 --- a/src/lib/unlang/subrequest.c +++ b/src/lib/unlang/subrequest.c @@ -80,7 +80,7 @@ static unlang_action_t unlang_subrequest_parent_resume(rlm_rcode_t *p_result, re * If there's a no destination tmpl, we're done. */ if (!child->reply) { - unlang_subrequest_free(&child); + unlang_subrequest_detach_and_free(&child); return UNLANG_ACTION_CALCULATE_RESULT; } @@ -114,7 +114,7 @@ static unlang_action_t unlang_subrequest_parent_resume(rlm_rcode_t *p_result, re } } - unlang_subrequest_free(&child); + unlang_subrequest_detach_and_free(&child); return UNLANG_ACTION_CALCULATE_RESULT; } @@ -254,13 +254,26 @@ static unlang_action_t unlang_subrequest_parent_init(rlm_rcode_t *p_result, requ * * @param[in] child to free. */ -void unlang_subrequest_free(request_t **child) +void unlang_subrequest_detach_and_free(request_t **child) { request_detach(*child); talloc_free(*child); *child = NULL; } +/** Allocate a subrequest to run through a virtual server at some point in the future + * + * @param[in] parent to hang sub request off of. + * @param[in] namespace the child will operate in. + * @return + * - A new child request. + * - NULL on failure. + */ +request_t *unlang_subrequest_alloc(request_t *parent, fr_dict_t const *namespace) +{ + return unlang_io_subrequest_alloc(parent, namespace, UNLANG_NORMAL_CHILD); +} + /** Initialise subrequest ops * */ diff --git a/src/lib/unlang/subrequest.h b/src/lib/unlang/subrequest.h index 8c386c41747..4bf40a7fb2b 100644 --- a/src/lib/unlang/subrequest.h +++ b/src/lib/unlang/subrequest.h @@ -37,6 +37,10 @@ typedef struct { int unique_int; //!< Session unique int identifier. } unlang_subrequest_session_t; +request_t *unlang_subrequest_alloc(request_t *parent, fr_dict_t const *namespace); + +void unlang_subrequest_detach_and_free(request_t **child); + int unlang_subrequest_lifetime_set(request_t *request); int unlang_subrequest_child_push(rlm_rcode_t *out, request_t *child, diff --git a/src/lib/unlang/subrequest_child.c b/src/lib/unlang/subrequest_child.c index e4f4273e84d..a3d28652a64 100644 --- a/src/lib/unlang/subrequest_child.c +++ b/src/lib/unlang/subrequest_child.c @@ -205,7 +205,7 @@ int unlang_subrequest_child_push_resume(request_t *child, unlang_frame_state_sub /* * Push a resume frame into the child */ - if (unlang_interpret_push_function(child, NULL, + if (unlang_function_push(child, NULL, unlang_subrequest_child_done, unlang_subrequest_child_signal, UNLANG_TOP_FRAME, state) < 0) return -1; @@ -274,7 +274,7 @@ unlang_action_t unlang_subrequest_child_run(UNUSED rlm_rcode_t *p_result, UNUSED * The child *MUST* have been allocated with unlang_io_subrequest_alloc, or something * that calls it. * - * After the child is no longer required it *MUST* be freed with #unlang_subrequest_free. + * After the child is no longer required it *MUST* be freed with #unlang_subrequest_detach_and_free. * It's not enough to free it with talloc_free. * * This function should be called _before_ pushing any additional frames onto the child's diff --git a/src/lib/unlang/subrequest_priv.h b/src/lib/unlang/subrequest_priv.h index 39e04d05a2e..5327428bd08 100644 --- a/src/lib/unlang/subrequest_priv.h +++ b/src/lib/unlang/subrequest_priv.h @@ -73,8 +73,6 @@ static inline unlang_group_t *unlang_subrequest_to_group(unlang_subrequest_t *su return (unlang_group_t *)subrequest; } -void unlang_subrequest_free(request_t **child); - int unlang_subrequest_detach_child(request_t *request); #ifdef __cplusplus diff --git a/src/modules/rlm_eap/rlm_eap.c b/src/modules/rlm_eap/rlm_eap.c index 36ac57d706b..099edc1f62d 100644 --- a/src/modules/rlm_eap/rlm_eap.c +++ b/src/modules/rlm_eap/rlm_eap.c @@ -733,7 +733,7 @@ static unlang_action_t eap_method_select(rlm_rcode_t *p_result, module_ctx_t con /* * Allocate a new subrequest */ - MEM(eap_session->subrequest = unlang_module_subrequest_alloc(request, + MEM(eap_session->subrequest = unlang_subrequest_alloc(request, method->submodule->namespace ? *(method->submodule->namespace) : request->dict)); diff --git a/src/modules/rlm_eap/types/rlm_eap_tls/rlm_eap_tls.c b/src/modules/rlm_eap/types/rlm_eap_tls/rlm_eap_tls.c index 3cdee039922..f35bb883c35 100644 --- a/src/modules/rlm_eap/types/rlm_eap_tls/rlm_eap_tls.c +++ b/src/modules/rlm_eap/types/rlm_eap_tls/rlm_eap_tls.c @@ -171,10 +171,8 @@ static unlang_action_t eap_tls_virtual_server(rlm_rcode_t *p_result, rlm_eap_tls /* * Catch the interpreter on the way back up the stack */ - if (unlang_interpret_push_function(request, NULL, eap_tls_virtual_server_result, NULL, - UNLANG_SUB_FRAME, eap_session) < 0) { - RETURN_MODULE_FAIL; - } + if (unlang_function_push(request, NULL, eap_tls_virtual_server_result, NULL, + UNLANG_SUB_FRAME, eap_session) < 0) RETURN_MODULE_FAIL; /* * Push unlang instructions for the virtual server section