[SSSD] [PATCH] Two deref fixes

Jakub Hrozek jhrozek at redhat.com
Fri Oct 21 07:41:42 UTC 2011


Simo was studying the dereference code lately and found two issues. #1
is an important bug, #2 is more of a cosmetic issue.

[PATCH 1/2] Use LDAPDerefSpec properly
ldap_create_deref_control_value expects an array of LDAPDerefSpec structures
with LDAPDerefSpec.derefAttr == NULL as a sentinel. We were passing a
single instance of a LDAPDerefSpec structure.

https://fedorahosted.org/sssd/ticket/1050


[PATCH 2/2] Remove confusing do-while loop
The deref processing would return a single control back. The do-while
loop was harmless but confusing.

-------------- next part --------------
From bbc10f69ab97eb7950517de903b442159a8179f9 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 21 Oct 2011 09:04:35 +0200
Subject: [PATCH 1/2] Use LDAPDerefSpec properly

ldap_create_deref_control_value expects an array of LDAPDerefSpec structures
with LDAPDerefSpec.derefAttr == NULL as a sentinel. We were passing a
single instance of a LDAPDerefSpec structure.

https://fedorahosted.org/sssd/ticket/1050
---
 src/providers/ldap/sdap_async.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
index 5d7019988fc3ec373179c6956abfa0e32b56a620..33664f146ec97e7b245bc8289c32634a5b9ce578 100644
--- a/src/providers/ldap/sdap_async.c
+++ b/src/providers/ldap/sdap_async.c
@@ -1371,12 +1371,14 @@ static int sdap_x_deref_create_control(struct sdap_handle *sh,
 {
     struct berval derefval;
     int ret;
-    static LDAPDerefSpec ds;
+    struct LDAPDerefSpec ds[2];
 
-    ds.derefAttr = discard_const(deref_attr);
-    ds.attributes = discard_const(attrs);
+    ds[0].derefAttr = discard_const(deref_attr);
+    ds[0].attributes = discard_const(attrs);
 
-    ret = ldap_create_deref_control_value(sh->ldap, &ds, &derefval);
+    ds[1].derefAttr = NULL; /* sentinel */
+
+    ret = ldap_create_deref_control_value(sh->ldap, ds, &derefval);
     if (ret != LDAP_SUCCESS) {
         DEBUG(1, ("sss_ldap_control_create failed: %s\n",
                   ldap_err2string(ret)));
-- 
1.7.6.4

-------------- next part --------------
From ebcb6afd9f432510c846ed312fa37ae95e94d265 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 21 Oct 2011 09:23:36 +0200
Subject: [PATCH 2/2] Remove confusing do-while loop

The deref processing would return a single control back. The do-while
loop was harmless but confusing.
---
 src/providers/ldap/sdap_async.c |   65 +++++++++++++++++++--------------------
 1 files changed, 32 insertions(+), 33 deletions(-)

diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
index 33664f146ec97e7b245bc8289c32634a5b9ce578..5a49bdd3387a835fcb0eb7d4f1d6507c155e82ea 100644
--- a/src/providers/ldap/sdap_async.c
+++ b/src/providers/ldap/sdap_async.c
@@ -1402,8 +1402,6 @@ static errno_t sdap_x_deref_parse_entry(struct sdap_handle *sh,
 {
     errno_t ret;
     LDAPControl **ctrls = NULL;
-    LDAPControl **next = NULL;
-    LDAPControl **start = NULL;
     LDAPControl *derefctrl = NULL;
     LDAPDerefRes    *deref_res = NULL;
     LDAPDerefRes    *dref;
@@ -1429,45 +1427,46 @@ static errno_t sdap_x_deref_parse_entry(struct sdap_handle *sh,
         goto done;
     }
 
-    start = ctrls;
     res = NULL;
-    do {
-        derefctrl = ldap_control_find(LDAP_CONTROL_X_DEREF, start, &next);
-        if (!derefctrl) break;
 
-        DEBUG(9, ("Got deref control\n"));
-        start = next;
+    derefctrl = ldap_control_find(LDAP_CONTROL_X_DEREF, ctrls, NULL);
+    if (!derefctrl) {
+        DEBUG(4, ("No deref controls found\n"));
+        ret = EOK;
+        goto done;
+    }
 
-        ret = ldap_parse_derefresponse_control(state->sh->ldap,
-                                               derefctrl,
-                                               &deref_res);
-        if (ret != LDAP_SUCCESS) {
-            DEBUG(1, ("ldap_parse_derefresponse_control failed: %s\n",
-                      ldap_err2string(ret)));
+    DEBUG(9, ("Got deref control\n"));
+
+    ret = ldap_parse_derefresponse_control(state->sh->ldap,
+                                            derefctrl,
+                                            &deref_res);
+    if (ret != LDAP_SUCCESS) {
+        DEBUG(1, ("ldap_parse_derefresponse_control failed: %s\n",
+                    ldap_err2string(ret)));
+        goto done;
+    }
+
+    for (dref = deref_res; dref; dref=dref->next) {
+        ret = sdap_parse_deref(tmp_ctx, state->maps, state->num_maps,
+                                state->sh, dref, &res);
+        if (ret) {
+            DEBUG(1, ("sdap_parse_deref failed [%d]: %s\n",
+                        ret, strerror(ret)));
             goto done;
         }
 
-        for (dref = deref_res; dref; dref=dref->next) {
-            ret = sdap_parse_deref(tmp_ctx, state->maps, state->num_maps,
-                                   state->sh, dref, &res);
-            if (ret) {
-                DEBUG(1, ("sdap_parse_deref failed [%d]: %s\n",
-                          ret, strerror(ret)));
-                goto done;
-            }
-
-            ret = add_to_deref_reply(state, state->num_maps,
-                                     &state->dreply, res);
-            if (ret != EOK) {
-                DEBUG(1, ("add_to_deref_reply failed.\n"));
-                goto done;
-            }
+        ret = add_to_deref_reply(state, state->num_maps,
+                                    &state->dreply, res);
+        if (ret != EOK) {
+            DEBUG(1, ("add_to_deref_reply failed.\n"));
+            goto done;
         }
+    }
 
-        DEBUG(9, ("All deref results from a single control parsed\n"));
-        ldap_derefresponse_free(deref_res);
-        deref_res = NULL;
-    } while (derefctrl);
+    DEBUG(9, ("All deref results from a single control parsed\n"));
+    ldap_derefresponse_free(deref_res);
+    deref_res = NULL;
 
     ret = EOK;
 done:
-- 
1.7.6.4



More information about the sssd-devel mailing list