Hello all,
i'm clearing up my patch queue and have some patches ready for merge. Unless i receive any comments or objections, i'll merge them to master by the end of the week.
They're in branch alexh/master now.
Hi Alex,
thanks for your contributions.
My only question is related to the update to allow_trusted(). Now it goes through all the records, if I understood it right, while the former version was returning at the first match. Maybe we can have a parameter to the function to control this behavior, default the backward compatible way (not to get unexpected behavior due to this change), and when a specific parameter value is given then have the behavior you added. What do you think?
Cheers, Daniel
On 3/7/11 3:24 PM, Alex Hermann wrote:
Hello all,
i'm clearing up my patch queue and have some patches ready for merge. Unless i receive any comments or objections, i'll merge them to master by the end of the week.
They're in branch alexh/master now.
On Tuesday 08 March 2011, Daniel-Constantin Mierla wrote:
thanks for your contributions.
My only question is related to the update to allow_trusted(). Now it goes through all the records, if I understood it right, while the former version was returning at the first match.
Correct. But only through the records of the matching hash table bucket.
Maybe we can have a parameter to the function to control this behavior, default the backward compatible way (not to get unexpected behavior due to this change), and when a specific parameter value is given then have the behavior you added. What do you think?
I thought about adding a parameter, but then i considered the current behaviour as buggy, because you can't see if there are multiple matches and you don't know which of the matching tags is returned.
For most configurations, the behaviour is backwards compatible, the return value is still positive if there is a match, and the tag is still added to the avp. The order of added tags was already undefined when there were multiple matches, it was undefined which tag would have been added to the avp. Now you get them all.
Nevertheless, adding the parameter was a no-brainer...
On 3/8/11 12:05 PM, Alex Hermann wrote:
On Tuesday 08 March 2011, Daniel-Constantin Mierla wrote:
thanks for your contributions.
My only question is related to the update to allow_trusted(). Now it goes through all the records, if I understood it right, while the former version was returning at the first match.
Correct. But only through the records of the matching hash table bucket.
Maybe we can have a parameter to the function to control this behavior, default the backward compatible way (not to get unexpected behavior due to this change), and when a specific parameter value is given then have the behavior you added. What do you think?
I thought about adding a parameter, but then i considered the current behaviour as buggy, because you can't see if there are multiple matches and you don't know which of the matching tags is returned.
For most configurations, the behaviour is backwards compatible, the return value is still positive if there is a match, and the tag is still added to the avp. The order of added tags was already undefined when there were multiple matches, it was undefined which tag would have been added to the avp. Now you get them all.
Nevertheless, adding the parameter was a no-brainer...
Thanks Alex, indeed it was not something complex, it is why I thought it worth doing. However,my idea was slightly different to use a parameter for allow_trusted(), so you can have many calls of the function with different behaviour. But probably makes no sense in this case at it is a single list of records to work with. The allow_address() can have different groups of addresses, there the multiple match can happen on network addresses -- it is not the case anyhow, different function.
Cheers, Daniel
On Tuesday 08 March 2011, Daniel-Constantin Mierla wrote:
indeed it was not something complex, it is why I thought it worth doing. However,my idea was slightly different to use a parameter for allow_trusted(), so you can have many calls of the function with different behaviour.
I looked at that option and I'm all for it (there are already enough functions dependent on 'global' parameters), but there are already 2 allow_trusted functions for 0 or 2 parameters. Adding another (1st or 3rd) optional parameter to those would make a complete mess of it, requiring 4 implementations and forwarding the option through a series of functions. Making them non-optional would not be backwards compatible, which was the whole purpose of making it configurable.
-
Alex Hermann -
Daniel-Constantin Mierla