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 e4ac338d4f951937e147a31967e4961b0395f96a..dc53b5ade09f3c127c31fe40de14b379bab9e197 100644 (file)
--- 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 743ed59347f35935d130d4b1731b9dd1e95cb750..a0ce6d8cdca441357c077cb9b337f03915876d1e 100644 (file)
--- 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 93e3057d46f6462eaf3d8714234ca60ddf9c800f..6bacd0ccd0a66e89592d46741b203e94ee99667f 100644 (file)
--- 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 18b874b444f72a9b7b844e16f53e77eae8e7d08e..c1986c3839b2e914c552ffde9b89f2f78e4ca893 100644 (file)
--- 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 a46e59970476fdf737e41b050f93ede8f670fa1b..1bc0b327735fbb7c171aa374c06ea434653fb236 100644 (file)
--- 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 af0871cc7323d08f59a57a634071c1817009ad39..97dae3e2bf0387fbd5b64df6a80256f075a11731 100644 (file)
--- 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, "<INVALID>"), *priority);
+                       fr_table_str_by_value(unlang_action_table, ua, "<INVALID>"), *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 53bb27c4199c735ce017065c626c4e1ed2aff4ea..c0324abd4d724b83e93f72125815a82afc0e76d9 100644 (file)
--- 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 103ecb41991833382f430fd34c7a043abe69dd39..ec6ece1eb919ed9a58bca953358dd00b72f5a3bb 100644 (file)
--- 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 958988ca33d8d2777222ed3b3db6c9e8216fda80..8b04ec41664d4dd88ec6a6627bdeec2fdb823681 100644 (file)
--- 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 c393b82998e087afb5983b2f2924d0b5a2572ce2..bd8785714a93d9e7472296152110c3acef8b892e 100644 (file)
--- 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 8c386c4174701dfc65eac59552007c6176b1c895..4bf40a7fb2b397616b405a6782429af521fd109f 100644 (file)
--- 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 e4f4273e84de3537da81e31bfa3322c14205d118..a3d28652a64246344210703eb2b55910f4bdad1c 100644 (file)
--- 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 39e04d05a2e9ab80c66ca94bc95edc152e29bc3c..5327428bd08269b4ea45b56a4f6ba6f7d0d67da3 100644 (file)
--- 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 36ac57d706b1b962d6dd9eb71b72b09cfe9be6ca..099edc1f62d8ce6957cfaee202f6d44aa8898289 100644 (file)
--- 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 3cdee0399227b0a4045864818e822bfff1396ee3..f35bb883c354dacd798e894436e89574868ece51 100644 (file)
--- 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