On Fri, 2008-05-09 at 13:20 +0200, Petter Reinholdtsen wrote: > Here is a better patch, making sure to only search once when the first > search succeeds.
I have a coulpe of problems with this patch (sorry, your work is very much appreciated): - I would really like to have the retry mechanism in myldap.c and not outside because most of the retry code is already there and this solution will probably conflict with that. - You should be able to leave the writing of the response header in place because the NSS code will do things right even if the response header is present and no entries are returned (it will report that NSS lookups through LDAP fail). Why did you move it? - This code fails when the search does not return any entries. E.g. when you lookup a non-existing user it will retry. It is better to check rc (result from myldap_get_entry()) for errors and retry when it is either LDAP_UNAVAILABLE or LDAP_SERVER_DOWN. The first point is very important because it makes it easier to test and also makes it available for the other searches that are done (e.g. in uid2dn() and dn2uid() and possibly others in the future). To do this in myldap.c the code should go in myldap_get_entry() and it should only be triggered if the first entry is to be retrieved (I don't know off hand if the myldap_search struct holds enough information for that yet). Also, it should only be triggered once (or a limited number of times) to not get in an infinite loop if getting the first entry always fails but the search succeeds. If the error is detected the connection is already closed (line 972 in r729) but instead of closing the search (line 974) the search should be retried. A myldap_retry_search() function should probably implemented and common code from myldap_search() should probably be moved to a separate function (maybe instead of a myldap_retry_search() a call to this new function will be enough). If the search is successfully retried another attempt to get the result should be made. This will obviously turn into a bigger patch and be more work but is a better solution in the end. -- -- arthur - [EMAIL PROTECTED] - http://people.debian.org/~adejong --
signature.asc
Description: This is a digitally signed message part
