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

Jakub Hrozek jhrozek at redhat.com
Fri Jul 18 14:23:14 UTC 2014


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)".
-------------- next part --------------
>From 619499ec35ac6fe5d626c58b023644d0eac26b5f 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..9154f65519889d44760f69ac47b15e3784f7935a 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 != NULL) {
+            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

-------------- next part --------------
>From 11365090f6329b78191c477cd2e9679b2cc13d1b 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 9154f65519889d44760f69ac47b15e3784f7935a..4e1237965b6f35f57a1672f47f9dda2f76437931 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;
 }
 
+static void sdap_print_server(struct sdap_handle *sh)
+{
+    int ret;
+    int fd;
+    struct sockaddr_storage ss;
+    socklen_t ss_len = sizeof(ss);
+    char ip[NI_MAXHOST];
+
+    if (!(debug_level & SSSDBG_TRACE_INTERNAL)) {
+        return;
+    }
+
+    ret = get_fd_from_ldap(sh->ldap, &fd);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_MINOR_FAILURE, "cannot get sdap fd\n");
+        return;
+    }
+
+    ret = getsockname(fd, (struct sockaddr *) &ss, &ss_len);
+    if (ret == -1) {
+        DEBUG(SSSDBG_MINOR_FAILURE, "getsockname failed\n");
+        return;
+    }
+
+    ret = getnameinfo((struct sockaddr *) &ss, ss_len,
+                      ip, sizeof(ip), NULL, 0, NI_NUMERICHOST);
+    ip[NI_MAXHOST-1] = '\0';
+    if (ret != 0) {
+        DEBUG(SSSDBG_MINOR_FAILURE, "getnameinfo failed\n");
+        return;
+    }
+
+    DEBUG(SSSDBG_TRACE_INTERNAL, "Searching %s\n", ip);
+}
+
 /* ==Generic Search exposing all options======================= */
 typedef errno_t (*sdap_parse_cb)(struct sdap_handle *sh,
                                  struct sdap_msg *msg,
@@ -1172,6 +1207,8 @@ sdap_get_generic_ext_send(TALLOC_CTX *memctx,
         return req;
     }
 
+    sdap_print_server(sh);
+
     /* Be extra careful and never allow paging for BASE searches,
      * even if requested.
      */
-- 
1.9.3



More information about the sssd-devel mailing list