From: Alan T. DeKok Date: Tue, 15 Jul 2025 18:45:03 +0000 (-0400) Subject: move MOD_ACTION_NOT_SET --> 0 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5e487057407fbb85e0d0e6421c5142cd3af3ad85;p=thirdparty%2Ffreeradius-server.git move MOD_ACTION_NOT_SET --> 0 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) --- diff --git a/src/lib/unlang/compile.c b/src/lib/unlang/compile.c index b87356dc26..e9772e7e52 100644 --- a/src/lib/unlang/compile.c +++ b/src/lib/unlang/compile.c @@ -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); diff --git a/src/lib/unlang/interpret.c b/src/lib/unlang/interpret.c index 97a0276896..5885ef0a07 100644 --- a/src/lib/unlang/interpret.c +++ b/src/lib/unlang/interpret.c @@ -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, ""), - 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 "); } + 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, "")); - 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, ""), @@ -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, "")); - 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, "")); - 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, ""), - frame_result->priority, + mod_action_name[frame_result->priority], fr_table_str_by_value(mod_rcode_table, result->rcode, ""), - 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, ""), - 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, ""), - 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, ""), - 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, ""), - frame_result->priority, + mod_action_name[frame_result->priority], fr_table_str_by_value(mod_rcode_table, result->rcode, ""), - 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, ""), - 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, ""), fr_table_str_by_value(mod_rcode_table, scratch->rcode, ""), - 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, ""), - 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, ""), - 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, ""), - 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, ""), - frame->section_result.priority); + mod_action_name[frame->section_result.priority]); continue; } diff --git a/src/lib/unlang/mod_action.c b/src/lib/unlang/mod_action.c index a719741d59..dc85eae620 100644 --- a/src/lib/unlang/mod_action.c +++ b/src/lib/unlang/mod_action.c @@ -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] = "", + + // 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", +}; diff --git a/src/lib/unlang/mod_action.h b/src/lib/unlang/mod_action.h index 0c3292e3d7..033ff29ca7 100644 --- a/src/lib/unlang/mod_action.h +++ b/src/lib/unlang/mod_action.h @@ -31,13 +31,13 @@ extern "C" { #include #include -#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 } diff --git a/src/lib/unlang/parallel.c b/src/lib/unlang/parallel.c index 4eb0e21fc5..4b50b54271 100644 --- a/src/lib/unlang/parallel.c +++ b/src/lib/unlang/parallel.c @@ -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, ""), - state->result.priority, + mod_action_name[state->result.priority], fr_table_str_by_value(mod_rcode_table, cr->result.rcode, ""), - 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); diff --git a/src/lib/unlang/unlang_priv.h b/src/lib/unlang/unlang_priv.h index fff7cbae51..3fc379a204 100644 --- a/src/lib/unlang/unlang_priv.h +++ b/src/lib/unlang/unlang_priv.h @@ -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; }