On 06/24/2011 09:20 AM, Brian Candler wrote:
On Thu, Jun 23, 2011 at 10:04:05PM +0100, Phil Mayers wrote:
Unless anyone can think of a reason not to, I'll take a look at this tomorrow / next week; it doesn't look terribly hard to solve.
Since you know this stuff, can you explain why there seem to be two sets of attribute handling code, some in lib/ (e.g. paircmp) and some in main/ (e.g. radius_compare_vps)?
I don't know the background, sorry. It's only recently I've looked at the server code code (as opposed to digging into the source for rlm_whatever, which I do a lot to find out how the things actually behave in edge cases). Alan can probably comment. I do know that some of the seemingly duplicate API points do subtly different things. It's probable they could be rationalised, although the resulting API calls might end up with "flags" arguments to control the behaviour, which might be worse, not better. You can argue that putting the behaviour in the function name is a lot more obvious than in a "flags" bitmask.
This is what has put me off trying to look at this area myself.
Start small maybe? Find one pair of calls that look like a duplicate, submit a patch to merge them, repeat/rinse. Assuming it's a problem worth solving, of course. What do we think about the various API doc tool e.g. Doxygen? It might make navigating the source easier for people unfamiliar with it.