[SSSD] [PATCHES] Use namingContext to set empty search bases

sbose at redhat.com sbose at redhat.com
Fri Nov 5 11:23:23 UTC 2010


On Thu, Nov 04, 2010 at 01:42:31PM -0400, Stephen Gallagher wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 11/03/2010 09:19 AM, Sumit Bose wrote:
> > On Wed, Nov 03, 2010 at 08:48:21AM -0400, Simo Sorce wrote:
> >> On Tue, 2 Nov 2010 22:39:45 +0100
> >> Sumit Bose <sbose at redhat.com> wrote:
> >>
> > ....
> >>> memberOf=cn=allowedusers,ou=Groups,dc=example,dc=com </para>
> >>>                          <para>
> >>>                              Default: the value of
> >>> -                            <emphasis>ldap_search_base</emphasis>
> >>> +                            <emphasis>ldap_search_base</emphasis> or
> >>> the value
> >>> +                            of the namingContexts attribute of the
> >>> RootDSE of the
> >>> +                            LDAP server.
> >>>                          </para>
> >>>                      </listitem>
> >>>                  </varlistentry>
> >>
> >> NACK, with the change to support defaultNamingContext, these
> >> explanations are not correct anymore.
> > 
> > Thanks for catching this. New versions attached.
> > 
> 
> 
> Nack. Please clean up the tabs in sdap_get_rootdse_send() for the
> attrs[] list.
> 
> Please use a different loop-control variable than "o" in
> sdap_set_config_options_with_rootdse(). It's difficult to differentiate
> at a glance between search_base_options[o] and search_base_options[0].
> 
> It's probably less confusing to leave the ldap_*_search_base manpage
> entries saying that they default to the value of ldap_search_base.
> Reading the manpage entry for that option will provide all of the
> necessary information.
> 
> Right now, the way these specific options read is ambiguous (does it use
> ldap_search_base, namingContexts or defaultNamingContexts?).
> 
> 

I think you have already pushed the patches accidentally. Please find
attached a patch which adds your comments.

bye,
Sumit

> 
> - -- 
> Stephen Gallagher
> RHCE 804006346421761
> 
> Delivering value year after year.
> Red Hat ranks #1 in value among software vendors.
> http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
> 
> iEYEARECAAYFAkzS8IcACgkQeiVVYja6o6PkZQCgqtUzQyYXEqSdtl7gbl8V1AZq
> T6sAoJGa80W5gnKrtbuwNmR6Yxtv99nh
> =fjKc
> -----END PGP SIGNATURE-----
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel
-------------- next part --------------
From e98fd8ddab98e507acce69b1bd31e2db35c8ade2 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 5 Nov 2010 12:20:21 +0100
Subject: [PATCH] Review comments for namingContexts patches

---
 src/man/sssd-ldap.5.xml         |   12 +++---------
 src/providers/ldap/sdap.c       |   10 +++++-----
 src/providers/ldap/sdap_async.c |   18 +++++++++---------
 3 files changed, 17 insertions(+), 23 deletions(-)

diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml
index 2762362..2123ffd 100644
--- a/src/man/sssd-ldap.5.xml
+++ b/src/man/sssd-ldap.5.xml
@@ -1056,9 +1056,7 @@ ldap_access_filter = memberOf=cn=allowedusers,ou=Groups,dc=example,dc=com
                         </para>
                         <para>
                             Default: the value of
-                            <emphasis>ldap_search_base</emphasis> or the value
-                            of the defaultNamingContext or namingContexts
-                            attribute of the RootDSE of the LDAP server.
+                            <emphasis>ldap_search_base</emphasis>
                         </para>
                     </listitem>
                 </varlistentry>
@@ -1072,9 +1070,7 @@ ldap_access_filter = memberOf=cn=allowedusers,ou=Groups,dc=example,dc=com
                         </para>
                         <para>
                             Default: the value of
-                            <emphasis>ldap_search_base</emphasis> or the value
-                            of the defaultNamingContext or namingContexts
-                            attribute of the RootDSE of the LDAP server.
+                            <emphasis>ldap_search_base</emphasis>
                         </para>
                     </listitem>
                 </varlistentry>
@@ -1088,9 +1084,7 @@ ldap_access_filter = memberOf=cn=allowedusers,ou=Groups,dc=example,dc=com
                         </para>
                         <para>
                             Default: the value of
-                            <emphasis>ldap_search_base</emphasis> or the value
-                            of the defaultNamingContext or namingContexts
-                            attribute of the RootDSE of the LDAP server.
+                            <emphasis>ldap_search_base</emphasis>
                         </para>
                     </listitem>
                 </varlistentry>
diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
index cf4e9ed..95b63d1 100644
--- a/src/providers/ldap/sdap.c
+++ b/src/providers/ldap/sdap.c
@@ -474,11 +474,11 @@ errno_t sdap_set_config_options_with_rootdse(struct sysdb_attrs *rootdse,
                                         SDAP_GROUP_SEARCH_BASE,
                                         SDAP_NETGROUP_SEARCH_BASE,
                                         -1 };
-    int o;
+    size_t c;
 
 
-    for (o = 0; search_base_options[o] != -1; o++) {
-        if (dp_opt_get_string(opts->basic, search_base_options[o]) == NULL) {
+    for (c = 0; search_base_options[c] != -1; c++) {
+        if (dp_opt_get_string(opts->basic, search_base_options[c]) == NULL) {
             if (naming_context == NULL) {
                 naming_context = get_naming_context(opts->basic, rootdse);
                 if (naming_context == NULL) {
@@ -489,9 +489,9 @@ errno_t sdap_set_config_options_with_rootdse(struct sysdb_attrs *rootdse,
             }
 
             DEBUG(3, ("Setting option [%s] to [%s].\n",
-                      opts->basic[search_base_options[o]].opt_name,
+                      opts->basic[search_base_options[c]].opt_name,
                       naming_context));
-            ret = dp_opt_set_string(opts->basic, search_base_options[o],
+            ret = dp_opt_set_string(opts->basic, search_base_options[c],
                                     naming_context);
             if (ret != EOK) {
                 DEBUG(1, ("dp_opt_set_string failed.\n"));
diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
index 01cd967..6808cea 100644
--- a/src/providers/ldap/sdap_async.c
+++ b/src/providers/ldap/sdap_async.c
@@ -646,16 +646,16 @@ struct tevent_req *sdap_get_rootdse_send(TALLOC_CTX *memctx,
     struct tevent_req *req, *subreq;
     struct sdap_get_rootdse_state *state;
     const char *attrs[] = {
-	    "*",
-	    "altServer",
-	    SDAP_ROOTDSE_ATTR_NAMING_CONTEXTS,
-	    "supportedControl",
-	    "supportedExtension",
-	    "supportedFeatures",
-	    "supportedLDAPVersion",
-	    "supportedSASLMechanisms",
+            "*",
+            "altServer",
+            SDAP_ROOTDSE_ATTR_NAMING_CONTEXTS,
+            "supportedControl",
+            "supportedExtension",
+            "supportedFeatures",
+            "supportedLDAPVersion",
+            "supportedSASLMechanisms",
             SDAP_ROOTDSE_ATTR_DEFAULT_NAMING_CONTEXT,
-	    NULL
+            NULL
     };
 
     DEBUG(9, ("Getting rootdse\n"));
-- 
1.7.3.2



More information about the sssd-devel mailing list