On 06/17/2010 04:53 PM, Stephen Gallagher wrote:
On 06/17/2010 08:32 AM, Eugene Indenbom wrote:
TALLOC library has changed meanwhile. TALLOC version 2 uses typed destructors. Just take a look into talloc.h and see how type safety is implemented using simple macros and modern gcc features.
I wasn't aware of that, but it still means that we're using features that may not be available in all dependent platforms (I don't know if Debian is using libtalloc2 yet, for example) At minimum, if we're going to change to this style we need to update our configure scripts to detect an appropriate talloc version.
Will change it back to untyped. No problem.
<...>
I agree that it's better to move to next server after current one failed, but it is not acceptable to mark current server as not working just because of broken connection as there is a good change that it was a recoverable problem like expired GSSAPI ticket. Unfortunately fail-over API currently does not have such functionality.
So I would propose to make a separate patch which will add such a functionality to fail-over and then use it in sdap_id_op_done when broken connection is detected.
Ok, do you want to do this (or at least scope out the exact changes you want to see)?
I'd like to do it myself. It looks like very simple change: just move current server pointer to the next record in the list.
I do not agree that short-circuiting in this case is not worthwhile. From my point of view, overhead of allocating request and tevent_request_post should be avoided.
But anyway, changed it to full tevent style compliance.
In general, being able to follow the same execution path in either situation makes the code more maintainable. This is worth the minimal overhead here (tevent_req_post() implements a tevent_req_immediate() call, so it's not going to wait for another loop or anything)
Looking at the new code version, I am inclined to agree with you. :)