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

Lukas Slebodnik lslebodn at redhat.com
Fri Jul 18 14:14:58 UTC 2014


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
>
>---
> src/providers/ldap/sdap_async.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
>diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
>index 590cbe100bdf5f3d6df8d9d7b356fd1d560cd729..50e60ff846d9f94bb9f7343246429f1345286719 100644
>--- a/src/providers/ldap/sdap_async.c
>+++ b/src/providers/ldap/sdap_async.c
>@@ -1335,6 +1335,8 @@ static void sdap_get_generic_ext_done(struct sdap_op *op,
>     struct sdap_get_generic_ext_state *state = tevent_req_data(req,
>                                             struct sdap_get_generic_ext_state);
>     char *errmsg = NULL;
>+    int i;
>+    char **refs = NULL;
>     int result;
>     int ret;
>     int lret;
>@@ -1370,7 +1372,7 @@ static void sdap_get_generic_ext_done(struct sdap_op *op,
> 
>     case LDAP_RES_SEARCH_RESULT:
>         ret = ldap_parse_result(state->sh->ldap, reply->msg,
>-                                &result, NULL, &errmsg, NULL,
>+                                &result, NULL, &errmsg, &refs,
>                                 &returned_controls, 0);
>         if (ret != LDAP_SUCCESS) {
>             DEBUG(SSSDBG_OP_FAILURE,
>@@ -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/

>+            for (i = 0; refs[i]; i++) {
>+                DEBUG(SSSDBG_TRACE_LIBS, "Ref: %s\n", refs[i]);
>+            }
>+            ldap_memvfree((void **) refs);
>+        }
>+
>         if (result == LDAP_SIZELIMIT_EXCEEDED) {
>             /* Try to return what we've got */
>             DEBUG(SSSDBG_MINOR_FAILURE,
>-- 
>1.9.3
>

>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.

>+{
>+    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 :-) :-) :-)

Otherwise, LGTM.
You asked for opinion of other developers, therefore I let final ACK for
someone else.  I am fine with pushing current version.

LS



More information about the sssd-devel mailing list