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

Stephen Gallagher stephen at gallagherhome.com
Thu Apr 22 22:21:06 UTC 2010


11 times out of 10, it's a bad idea to use inline explicitly. Modern
compilers will automatically optimize small functions into inline
where it is safe to do so.

On Apr 22, 2010, at 5:58 PM, Martin Nagy <mnagy at redhat.com> wrote:

> 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
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel



More information about the sssd-devel mailing list