Alister Winfield wrote:
Sent this before but I had a mixed up valuepair.c which ment I couldn't just get a diff ;-(. Now I have found time to unpick the changes to make a sane copy of valuepair and, therefore, a good looking diff.
Re-iterating the problem... When trying to get a not equals match the existing code, up to 1.0.5 and probably in the later CVS HEAD, only notices the first value, which, if it doesn't match returns a true response. Should you actually have many values for the same attribute then you may want the test to only return true if none of the values of that attribute are a match.
To get the effect of finding that a value doesn't appear at all for a given attribute this patch avoids negative tests and instead does a positive match and inverts the response at the exit points in the function as required. BTW. Hopefully this doesn't touch the current behaviour for anything else certainly my tests suggest its fine.
This patch looks good for the latest released version 1.0.5.
--
Not certain I understand what you mean, Tell me if the attached patch looks like what you want and I will test it. --- valuepair.c.orig 2005-11-04 15:27:46.000000000 -0500 +++ valuepair.c.JM 2005-11-04 15:34:08.000000000 -0500 @@ -284,7 +284,7 @@ * to not find it, then we succeeded. */ if (check_item->operator == T_OP_CMP_FALSE) - return 0; + continue; else return -1; } @@ -295,7 +295,10 @@ */ if (check_item->operator == T_OP_CMP_FALSE) return -1; - + + /* no comparison needed if it exists */ + if (check_item->operator == T_OP_CMP_TRUE) + continue; /* * We've got to xlat the string before doing @@ -327,14 +330,14 @@ radlog(L_ERR, "Invalid operator for item %s: " "reverting to '=='", check_item->name); /*FALLTHRU*/ - case T_OP_CMP_TRUE: /* compare always == 0 */ - case T_OP_CMP_FALSE: /* compare always == 1 */ case T_OP_CMP_EQ: if (compare != 0) result = -1; break; case T_OP_NE: - if (compare == 0) result = -1; + /* != only succeeds if NO reply values match*/ + if (compare == 0) return -1; + result = -1; /*check the rest for no match */ break; case T_OP_LT: @@ -428,7 +431,9 @@ compare = regexec(®, (char *)auth_item->strvalue, 0, NULL, 0); regfree(®); - if (compare == 0) result = -1; + /* !~ only succeeds if NO matches are found in reply pair */ + if (compare == 0) return -1; + result = -1; /*check the rest for no match */ break; #endif