From: Alan T. DeKok Date: Sat, 7 Oct 2023 14:34:51 +0000 (-0400) Subject: make cmp return bool and fix associated tests X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4eb6a26ff7a8ba2ad4d85f7269c671dd3cdbe851;p=thirdparty%2Ffreeradius-server.git make cmp return bool and fix associated tests --- diff --git a/src/lib/unlang/xlat_expr.c b/src/lib/unlang/xlat_expr.c index a51061f210f..a384ba2b976 100644 --- a/src/lib/unlang/xlat_expr.c +++ b/src/lib/unlang/xlat_expr.c @@ -471,8 +471,6 @@ static xlat_action_t xlat_cmp_op(TALLOC_CTX *ctx, fr_dcursor_t *out, int rcode; fr_value_box_t *dst, *a, *b; - MEM(dst = fr_value_box_alloc_null(ctx)); - /* * Each argument is a FR_TYPE_GROUP, with one or more elements in a list. */ @@ -486,16 +484,18 @@ static xlat_action_t xlat_cmp_op(TALLOC_CTX *ctx, fr_dcursor_t *out, fr_assert(fr_comparison_op[op]); + MEM(dst = fr_value_box_alloc(ctx, FR_TYPE_BOOL, attr_expr_bool_enum)); + rcode = fr_value_calc_list_cmp(dst, dst, &a->vb_group, op, &b->vb_group); if (rcode < 0) { - RPEDEBUG("Failed calculating result, returning NULL"); - goto done; + talloc_free(dst); + RPEDEBUG("Failed calculating result, returning fail"); + return XLAT_ACTION_FAIL; } fr_assert(dst->type == FR_TYPE_BOOL); dst->enumv = attr_expr_bool_enum; -done: fr_dcursor_append(out, dst); VALUE_BOX_LIST_VERIFY((fr_value_box_list_t *)out->dlist); return XLAT_ACTION_DONE; @@ -1666,7 +1666,7 @@ do { \ #undef XLAT_REGISTER_BINARY_CMP #define XLAT_REGISTER_BINARY_CMP(_op, _name) \ do { \ - if (unlikely((xlat = xlat_func_register(NULL, "cmp_" STRINGIFY(_name), xlat_func_cmp_ ## _name, FR_TYPE_VOID)) == NULL)) return -1; \ + if (unlikely((xlat = xlat_func_register(NULL, "cmp_" STRINGIFY(_name), xlat_func_cmp_ ## _name, FR_TYPE_BOOL)) == NULL)) return -1; \ xlat_func_args_set(xlat, binary_cmp_xlat_args); \ xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_PURE | XLAT_FUNC_FLAG_INTERNAL); \ xlat_func_print_set(xlat, xlat_expr_print_binary); \ diff --git a/src/lib/unlang/xlat_purify.c b/src/lib/unlang/xlat_purify.c index 82f242cf24e..fd297fe6fb0 100644 --- a/src/lib/unlang/xlat_purify.c +++ b/src/lib/unlang/xlat_purify.c @@ -255,10 +255,25 @@ static xlat_exp_t *peephole_optimize_lor(xlat_exp_t *lhs, xlat_exp_t *rhs) /* * FOO || 0 --> FOO much of the time * FOO || 1 --> FOO much of the time + */ + if (!is_truthy(rhs, &value)) return NULL; + + /* + * BOOL || 1 --> 1 + * + * Because if the LHS is 1, then we return the LHS (1) + * On the other hand, it the LHS is 0, then we return + * the RHS, which is also 1. * - * @todo - if the LHS is a function AND the LHS returns a boolean, THEN we can optimized - * the "FOO || 1" case to just "1". + * But we can't do + * + * || 1 --> 1 */ + if (value && (lhs->type == XLAT_FUNC) && (lhs->call.func->return_type == FR_TYPE_BOOL)) { + talloc_free(lhs); + return rhs; + } + return NULL; } diff --git a/src/tests/unit/condition/base.txt b/src/tests/unit/condition/base.txt index afe19da2343..5a5fafc2b66 100644 --- a/src/tests/unit/condition/base.txt +++ b/src/tests/unit/condition/base.txt @@ -668,7 +668,7 @@ match (&User-Name || true) # can't optimize it. # condition (&User-Name == "bob") || (true) -match ((&User-Name == "bob") || true) +match true # # A && (B || C) is not the same as (A && B) || C, for 0/1/1 diff --git a/src/tests/unit/xlat/cond_base.txt b/src/tests/unit/xlat/cond_base.txt index 3601a39ee6c..2a24899e863 100644 --- a/src/tests/unit/xlat/cond_base.txt +++ b/src/tests/unit/xlat/cond_base.txt @@ -110,7 +110,7 @@ xlat_purify (!"foo" == "bar") match ERROR offset 2: Operator '!' is only applied to the left hand side of the '==' operation, add (..) to evaluate the operation first xlat_purify ((!"foo") == "bar") -match NULL +match (!"foo" == "bar") xlat_purify ((!"foo") == false) match true @@ -251,9 +251,9 @@ xlat_purify (ipaddr)127.0.0.1 == "127.0.0.1" match true # LHS is IPaddr, RHS is string (malformed IP address). -# Condition code attempts to cast md4 hash to IP address resulting in an invalid comparison +# We can only fail this at run-time. xlat_purify (ipaddr)127.0.0.1 == "%{md4: 127.0.0.1}" -match NULL +match (127.0.0.1 == %(cast:string "%{md4: 127.0.0.1}")) # # Bare %{...} is allowed. @@ -261,7 +261,7 @@ match NULL # Invalid cast from octets to ipaddr. xlat_purify (ipaddr)127.0.0.1 == %{md4:127.0.0.1} -match NULL +match (127.0.0.1 == %{md4:127.0.0.1}) xlat_purify (ipaddr)127.0.0.1 == %{md4: SELECT user FROM table WHERE user='%{User-Name}'} match (127.0.0.1 == %{md4: SELECT user FROM table WHERE user='%{User-Name}'}) @@ -271,7 +271,7 @@ match true # Invalid cast from octets to ether. xlat_purify (ether)00:11:22:33:44:55 == "%{md4:00:11:22:33:44:55}" -match NULL +match (00:11:22:33:44:55 == %(cast:string "%{md4:00:11:22:33:44:55}")) xlat_purify (ether) 00:XX:22:33:44:55 == 00:11:22:33:44:55 match ERROR offset 12: Missing separator, expected ':'