]> git.ipfire.org Git - thirdparty/freeradius-server.git/commitdiff
move MOD_ACTION_NOT_SET --> 0
authorAlan T. DeKok <aland@freeradius.org>
Tue, 15 Jul 2025 18:45:03 +0000 (14:45 -0400)
committerAlan T. DeKok <aland@freeradius.org>
Wed, 16 Jul 2025 12:19:32 +0000 (08:19 -0400)
add macros for valid values.  sprinkle assertions throughout the
code.  Move the action names to a fixed-size array.  Update the
printing to print out the names in all cases (not the numbers)

src/lib/unlang/compile.c
src/lib/unlang/interpret.c
src/lib/unlang/mod_action.c
src/lib/unlang/mod_action.h
src/lib/unlang/parallel.c
src/lib/unlang/unlang_priv.h

index b87356dc26091f8e1c75bf0e6d3a7025836683d3..e9772e7e521a5564f78d0583e4a313ef54ab0650 100644 (file)
@@ -88,14 +88,6 @@ fr_table_num_sorted_t const mod_rcode_table[] = {
 };
 size_t mod_rcode_table_len = NUM_ELEMENTS(mod_rcode_table);
 
-fr_table_num_sorted_t const mod_action_table[] = {
-       { L("..."),             MOD_ACTION_NOT_SET      },
-       { L("reject"),          MOD_ACTION_REJECT       },
-       { L("retry"),           MOD_ACTION_RETRY        },
-       { L("return"),          MOD_ACTION_RETURN       }
-};
-size_t mod_action_table_len = NUM_ELEMENTS(mod_action_table);
-
 #define UPDATE_CTX2 unlang_compile_ctx_copy(&unlang_ctx2, unlang_ctx)
 
 
@@ -971,14 +963,17 @@ static int compile_action_pair(unlang_mod_actions_t *actions, CONF_PAIR *cp)
        else if (!strcasecmp(value, "retry"))
                action = MOD_ACTION_RETRY;
 
-       else if (strspn(value, "0123456789")==strlen(value)) {
-               action = atoi(value);
-
-               if (!action || (action > MOD_PRIORITY_MAX)) {
+       else if (strspn(value, "0123456789") == strlen(value)) {
+               if (strlen(value) > 2) {
+               invalid_action:
                        cf_log_err(cp, "Priorities MUST be between 1 and 64.");
                        return 0;
                }
 
+               action = MOD_PRIORITY(atoi(value));
+
+               if (!MOD_ACTION_VALID_SET(action)) goto invalid_action;
+
        } else {
                cf_log_err(cp, "Unknown action '%s'.\n",
                           value);
index 97a027689651906fc6f3fd47caa179920a7fa046..5885ef0a079222e123c59510a532c32a2d8755c7 100644 (file)
@@ -160,18 +160,14 @@ static void instruction_dump(request_t *request, unlang_t const *instruction)
 
 static void CC_HINT(nonnull) actions_dump(request_t *request, unlang_t const *instruction)
 {
-       char buffer[20];
        int i;
 
        RDEBUG2("actions");
        RINDENT();
        for (i = 0; i < RLM_MODULE_NUMCODES; i++) {
-               snprintf(buffer, sizeof(buffer), "%d", instruction->actions.actions[i]);
-
                RDEBUG2("%s: %s",
                        fr_table_str_by_value(mod_rcode_table, i, "<invalid>"),
-                       fr_table_str_by_value(mod_action_table, instruction->actions.actions[i], buffer),
-                       instruction->actions.actions[i]);
+                       mod_action_name[instruction->actions.actions[i]]);
        }
        REXDENT();
 }
@@ -193,13 +189,15 @@ static void frame_dump(request_t *request, unlang_stack_frame_t *frame, bool wit
                RDEBUG2("next           <none>");
        }
 
+       fr_assert(MOD_ACTION_VALID(frame->result_p->priority));
+
        if (is_private_result(frame)) {
                int location;
                result_p_location_t type;
                void *chunk;
 
                RDEBUG2("p_rcode        %s", fr_table_str_by_value(mod_rcode_table, frame->result_p->rcode, "<invalid>"));
-               RDEBUG2("p_priority     %d", frame->result_p->priority);
+               RDEBUG2("p_priority     %s", mod_action_name[frame->result_p->priority]);
 
                location = find_result_p_location(&type, &chunk, request, frame->result_p);
                RDEBUG2("p_location     %s [%i] %p (%s)", fr_table_str_by_value(result_p_location_table, type, "<invalid>"),
@@ -207,10 +205,10 @@ static void frame_dump(request_t *request, unlang_stack_frame_t *frame, bool wit
                        );
        } else {
                RDEBUG2("sec_rcode      %s", fr_table_str_by_value(mod_rcode_table, frame->section_result.rcode, "<invalid>"));
-               RDEBUG2("sec_priority   %d", frame->section_result.priority);
+               RDEBUG2("sec_priority   %s", mod_action_name[frame->section_result.priority]);
        }
        RDEBUG2("scr_rcode      %s", fr_table_str_by_value(mod_rcode_table, frame->scratch_result.rcode, "<invalid>"));
-       RDEBUG2("scr_priority   %d", frame->scratch_result.priority);
+       RDEBUG2("scr_priority   %s", mod_action_name[frame->scratch_result.priority]);
        RDEBUG2("top_frame      %s", is_top_frame(frame) ? "yes" : "no");
        RDEBUG2("repeat         %s", is_repeatable(frame) ? "yes" : "no");
        RDEBUG2("yielded        %s", is_yielded(frame) ? "yes" : "no");
@@ -468,12 +466,15 @@ unlang_frame_action_t result_calculate(request_t *request, unlang_stack_frame_t
                return UNLANG_FRAME_ACTION_NEXT;
        }
 
-       RDEBUG4("** [%i] %s - have (%s %d) frame or module returned (%s %d)",
+       fr_assert(MOD_ACTION_VALID(frame_result->priority));
+       fr_assert(MOD_ACTION_VALID(result->priority));
+
+       RDEBUG4("** [%i] %s - have (%s %s) frame or module returned (%s %s)",
                stack->depth, __FUNCTION__,
                fr_table_str_by_value(mod_rcode_table, frame_result->rcode, "<invalid>"),
-               frame_result->priority,
+               mod_action_name[frame_result->priority],
                fr_table_str_by_value(mod_rcode_table, result->rcode, "<invalid>"),
-               result->priority);
+               mod_action_name[result->priority]);
 
        /*
         *      Update request->rcode if the instruction says we should
@@ -495,10 +496,12 @@ unlang_frame_action_t result_calculate(request_t *request, unlang_stack_frame_t
        if (result->priority == MOD_ACTION_NOT_SET) {
                result->priority = instruction->actions.actions[result->rcode];
 
-               RDEBUG4("** [%i] %s - using default instruction priority for %s, %d",
+               fr_assert(MOD_ACTION_VALID(result->priority));
+
+               RDEBUG4("** [%i] %s - using default instruction priority for %s, %s",
                        stack->depth, __FUNCTION__,
                        fr_table_str_by_value(mod_rcode_table, result->rcode, "<invalid>"),
-                       result->priority);
+                       mod_action_name[result->priority]);
        }
 
        /*
@@ -512,10 +515,10 @@ unlang_frame_action_t result_calculate(request_t *request, unlang_stack_frame_t
         *      should return from this frame.
         */
        case MOD_ACTION_RETURN:
-               RDEBUG4("** [%i] %s - action says to return with (%s %d)",
+               RDEBUG4("** [%i] %s - action says to return with (%s %s)",
                        stack->depth, __FUNCTION__,
                        fr_table_str_by_value(mod_rcode_table, result->rcode, "<invalid>"),
-                       result->priority);
+                       mod_action_name[result->priority]);
 
                *frame_result = UNLANG_RESULT_RCODE(result->rcode);
                return UNLANG_FRAME_ACTION_POP;
@@ -530,10 +533,10 @@ unlang_frame_action_t result_calculate(request_t *request, unlang_stack_frame_t
         *      after the module returns...
         */
        case MOD_ACTION_REJECT:
-               RDEBUG4("** [%i] %s - action says to return with (%s %d)",
+               RDEBUG4("** [%i] %s - action says to return with (%s %s)",
                        stack->depth, __FUNCTION__,
                        fr_table_str_by_value(mod_rcode_table, RLM_MODULE_REJECT, "<invalid>"),
-                       result->priority);
+                       mod_action_name[result->priority]);
 
                *frame_result = UNLANG_RESULT_RCODE(RLM_MODULE_REJECT);
                return UNLANG_FRAME_ACTION_POP;
@@ -621,12 +624,15 @@ finalize:
         *      return code and priority.
         */
        if (result->priority >= frame_result->priority) {
-               RDEBUG4("** [%i] %s - overwriting existing result (%s %d) with higher priority (%s %d)",
+               fr_assert(MOD_ACTION_VALID(result->priority));
+               fr_assert(MOD_ACTION_VALID(frame_result->priority));
+
+               RDEBUG4("** [%i] %s - overwriting existing result (%s %s) with higher priority (%s %s)",
                        stack->depth, __FUNCTION__,
                        fr_table_str_by_value(mod_rcode_table, frame_result->rcode, "<invalid>"),
-                       frame_result->priority,
+                       mod_action_name[frame_result->priority],
                        fr_table_str_by_value(mod_rcode_table, result->rcode, "<invalid>"),
-                       result->priority);
+                       mod_action_name[result->priority]);
                *frame->result_p = *result;
        }
 
@@ -653,6 +659,8 @@ unlang_frame_action_t result_pop(request_t *request, unlang_stack_frame_t *frame
        unlang_stack_t  *stack = request->stack;
        unlang_result_t our_result = *result;
 
+       fr_assert(MOD_ACTION_VALID(result->priority));
+
        /*
         *      When a stack frame is being popped, the priority of the
         *      source (lower) frame is ignored, and the default priority
@@ -664,13 +672,14 @@ unlang_frame_action_t result_pop(request_t *request, unlang_stack_frame_t *frame
         *      cases for this yet.
         */
        if (result->rcode != RLM_MODULE_NOT_SET) {
+               fr_assert(MOD_ACTION_VALID(frame->instruction->actions.actions[result->rcode]));
                our_result.priority = frame->instruction->actions.actions[result->rcode];
        }
 
-       RDEBUG4("** [%i] %s - using instruction priority for higher frame (%s, %d)",
+       RDEBUG4("** [%i] %s - using instruction priority for higher frame (%s, %s)",
                stack->depth, __FUNCTION__,
                fr_table_str_by_value(mod_rcode_table, our_result.rcode, "<invalid>"),
-               our_result.priority);
+               mod_action_name[our_result.priority]);
 
        return result_calculate(request, frame, &our_result);
 }
@@ -803,13 +812,12 @@ unlang_frame_action_t frame_eval(request_t *request, unlang_stack_frame_t *frame
                 */
                ua = frame->process(&frame->scratch_result, request, frame);
 
-               RDEBUG4("** [%i] %s << %s (%s %d)", stack->depth, __FUNCTION__,
+               fr_assert(MOD_ACTION_VALID(scratch->priority));
+
+               RDEBUG4("** [%i] %s << %s (%s %s)", stack->depth, __FUNCTION__,
                        fr_table_str_by_value(unlang_action_table, ua, "<INVALID>"),
                        fr_table_str_by_value(mod_rcode_table, scratch->rcode, "<INVALID>"),
-                       scratch->priority);
-
-               fr_assert(scratch->priority >= MOD_ACTION_NOT_SET);
-               fr_assert(scratch->priority <= MOD_PRIORITY_MAX);
+                       mod_action_name[scratch->priority]);
 
                /*
                 *      If the frame is cancelled we ignore the
@@ -854,9 +862,9 @@ unlang_frame_action_t frame_eval(request_t *request, unlang_stack_frame_t *frame
                                      "instead", instruction->name);
                        unlang_frame_perf_yield(frame);
                        yielded_set(frame);
-                       RDEBUG4("** [%i] %s - yielding with current (%s %d)", stack->depth, __FUNCTION__,
+                       RDEBUG4("** [%i] %s - yielding with current (%s %s)", stack->depth, __FUNCTION__,
                                fr_table_str_by_value(mod_rcode_table, scratch->rcode, "<invalid>"),
-                               scratch->priority);
+                               mod_action_name[scratch->priority]);
                        return UNLANG_FRAME_ACTION_YIELD;
 
                /*
@@ -914,10 +922,12 @@ unlang_frame_action_t frame_eval(request_t *request, unlang_stack_frame_t *frame
        }
 
 pop:
-       RDEBUG4("** [%i] %s - done current subsection with (%s %d), %s",
+       fr_assert(MOD_ACTION_VALID(frame->result_p->priority));
+
+       RDEBUG4("** [%i] %s - done current subsection with (%s %s), %s",
                stack->depth, __FUNCTION__,
                fr_table_str_by_value(mod_rcode_table, frame->result_p->rcode, "<invalid>"),
-               frame->result_p->priority,
+               mod_action_name[frame->result_p->priority],
                frame->result_p == &(frame->section_result) ? "will set higher frame rcode" : "will NOT set higher frame rcode (result_p)");
 
        return UNLANG_FRAME_ACTION_POP;
@@ -1040,6 +1050,8 @@ CC_HINT(hot) rlm_rcode_t unlang_interpret(request_t *request, bool running)
                         */
                        instruction_done_debug(request, frame, frame->instruction);
 
+                       fr_assert(MOD_ACTION_VALID(frame->section_result.priority));
+
                        /*
                         *      If we're continuing after popping a frame
                         *      then we advance the instruction else we
@@ -1047,10 +1059,10 @@ CC_HINT(hot) rlm_rcode_t unlang_interpret(request_t *request, bool running)
                         */
                        switch (fa) {
                        case UNLANG_FRAME_ACTION_NEXT:
-                               DEBUG4("** [%i] %s - continuing after subsection with (%s %d)",
+                               DEBUG4("** [%i] %s - continuing after subsection with (%s %s)",
                                        stack->depth, __FUNCTION__,
                                        fr_table_str_by_value(mod_rcode_table, frame->section_result.rcode, "<invalid>"),
-                                       frame->section_result.priority);
+                                       mod_action_name[frame->section_result.priority]);
                                frame_next(stack, frame);
                                goto next;
 
@@ -1059,10 +1071,10 @@ CC_HINT(hot) rlm_rcode_t unlang_interpret(request_t *request, bool running)
                         *      print some helpful debug...
                         */
                        default:
-                               RDEBUG4("** [%i] %s - done current subsection with (%s %d)",
+                               RDEBUG4("** [%i] %s - done current subsection with (%s %s)",
                                        stack->depth, __FUNCTION__,
                                        fr_table_str_by_value(mod_rcode_table, frame->section_result.rcode, "<invalid>"),
-                                       frame->section_result.priority);
+                                       mod_action_name[frame->section_result.priority]);
                                continue;
                        }
 
index a719741d594652f197bafdc8550ffdf41ab0aa86..dc85eae620ee75502a09189f0f805b9ec20d1702 100644 (file)
@@ -105,3 +105,34 @@ unlang_mod_actions_t const mod_actions_postauth = {
        },
        .retry = RETRY_INIT
 };
+
+#ifdef __clang__
+#pragma clang diagnostic ignored "-Wgnu-designator"
+#endif
+
+const char *mod_action_name[MOD_PRIORITY_MAX + 1] = {
+       [MOD_ACTION_NOT_SET] = "not-set",
+       [MOD_ACTION_RETRY]   = "retry",
+       [MOD_ACTION_REJECT]  = "reject",
+       [MOD_ACTION_RETURN]  = "return",
+
+       [4 ... 0x80] = "<INVALID>",
+
+       // zsh: for x ({1..64}); do print -n "[MOD_PRIORITY($x)] = \"$x\", "; done
+       [MOD_PRIORITY(1)] = "1", [MOD_PRIORITY(2)] = "2", [MOD_PRIORITY(3)] = "3", [MOD_PRIORITY(4)] = "4",
+       [MOD_PRIORITY(5)] = "5", [MOD_PRIORITY(6)] = "6", [MOD_PRIORITY(7)] = "7", [MOD_PRIORITY(8)] = "8",
+       [MOD_PRIORITY(9)] = "9", [MOD_PRIORITY(10)] = "10", [MOD_PRIORITY(11)] = "11", [MOD_PRIORITY(12)] = "12",
+       [MOD_PRIORITY(13)] = "13", [MOD_PRIORITY(14)] = "14", [MOD_PRIORITY(15)] = "15", [MOD_PRIORITY(16)] = "16",
+       [MOD_PRIORITY(17)] = "17", [MOD_PRIORITY(18)] = "18", [MOD_PRIORITY(19)] = "19", [MOD_PRIORITY(20)] = "20",
+       [MOD_PRIORITY(21)] = "21", [MOD_PRIORITY(22)] = "22", [MOD_PRIORITY(23)] = "23", [MOD_PRIORITY(24)] = "24",
+       [MOD_PRIORITY(25)] = "25", [MOD_PRIORITY(26)] = "26", [MOD_PRIORITY(27)] = "27", [MOD_PRIORITY(28)] = "28",
+       [MOD_PRIORITY(29)] = "29", [MOD_PRIORITY(30)] = "30", [MOD_PRIORITY(31)] = "31", [MOD_PRIORITY(32)] = "32",
+       [MOD_PRIORITY(33)] = "33", [MOD_PRIORITY(34)] = "34", [MOD_PRIORITY(35)] = "35", [MOD_PRIORITY(36)] = "36",
+       [MOD_PRIORITY(37)] = "37", [MOD_PRIORITY(38)] = "38", [MOD_PRIORITY(39)] = "39", [MOD_PRIORITY(40)] = "40",
+       [MOD_PRIORITY(41)] = "41", [MOD_PRIORITY(42)] = "42", [MOD_PRIORITY(43)] = "43", [MOD_PRIORITY(44)] = "44",
+       [MOD_PRIORITY(45)] = "45", [MOD_PRIORITY(46)] = "46", [MOD_PRIORITY(47)] = "47", [MOD_PRIORITY(48)] = "48",
+       [MOD_PRIORITY(49)] = "49", [MOD_PRIORITY(50)] = "50", [MOD_PRIORITY(51)] = "51", [MOD_PRIORITY(52)] = "52",
+       [MOD_PRIORITY(53)] = "53", [MOD_PRIORITY(54)] = "54", [MOD_PRIORITY(55)] = "55", [MOD_PRIORITY(56)] = "56",
+       [MOD_PRIORITY(57)] = "57", [MOD_PRIORITY(58)] = "58", [MOD_PRIORITY(59)] = "59", [MOD_PRIORITY(60)] = "60",
+       [MOD_PRIORITY(61)] = "61", [MOD_PRIORITY(62)] = "62", [MOD_PRIORITY(63)] = "63", [MOD_PRIORITY(64)] = "64",
+};
index 0c3292e3d7791d7140bd5896787e830033f35707..033ff29ca7c65aa5824fcb5e6607fda7a335d127 100644 (file)
@@ -31,13 +31,13 @@ extern "C" {
 #include <freeradius-devel/server/rcode.h>
 #include <freeradius-devel/util/retry.h>
 
-#define MOD_PRIORITY(_x) (_x)
+#define MOD_PRIORITY(_x) ((_x) | 0x80)
 
 typedef enum {
-       MOD_ACTION_NOT_SET = -4,        //!< default "not set by anything"
-       MOD_ACTION_RETRY = -3,          //!< retry the instruction, MUST also set a retry config
-       MOD_ACTION_REJECT = -2,         //!< change the rcode to REJECT, with unset priority
-       MOD_ACTION_RETURN = -1,         //!< stop processing the section,
+       MOD_ACTION_NOT_SET = 0,         //!< default "not set by anything"
+       MOD_ACTION_RETRY = 1,           //!< retry the instruction, MUST also set a retry config
+       MOD_ACTION_REJECT = 2,          //!< change the rcode to REJECT, with unset priority
+       MOD_ACTION_RETURN = 3,          //!< stop processing the section,
                                        //!<  and return the rcode with unset priority
 
        MOD_PRIORITY_1 = MOD_PRIORITY(1),
@@ -57,6 +57,8 @@ typedef enum {
 } unlang_mod_action_t;
 
 #define MOD_PRIORITY_MIN MOD_PRIORITY_1
+#define MOD_ACTION_VALID(_x)     ((((_x) >= 0) && ((_x) <= 3)) || (((_x) >= MOD_PRIORITY_MIN) && ((_x) <= MOD_PRIORITY_MAX)))
+#define MOD_ACTION_VALID_SET(_x) ((((_x) > 0)  && ((_x) <= 3)) || (((_x) >= MOD_PRIORITY_MIN) && ((_x) <= MOD_PRIORITY_MAX)))
 
 typedef struct {
        unlang_mod_action_t     actions[RLM_MODULE_NUMCODES];
@@ -70,6 +72,7 @@ extern unlang_mod_actions_t const mod_actions_authorize;
 extern unlang_mod_actions_t const mod_actions_preacct;
 extern unlang_mod_actions_t const mod_actions_accounting;
 extern unlang_mod_actions_t const mod_actions_postauth;
+extern const char *mod_action_name[MOD_PRIORITY_MAX + 1];
 
 #ifdef __cplusplus
 }
index 4eb0e21fc58f8ca9a21f53d938aec27eb4256176..4b50b54271085e473d5dfed12693e95f78eab431 100644 (file)
@@ -164,18 +164,19 @@ static unlang_action_t unlang_parallel_resume(unlang_result_t *p_result, request
 
                } else {
                        fr_assert(cr->result.priority != MOD_ACTION_RETRY);
+                       fr_assert(MOD_ACTION_VALID(cr->result.priority));
                }
 
                /*
                 *      Do priority over-ride.
                 */
                if (cr->result.priority > state->result.priority) {
-                       RDEBUG4("** [%i] %s - overwriting existing result (%s %d) from higher priority to (%s %d)",
+                       RDEBUG4("** [%i] %s - overwriting existing result (%s %s) from higher priority to (%s %s)",
                                stack_depth_current(request), __FUNCTION__,
                                fr_table_str_by_value(mod_rcode_table, state->result.rcode, "<invalid>"),
-                               state->result.priority,
+                               mod_action_name[state->result.priority],
                                fr_table_str_by_value(mod_rcode_table, cr->result.rcode, "<invalid>"),
-                               cr->result.priority);
+                               mod_action_name[cr->result.priority]);
                        state->result = cr->result;
                }
        }
@@ -203,7 +204,6 @@ static unlang_action_t unlang_parallel(unlang_result_t *p_result, request_t *req
        unlang_group_t                  *g;
        unlang_parallel_t               *gext;
        unlang_parallel_state_t         *state;
-
        int                     i;
 
        g = unlang_generic_to_group(frame->instruction);
index fff7cbae5139d7b3bd724a8b421742d702aaf786..3fc379a204f476c6343362c1b239af6f84330e56 100644 (file)
@@ -245,17 +245,22 @@ bool pass2_fixup_map_rhs(unlang_group_t *g, tmpl_rules_t const *rules);
 static inline CC_HINT(always_inline)
 void unlang_compile_ctx_copy(unlang_compile_ctx_t *dst, unlang_compile_ctx_t const *src)
 {
+#ifndef NDEBUG
        int i;
+#endif
 
        *dst = *src;
 
+#ifndef NDEBUG
        /*
-        *      Ensure that none of the actions are RETRY.
+        *      Ensure that none of the actions are RETRY.  The actions { ... } section is applied to the
+        *      instruction, and not to the unlang_compile_ctx_t
         */
        for (i = 0; i < RLM_MODULE_NUMCODES; i++) {
-               if (dst->actions.actions[i] == MOD_ACTION_RETRY) dst->actions.actions[i] = MOD_PRIORITY_MIN;
+               fr_assert(dst->actions.actions[i] != MOD_ACTION_RETRY);
+               fr_assert(MOD_ACTION_VALID(dst->actions.actions[i]));
        }
-       memset(&dst->actions.retry, 0, sizeof(dst->actions.retry)); \
+#endif
 }
 
 
@@ -465,8 +470,6 @@ extern fr_hash_table_t *unlang_op_table;
 
 extern fr_table_num_sorted_t const mod_rcode_table[];
 extern size_t mod_rcode_table_len;
-extern fr_table_num_sorted_t const mod_action_table[];
-extern size_t mod_action_table_len;
 
 static inline void repeatable_set(unlang_stack_frame_t *frame)                 { frame->flag |= UNLANG_FRAME_FLAG_REPEAT; }
 static inline void top_frame_set(unlang_stack_frame_t *frame)                  { frame->flag |= UNLANG_FRAME_FLAG_TOP_FRAME; }