Make discarding function rcodes official

diff --git a/src/lib/unlang/function.c b/src/lib/unlang/function.c
index 623763810aeb779100811ac51b42f851d978d9c2..a0da288df1c74cb717334db5d115b0eb260dc1b4 100644 (file)
--- a/src/lib/unlang/function.c
+++ b/src/lib/unlang/function.c
@@ -50,16 +50,24 @@ static unlang_t function_instruction = {
        .name = "function",
        .debug_name = "function",
        .actions = {
+               /*
+                *      By default, functions don't change the section rcode.
+                *      We can't make generalisations about what the intent
+                *      of the function callbacks are, so isntead of having
+                *      implicit, confusing behaviour, we always discard the
+                *      rcode UNLESS the function explicitly sets it.
+                */
                .actions = {
-                       [RLM_MODULE_REJECT]     = 0,
-                       [RLM_MODULE_FAIL]       = 0,
-                       [RLM_MODULE_OK]         = 0,
-                       [RLM_MODULE_HANDLED]    = 0,
-                       [RLM_MODULE_INVALID]    = 0,
-                       [RLM_MODULE_DISALLOW]   = 0,
-                       [RLM_MODULE_NOTFOUND]   = 0,
-                       [RLM_MODULE_NOOP]       = 0,
-                       [RLM_MODULE_UPDATED]    = 0
+                       [RLM_MODULE_REJECT]     = MOD_ACTION_NOT_SET,
+                       [RLM_MODULE_FAIL]       = MOD_ACTION_NOT_SET,
+                       [RLM_MODULE_OK]         = MOD_ACTION_NOT_SET,
+                       [RLM_MODULE_HANDLED]    = MOD_ACTION_NOT_SET,
+                       [RLM_MODULE_INVALID]    = MOD_ACTION_NOT_SET,
+                       [RLM_MODULE_DISALLOW]   = MOD_ACTION_NOT_SET,
+                       [RLM_MODULE_NOTFOUND]   = MOD_ACTION_NOT_SET,
+                       [RLM_MODULE_NOOP]       = MOD_ACTION_NOT_SET,
+                       [RLM_MODULE_UPDATED]    = MOD_ACTION_NOT_SET,
+                       [RLM_MODULE_TIMEOUT]    = MOD_ACTION_NOT_SET
                },
                .retry = RETRY_INIT,
        }

diff --git a/src/lib/unlang/function.h b/src/lib/unlang/function.h
index 4c327b4dd1e8b2df6fb19873da2b4838466244fb..80126c8c54d201bbbf69ae39d07c7be9c9e0043f 100644 (file)
--- a/src/lib/unlang/function.h
+++ b/src/lib/unlang/function.h
@@ -96,7 +96,16 @@ int          _unlang_function_repeat_set(request_t *request, unlang_function_t repeat, c
  * These can be pushed by any other type of unlang op to allow a submodule or function
  * deeper in the C call stack to establish a new resumption point.
  *
- * @param[in] _reuslt_p                Where to write the result.
+ * @note If you're pushing a function onto the stack to resume execution in a module, you're probably
+ *      doing it wrong.  Use unlang_module_yield() instead, and change the process function for the
+ *      module.
+ *
+ * @note By default the rcodes from functions will be discarded.  This can be altered on a per-function
+ *       basis by the func or repeat functions setting result_p->priority to a non-zero value.
+ *      Results can also be collected by passing in a non-NULL _result_p pointer.
+ *      You may disagree with this behaviour, but you're wrong.
+ *
+ * @param[in] _result_p                Where to write the result.
  * @param[in] _request         The current request.
  * @param[in] _func            to call going up the stack.
  * @param[in] _repeat          function to call going back down the stack (may be NULL).

diff --git a/src/lib/unlang/interpret.c b/src/lib/unlang/interpret.c
index 8037842395482bfb82ff7dd61d29a3cffd6533d9..bf1857b125ace63d274ea8203c80952ffcec9832 100644 (file)
--- a/src/lib/unlang/interpret.c
+++ b/src/lib/unlang/interpret.c
@@ -455,23 +455,25 @@ unlang_frame_action_t result_calculate(request_t *request, unlang_stack_frame_t
         *      This is the field that's evaluated in unlang conditions
         *      like `if (ok)`.
         */
-       if (frame->instruction && is_rcode_set(frame) && (request->rcode != result->rcode)) {
-               RDEBUG3("Setting request->rcode to '%s'",
-                       fr_table_str_by_value(rcode_table, result->rcode, "<INVALID>"));
-               request->rcode = result->rcode;
-       }
+       if (frame->instruction) {
+               if (is_rcode_set(frame) && (request->rcode != result->rcode)) {
+                       RDEBUG3("Setting request->rcode to '%s'",
+                               fr_table_str_by_value(rcode_table, result->rcode, "<INVALID>"));
+                       request->rcode = result->rcode;
+               }
 
-       /*
-        *      The array holds a default priority for this return
-        *      code.  Grab it in preference to any unset priority.
-        */
-       if (result->priority == MOD_ACTION_NOT_SET) {
-               result->priority = instruction->actions.actions[result->rcode];
+               /*
+               *       The array holds a default priority for this return
+               *       code.  Grab it in preference to any unset priority.
+               */
+               if (result->priority == MOD_ACTION_NOT_SET) {
+                       result->priority = instruction->actions.actions[result->rcode];
 
-               RDEBUG4("** [%i] %s - using default instruction priority for %s, %d",
-                       stack->depth, __FUNCTION__,
-                       fr_table_str_by_value(mod_rcode_table, result->rcode, "<invalid>"),
-                       result->priority);
+                       RDEBUG4("** [%i] %s - using default instruction priority for %s, %d",
+                               stack->depth, __FUNCTION__,
+                               fr_table_str_by_value(mod_rcode_table, result->rcode, "<invalid>"),
+                               result->priority);
+               }
        }
 
        /*