From: Alan T. DeKok <aland@freeradius.org>
Date: Wed, 11 Oct 2023 12:26:28 +0000 (-0400)
Subject: make !(a == b) the same as (a != b)
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=26822e09b9d59b532e9f2b8e1d5f6ad2bae3cbcb;p=thirdparty%2Ffreeradius-server.git

make !(a == b) the same as (a != b)
---

diff --git a/doc/antora/modules/reference/pages/unlang/condition/index.adoc b/doc/antora/modules/reference/pages/unlang/condition/index.adoc
index ff863d68fa5..0670b3afd30 100644
--- a/doc/antora/modules/reference/pages/unlang/condition/index.adoc
+++ b/doc/antora/modules/reference/pages/unlang/condition/index.adoc
@@ -115,7 +115,5 @@ In general, the following rules apply:
 
 * There is no longer a need to use `%{expr:..}` to do math in conditions.  Using the math in-place will work, as with `if (1 + 2 == 3)`.  See xref:unlang/condition/expression.adoc[expressions].
 
-* If you want to check that an attribute exists _and_ has a particular value, use `if !(&Attr == value)`, instead of `if (&Attr != value)`.  The difference us that if the attribute does not exist, it will match the condition `(&Attr != value)`, because the attribute has no value.
-
-// Copyright (C) 2021 Network RADIUS SAS.  Licenced under CC-by-NC 4.0.
+// Copyright (C) 2023 Network RADIUS SAS.  Licenced under CC-by-NC 4.0.
 // This documentation was developed by Network RADIUS SAS.
diff --git a/src/lib/unlang/xlat_expr.c b/src/lib/unlang/xlat_expr.c
index 6170d977ccd..abf43e6b238 100644
--- a/src/lib/unlang/xlat_expr.c
+++ b/src/lib/unlang/xlat_expr.c
@@ -422,8 +422,8 @@ static xlat_action_t xlat_binary_op(TALLOC_CTX *ctx, fr_dcursor_t *out,
 
 	rcode = fr_value_calc_binary_op(dst, dst, default_type, a, op, b);
 	if (rcode < 0) {
-		RPEDEBUG("Failed calculating result, returning NULL");
-		goto done;
+		talloc_free(dst);
+		return XLAT_ACTION_FAIL;
 	}
 
 	/*
@@ -432,7 +432,6 @@ static xlat_action_t xlat_binary_op(TALLOC_CTX *ctx, fr_dcursor_t *out,
 	 */
 	if (enumv) dst->enumv = enumv;
 
-done:
 	fr_dcursor_append(out, dst);
 	VALUE_BOX_LIST_VERIFY((fr_value_box_list_t *)out->dlist);
 	return XLAT_ACTION_DONE;
@@ -458,14 +457,14 @@ XLAT_BINARY_FUNC(op_rshift, T_RSHIFT)
 XLAT_BINARY_FUNC(op_lshift, T_LSHIFT)
 
 static xlat_arg_parser_t const binary_cmp_xlat_args[] = {
-	{ .required = true, .type = FR_TYPE_VOID },
-	{ .required = true, .type = FR_TYPE_VOID },
+	{ .required = false, .type = FR_TYPE_VOID },
+	{ .required = false, .type = FR_TYPE_VOID },
 	XLAT_ARG_PARSER_TERMINATOR
 };
 
 static xlat_action_t xlat_cmp_op(TALLOC_CTX *ctx, fr_dcursor_t *out,
 				 UNUSED xlat_ctx_t const *xctx,
-				 request_t *request, fr_value_box_list_t *in,
+				 UNUSED request_t *request, fr_value_box_list_t *in,
 				 fr_token_t op)
 {
 	int rcode;
@@ -489,7 +488,6 @@ static xlat_action_t xlat_cmp_op(TALLOC_CTX *ctx, fr_dcursor_t *out,
 	rcode = fr_value_calc_list_cmp(dst, dst, &a->vb_group, op, &b->vb_group);
 	if (rcode < 0) {
 		talloc_free(dst);
-		RPEDEBUG("Failed calculating result, returning fail");
 		return XLAT_ACTION_FAIL;
 	}
 
@@ -1435,8 +1433,8 @@ static xlat_action_t xlat_func_unary_op(TALLOC_CTX *ctx, fr_dcursor_t *out,
 	 */
 	rcode = fr_value_calc_unary_op(dst, dst, op, vb);
 	if ((rcode < 0) || fr_type_is_null(dst->type)) {
-		RPEDEBUG("Failed calculating result, returning NULL");
-		fr_value_box_init_null(dst);
+		talloc_free(dst);
+		return XLAT_ACTION_FAIL;
 	}
 
 	fr_dcursor_append(out, dst);
diff --git a/src/tests/keywords/cmp-eq-ne b/src/tests/keywords/cmp-eq-ne
new file mode 100644
index 00000000000..7079302515e
--- /dev/null
+++ b/src/tests/keywords/cmp-eq-ne
@@ -0,0 +1,25 @@
+uint32 foo
+uint32 result
+
+#
+#  "foo" does not yet exist.  So it doesn't have value "42"
+#
+if (&foo != 42) {
+	&result += 1
+}
+
+#
+#  This should be the same as above.
+#
+if !(&foo == 42) {
+	&result += 1
+}
+
+if !&result {
+	test_fail
+}
+
+if (&result != 2) {
+	test_fail
+}
+success
diff --git a/src/tests/unit/xlat/cond_base.txt b/src/tests/unit/xlat/cond_base.txt
index df99fbcee36..cd878fdb665 100644
--- a/src/tests/unit/xlat/cond_base.txt
+++ b/src/tests/unit/xlat/cond_base.txt
@@ -34,7 +34,7 @@ match ERROR offset 4: Unexpected text after enum value.  Expected operator
 #  0 - 111 is smaller than zero, and Service-Type is uint32.
 #
 xlat_purify (&Service-Type == 000-111)
-match (&Service-Type == NULL)
+match (&Service-Type == (0 - 111))
 
 xlat_purify (ok FOO handled)
 match ERROR offset 5: Invalid operator
diff --git a/src/tests/xlat/expr.txt b/src/tests/xlat/expr.txt
index 03527e2b6fb..d5a51373476 100644
--- a/src/tests/xlat/expr.txt
+++ b/src/tests/xlat/expr.txt
@@ -68,8 +68,7 @@ match 256
 
 # Cannot perform any operations for destination type ether
 xlat_expr 00:01:02:03:04:05 ^ 11:22:33:44:55:66
-match NULL
-#match ERROR expanding xlat: Cannot perform any operations for destination type ether
+match ERROR expanding xlat: Invalid operation on data types - 'ether' ^ 'ether'
 
 #
 #  NAS-IP-Address doesn't exist.