[SSSD] [PATCH] - Improved detection of user domain

Lukas Slebodnik lslebodn at redhat.com
Thu Nov 28 09:31:13 UTC 2013


On (27/11/13 15:24), Pavel Reichl wrote:
>On Tue, 2013-11-26 at 18:25 +0100, Jakub Hrozek wrote:
>> On Tue, Nov 26, 2013 at 05:28:18PM +0100, Pavel Reichl wrote:
>> > Hi Jakub,
>> > 
>> > thanks for review. I have just a few little remarks, please see inline.
>> > 
>> > On Tue, 2013-11-26 at 15:23 +0100, Jakub Hrozek wrote:
>> > > On Tue, Nov 19, 2013 at 10:14:49PM +0100, Pavel Reichl wrote:
>> > > > On Fri, 2013-11-15 at 21:01 +0100, Jakub Hrozek wrote:
>> > > > > On Fri, Nov 15, 2013 at 11:15:53AM +0100, Pavel Reichl wrote:
>> > > > > > Hello,
>> > > > > > 
>> > > > > > First patch improves domain detection taking matched length into account
>> > > > > > as proposed and elaborated by Jakub. 
>> > > > > > 
>> > > > > > Second patch is a simple unit test testing sss_ldap_dn_in_search_bases
>> > > > > > and sdap_domain_get_by_dn.
>> > > > > > 
>> > > > > > Pavel Reichl
>> > > > > 
>> > > > > Hi Pavel,
>> > > > > 
>> > > > > I haven't really read the full patches yet, just scrolled through them,
>> > > > > so I have only one comment so far:
>> > > > > 
>> > > > > > +++ b/src/tests/cmocka/test_search_bases.c
>> > > > > > @@ -0,0 +1,214 @@
>> > > > > > +/*
>> > > > > > +    SSSD
>> > > > > > +
>> > > > > > +    find_uid - Utilities tests
>> > > > > > +
>> > > > > > +    Authors:
>> > > > > > +        Abhishek Singh <abhishekkumarsingh.cse at gmail.com>
>> > > > > > +
>> > > > > > +    Copyright (C) 2013 Red Hat
>> > > > > 
>> > > > > You should credit yourself :-)
>> > > > 
>> > > > Hi Jakub,
>> > > > 
>> > > > you are right, thanks for noticing.
>> > > 
>> > > Hi, in general the patches are working well and looking good. I only have
>> > > a couple of comments, see inline.
>> > > 
>> > > > From c7632a2310442cfc10708c5728bd63e6a8c916d2 Mon Sep 17 00:00:00 2001
>> > > > From: Pavel Reichl <pavel.reichl at redhat.com>
>> > > > Date: Thu, 14 Nov 2013 21:34:51 +0000
>> > > > Subject: [PATCH 1/2] SSSD: Improved domain detection
>> > > > 
>> > > > A bit more elegant way of detection of what domain the group member belongs to
>> > > > 
>> > > > Resolves:
>> > > > https://fedorahosted.org/sssd/ticket/2132
>> > > 
>> > > I only have code style comments about this patch.
>> > > 
>> > > > +    tmp_ctx = talloc_new(NULL);
>> > > > +    if (tmp_ctx == NULL)
>> > > > +        return NULL;
>> > > 
>> > > We use brackets around single-line conditionals as well.
>> > 
>> > OK, I have no trouble fixing this, but accordingly to
>> > http://www.freeipa.org/page/Coding_Style#IF_Statements
>> > I got an idea that statements like:
>> >    
>> >    if (foo)
>> >        bar();
>> >    else
>> >        baz();
>> > 
>> > are legal so I assumed that:
>> >    
>> >    if (foo)
>> >        bar();
>> > 
>> > would be legal too. So it may be a good idea to clarify this in above
>> > mentioned document or maybe It's just my bad reading. :-) 
>> 
>> Per IPA developers not using braces is discouraged. I agree the wiki
>> page should be clarified.
>> _______________________________________________
>> sssd-devel mailing list
>> sssd-devel at lists.fedorahosted.org
>> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
>Hi,
>
>thanks for information. Improved patches are attached.
>
>PR
>

>From 669ba173b0daaef99fab6ea1a8d5e5eb71c2cea1 Mon Sep 17 00:00:00 2001
>From: Pavel Reichl <pavel.reichl at redhat.com>
>Date: Thu, 14 Nov 2013 21:34:51 +0000
>Subject: [PATCH 1/2] SSSD: Improved domain detection
>

>From cdd879924d048de33b604a0730369e05cae8a813 Mon Sep 17 00:00:00 2001
>From: Pavel Reichl <pavel.reichl at redhat.com>
>Date: Thu, 14 Nov 2013 21:52:26 +0000
>Subject: [PATCH 2/2] SSSD: Unit test - sss_ldap_dn_in_search_bases
>
>Unit test testing detection of the right domain when processing group with members from several domains
>
>Resolves:
>https://fedorahosted.org/sssd/ticket/2132
>---
> Makefile.am                          |  27 ++++-
> src/tests/cmocka/test_search_bases.c | 191 +++++++++++++++++++++++++++++++++++
> 2 files changed, 217 insertions(+), 1 deletion(-)
> create mode 100644 src/tests/cmocka/test_search_bases.c
>
>diff --git a/Makefile.am b/Makefile.am
>index 992d5796ea3e729238a8d7664c128112291c40ab..ed155bf25c1d1adfe91a7c3c76cdbce62bf6ae59 100644
>--- a/Makefile.am
>+++ b/Makefile.am
>@@ -154,7 +154,8 @@ if HAVE_CMOCKA
>         fqnames-tests \
>         test_sss_idmap \
>         test_utils \
>-        ad_access_filter_tests
>+        ad_access_filter_tests \
>+        test_search_bases
> endif
> 
> check_PROGRAMS = \
>@@ -1388,6 +1389,30 @@ test_utils_LDADD = \
>     $(SSSD_INTERNAL_LTLIBS) \
>     libsss_test_common.la
> 
>+test_search_bases_SOURCES = \
>+    $(sssd_be_SOURCES) \
>+    src/util/sss_ldap.c \
>+    src/util/sss_krb5.c \
>+    src/util/find_uid.c \
>+    src/util/user_info_msg.c \
>+    src/tests/cmocka/test_search_bases.c
>+test_search_bases_CFLAGS = \
>+    $(AM_CFLAGS) \
>+    -DUNIT_TESTING
>+test_search_bases_LDADD = \
>+    $(PAM_LIBS) \
>+    $(CMOCKA_LIBS) \
>+    $(POPT_LIBS) \
>+    $(SSSD_LIBS) \
>+    $(CARES_LIBS) \
>+    $(KRB5_LIBS) \
>+    $(SSSD_INTERNAL_LTLIBS) \
>+    $(SYSTEMD_LOGIN_LIBS) \
>+    libsss_idmap.la \
>+    libsss_ldap_common.la \
>+    libsss_krb5_common.la \
>+    libsss_test_common.la
>+

Compilation fails with LDFLAG --as-needed

  CCLD     test_search_bases
./.libs/libsss_ldap_common.so: undefined reference to `is_domain_sid'
./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_bin_sid_to_sid'
./.libs/libsss_ldap_common.so: undefined reference to `idmap_error_string'
./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_ctx_set_autorid'
./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_ctx_set_rangesize'
./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_domain_by_name_has_algorithmic_mapping'
./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_free_sid'
./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_init'
./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_sid_to_unix'
./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_ctx_set_upper'
./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_calculate_range'
./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_ctx_set_lower'
./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_add_domain_ex'
./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_ctx_get_upper'
./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_unix_to_sid'
./.libs/libsss_ldap_common.so: undefined reference to `sss_idmap_domain_has_algorithmic_mapping'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [test_search_bases] Error 1
make[2]: Leaving directory `/dev/shm/sssd_build'
make[1]: *** [check-am] Error 2
make[1]: Leaving directory `/dev/shm/sssd_build'
make: *** [check-recursive] Error 1

Please change order of libraries.
diff --git a/Makefile.am b/Makefile.am
index 239a248..7fdb315 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1409,8 +1409,8 @@ test_search_bases_LDADD = \
     $(KRB5_LIBS) \
     $(SSSD_INTERNAL_LTLIBS) \
     $(SYSTEMD_LOGIN_LIBS) \
-    libsss_idmap.la \
     libsss_ldap_common.la \
+    libsss_idmap.la \
     libsss_krb5_common.la \
     libsss_test_common.la

LS



More information about the sssd-devel mailing list