[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