Alan DeKok wrote:
Joe Maimon wrote:
It does make the code a bit more hairy - I have been making a stab at
this and it seems to be quite ugly.
Hmm... much of this work could be relegated to the radius_do_cmp()
function. It needs to do:
a) return if condition matches
b) continue if it doesn't match
With some sanity checks to ensure that:
(Foo != bar)
is the same as
!(Foo == bar)
Alan DeKok.
-
Ok, I have a preliminary patch, targeted at a pre-2.1.8 code base (git
stable 20090923)
480-evaluate-unlang-allvps.dpatch
(attached and pushed to my out of date github fork/branch)
It is indeed fairly ugly. I have not as of yet determined whether to
hang operational hat on it yet.
This patch aims to have the following as true:
(Foo != bar) == !(Foo == bar) && (Foo !~ bar) == !(Foo =~ bar)
Which is inline with the patch I currently use operationally.
200-cmp-operators-fix.dpatch
(attached)
I do not believe that this behavior is present unpatched.
I am undecided as to proper behavior for many of the other tests.
Thank you for all your help and feedback.
Joe
#! /bin/sh /usr/share/dpatch/dpatch-run
## 200-cmp-operators-fix.dpatch by <j...@sv04>
##
## All lines beginning with `## DP:' are a description of the patch.
## DP: No description.
@DPATCH@
diff -urNad radiusd~/src/main/valuepair.c radiusd/src/main/valuepair.c
--- radiusd~/src/main/valuepair.c 2008-07-20 15:35:53.000000000 -0400
+++ radiusd/src/main/valuepair.c 2008-07-20 15:36:07.000000000 -0400
@@ -155,10 +155,9 @@
compare = regexec(®, value, REQUEST_MAX_REGEX + 1,
rxmatch, 0);
regfree(®);
-
- if (compare != 0) return 0;
- return -1;
-
+ /* !~ only succeeds if NO matches are found in reply pair */
+ if (compare == 0) return -1;
+ ret = -1; /*check the rest for no match */
}
#endif
@@ -432,7 +431,12 @@
* Didn't find it. If we were *trying*
* to not find it, then we succeeded.
*/
- if (check_item->operator == T_OP_CMP_FALSE)
+ if (check_item->operator == T_OP_CMP_FALSE ||
+#ifdef HAVE_REGEX_H
+ check_item->operator == T_OP_REG_NE ||
+#endif
+ check_item->operator == T_OP_NE
+ )
continue;
else
return -1;
#! /bin/sh /usr/share/dpatch/dpatch-run
## 470-evaluate-unlang-allvps.dpatch by <j...@jmdeb01>
##
## All lines beginning with `## DP:' are a description of the patch.
## DP: No description.
@DPATCH@
diff -urNad freeradius-server~/src/main/evaluate.c
freeradius-server/src/main/evaluate.c
--- freeradius-server~/src/main/evaluate.c 2010-01-10 18:16:59.000000000
-0500
+++ freeradius-server/src/main/evaluate.c 2010-01-10 18:44:25.000000000
-0500
@@ -302,7 +302,8 @@
FR_TOKEN rt, const char *pright,
int cflags, int modreturn)
{
- int result;
+ int attempts = 0;
+ int result = 0;
int lint, rint;
VALUE_PAIR *vp = NULL;
#ifdef HAVE_REGEX_H
@@ -336,6 +337,32 @@
if (radius_get_vp(request, pleft, &vp)) {
VALUE_PAIR myvp;
+restart_cmp:
+ if (attempts) {
+ DICT_ATTR *da;
+
+ switch (token) {
+#ifdef HAVE_REGEX_H
+ case T_OP_REG_NE:
+#endif
+ case T_OP_NE:
+ /* Testing stops at the first negative
match */
+ if (!result)
+ return TRUE;
+ break;
+ default:
+ /* For all other tests, testing stops
at the first positive match */
+ if (result)
+ return TRUE;
+ }
+
+ /* pleft is trampled below, reinitialize it */
+ pleft = vp->name;
+ da = dict_attrbyname(pleft);
+ if (da)
+ vp = pairfind(vp->next, da->attr);
+ }
+
/*
* VP exists, and that's all we're looking for.
*/
@@ -344,6 +371,15 @@
return TRUE;
}
+ /*
+ * VP exists and we were looking for non-existence
+ */
+
+ if (token == T_OP_CMP_FALSE) {
+ *presult = (vp == NULL);
+ return TRUE;
+ }
+
if (!vp) {
DICT_ATTR *da;
@@ -361,12 +397,21 @@
RDEBUG3(" Callback returns %d",
*presult);
pairfree(&check);
- return TRUE;
+ if (vp && vp->next) {
+ result = *presult;
+ attempts++;
+ goto restart_cmp;
+ }
+ else
+ return TRUE;
}
- RDEBUG2(" (Attribute %s was not found)",
- pleft);
- return FALSE;
+ RDEBUG2(" (Attribute %s was not found%s)",
+ pleft, attempts ? " again":"");
+ if (!attempts)
+ return FALSE;
+ else
+ return TRUE;
}
#ifdef HAVE_REGEX_H
@@ -392,7 +437,13 @@
myvp.operator = token;
*presult = paircmp(&myvp, vp);
RDEBUG3(" paircmp -> %d", *presult);
- return TRUE;
+ if (vp && vp->next) {
+ result = *presult;
+ attempts++;
+ goto restart_cmp;
+ }
+ else
+ return TRUE;
} /* else it's not a VP in a list */
}
@@ -406,7 +457,14 @@
case T_OP_LT:
if (!all_digits(pright)) {
RDEBUG2(" (Right field is not a number at: %s)",
pright);
- return FALSE;
+ if (vp && vp->next) {
+ attempts++;
+ goto restart_cmp;
+ }
+ else if (attempts)
+ return TRUE;
+ else
+ return FALSE;
}
rint = atoi(pright);
if (!all_digits(pleft)) {
@@ -552,7 +610,12 @@
}
*presult = result;
- return TRUE;
+ if (vp && vp->next) {
+ attempts++;
+ goto restart_cmp;
+ }
+ else
+ return TRUE;
}
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
