[SSSD] [PATCH] Make the handling of fd events opaque

Martin Nagy mnagy at redhat.com
Thu Apr 22 21:58:10 UTC 2010


On 04/16/2010 12:22 PM, Sumit Bose wrote:
> Hi,
>
> to support the current effort to make the LDAP provider more robust this
> patch removes all the #ifdef HAVE_LDAP_CONNCB calls from the main code
> into a separate file which provides generic helper functions.
>
> bye,
> Sumit

Nitpick:
-static void sdap_ldap_result(struct tevent_context *ev,
+void sdap_ldap_result(struct tevent_context *ev,
                               struct tevent_fd *fde,
                               uint16_t flags, void *pvt)
Please indent the last two lines so that "struct" and "uint16_t" begin 
where the "struct" on the first line begins. This is in sdap_async.c and 
sdap_async_private.h.

In setup_ldap_connection_callbacks():
     ret = ldap_set_option(sh->ldap, LDAP_OPT_CONNECT_CB,
                           sh->sdap_fd_events->conncb);
     if (ret != LDAP_OPT_SUCCESS) {
         DEBUG(1, ("Failed to set connection callback\n"));
         goto fail;
     }
Mixing of errno and ldap return values.

sdap_set_connected():
I'm not sure if the way you defined the inline function is very the 
right one [1]. I'd recommend either using "static inline" and define the 
function in the header file, or simply not make it inline.

Other than this, the patch looks very good.

Martin

[1] http://www.greenend.org.uk/rjk/2003/03/inline.html



More information about the sssd-devel mailing list