[SSSD] [PATCH] Two patches with more debugging for the LDAP provider

Pavel Březina pbrezina at redhat.com
Mon Jul 21 08:42:05 UTC 2014


On 07/18/2014 04:52 PM, Lukas Slebodnik wrote:
> On (18/07/14 16:23), Jakub Hrozek wrote:
>> On Fri, Jul 18, 2014 at 04:14:58PM +0200, Lukas Slebodnik wrote:
>>> On (18/07/14 15:52), Jakub Hrozek wrote:
>>>> Hi,
>>>>
>>>> I was trying to help a user who had problems with the AD provider and
>>>> realized I had hard time following what exact server are we searching at
>>>> the time and, because we hit a referral, where the referral pointed to.
>>>>
>>>> I wrote the two attached patches. I'm OK with keeping them in my local
>>>> tree, but if other developers agree, maybe we can include them upstream
>>>> as well.
>>>
>>> >From 10edfd72516e7c1ac24b3ebe148825d12325e04e Mon Sep 17 00:00:00 2001
>>>> From: Jakub Hrozek <jhrozek at redhat.com>
>>>> Date: Fri, 18 Jul 2014 15:04:28 +0200
>>>> Subject: [PATCH 1/2] LDAP: Print referrals for debugging purposes
>>
>> [...]
>>
>>>> @@ -1383,6 +1385,13 @@ static void sdap_get_generic_ext_done(struct sdap_op *op,
>>>>                    sss_ldap_err2string(result), result,
>>>>                    errmsg ? errmsg : "no errmsg set");
>>>>
>>>> +        if (refs) {
>>> I thought you prefer refs != NULL
>>> http://blog.cryptomilk.org/2013/03/28/writing-and-reading-code/
>>
>> Fixed, I admit I copied this code from ldapsearch.c
>>
>>>
>>>> +            for (i = 0; refs[i]; i++) {
>>>> +                DEBUG(SSSDBG_TRACE_LIBS, "Ref: %s\n", refs[i]);
>>>> +            }
>>>> +            ldap_memvfree((void **) refs);
>>>> +        }
>>
>>> >From 8b1ad7a4797b941d8d35700c7a5558f3362ba758 Mon Sep 17 00:00:00 2001
>>>> From: Jakub Hrozek <jhrozek at redhat.com>
>>>> Date: Fri, 18 Jul 2014 15:37:34 +0200
>>>> Subject: [PATCH 2/2] LDAP: Dump LDAP server IP address with a high DEBUG level
>>>>
>>>> ---
>>>> src/providers/ldap/sdap_async.c | 37 +++++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 37 insertions(+)
>>>>
>>>> diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
>>>> index 50e60ff846d9f94bb9f7343246429f1345286719..4e91c02b7ba82ad9d1d8500f84906f0e40f74399 100644
>>>> --- a/src/providers/ldap/sdap_async.c
>>>> +++ b/src/providers/ldap/sdap_async.c
>>>> @@ -1083,6 +1083,41 @@ static errno_t add_to_deref_reply(TALLOC_CTX *mem_ctx,
>>>>      return EOK;
>>>> }
>>>>
>>>> +void sdap_print_server(struct sdap_handle *sh)
>>> It would be better to have this function static.
>>
>> Done.
>>
>>>
>>>> +{
>>>> +    int ret;
>>>> +    int fd;
>>>> +    struct sockaddr_storage ss;
>>>> +    socklen_t ss_len = sizeof(ss);
>>>> +    char ip[NI_MAXHOST];
>>>> +
>>>> +    if (!(debug_level & SSSDBG_TRACE_INTERNAL)) {
>>> and one more time
>>> http://blog.cryptomilk.org/2013/03/28/writing-and-reading-code/
>>> I couldn't resist :-) :-) :-)
>>
>> I don't agree with that blog post completely, I think using "!" if you
>> really mean boolean false is OK. I dislike "if (!ptr)" as a way of
>> saying "if (ptr == NULL)".
>
>> +    if (!(debug_level & SSSDBG_TRACE_INTERNAL)) {
>             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>         type of this expression is integer and not boolean :-)

We have DEBUG_IS_SET macro for this type of tests.

>
> LS
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>




More information about the sssd-devel mailing list