[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: (ITS#7027) [PATCH] Implement priority/weigh for DNS SRV records



Taking a look again as it was recently called to our attention in 
https://bugzilla.redhat.com/show_bug.cgi?id=1095976

>> In addition, random seed has to be initialized, otherwise the behavior
>> is predictable.
>
> I didn't think it too big of a deal, since it doesn't have to be
> cryptographically random. I can submit a patch to call srandom()
> first.
>
>> On the other side, I do not think that calling srand()
>> in libldap is a good idea. The client application should do that. Or
>> is there any other option?

Agreed, calling srand() inside libldap would probably be a bad idea. Nor does 
it seem critical to use any particularly high quality PRNG here. I've added a 
commonly used LCG instead, seeded by time().

> The function ldap_domain2hostlist returns a list of strings, there
> isn't any weight/prio information for the client to act
> on. Furthermore the weight prio section was commented out, I figured
> the library was the right place to do it.
>
> The algo specified in rfc2782 is detailed, and specifies a random
> number should be generated., I didn't want to implement it because it
> results in sorting once by prio, once by weight instead of one sort
> that does both.

You cannot shuffle inside quicksort, its behavior is undefined if the compare 
function isn't consistent/transitive. There's a good illustration of one 
problem with that here http://bost.ocks.org/mike/algorithms/#shuffling

I've simplified the qsort comparator and added the explicit shuffle step.

This is now fixed in git master.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/