[SSSD] [PATCH] Check our libraries properly resolve symbols

Lukas Slebodnik lslebodn at redhat.com
Mon Sep 9 15:29:49 UTC 2013


On (09/09/13 09:52), Lukas Slebodnik wrote:
>On (07/09/13 13:17), Simo Sorce wrote:
>>On Sat, 2013-09-07 at 12:39 -0400, Simo Sorce wrote:
>>> On Sat, 2013-09-07 at 11:42 +0200, Lukas Slebodnik wrote:
>>> > >+libdlopen_test_providers_la_LDFLAGS = \
>>> > >+    -module \
>>> > >+    -avoid-version \
>>> > >+    -Wl,--version-script,$(srcdir)/src/providers/sssd_be.exports \
>>> > add "-rpath $(abs_builddir) \"
>>> > libdlopen_test_providers will be dynamic library with this line.
>>> > 
>>> I had already tried this trick and it did not work, but the exact line I
>>> used was slightly different.
>>
>>Ah it did work with this syntax, I had tried with -Wl,-rpath previously.
>>Thanks a lot.
>>
>>> I'll retry and if it works I'll send an updated patch I do preferer
>>> check_LTLIBRARY as well, rather than the current hack.
>>
>>so now check_LTLIBRARY is used instead, moreover I added guards around
>>libraries that we compiled only conditionally, I had to add a couple of
>>statements in m4 files to make stuff appear in config.in.
>>Also I added the 2 missing python bindings.
>>
>>This should be final, it all works here.
>>
>>Simo.
>>
>>-- 
>>Simo Sorce * Red Hat, Inc * New York
>
>>From a957e4642ec49a36831270c8007bcc8d7818db90 Mon Sep 17 00:00:00 2001
>>From: Simo Sorce <simo at redhat.com>
>>Date: Thu, 5 Sep 2013 11:52:08 -0400
>>Subject: [PATCH] tests: Add dlopen test to make sure modules works
>>
>>This tests dlopens and resolves all symbols to make sure there are no missing
>>symbols in our provider modules.
>>---
>> Makefile.am              |  31 ++++++++++
>> configure.ac             |   1 +
>> src/external/krb5.m4     |   3 +-
>> src/tests/dlopen-tests.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 189 insertions(+), 1 deletion(-)
>> create mode 100644 src/tests/dlopen-tests.c
>>
>>diff --git a/Makefile.am b/Makefile.am
>>index b913a12b895d68f1f3e23c185e493e576641d0e2..bddb403b373ecee21f6ad41f36e69338b4cbb5e2 100644
>>--- a/Makefile.am
>>+++ b/Makefile.am
>>@@ -114,6 +114,7 @@ endif
>> 
>> if HAVE_CHECK
>>     non_interactive_check_based_tests = \
>>+        dlopen-tests \
>>         sysdb-tests \
>>         strtonum-tests \
>>         resolv-tests \
>>@@ -886,6 +887,36 @@ if HAVE_CHECK
>> libsss_test_common_la_SOURCES += \
>>     src/tests/common_check.c
>> 
>>+check_LTLIBRARIES = \
>>+    libdlopen_test_providers.la
>>+
>>+libdlopen_test_providers_la_SOURCES = \
>>+    $(sssd_be_SOURCES)
>>+libdlopen_test_providers_la_CFLAGS = \
>>+    $(AM_CFLAGS) \
>>+    $(CHECK_CFLAGS) \
>>+    -DUNIT_TESTING
>>+libdlopen_test_providers_la_LIBADD = \
>>+    $(PAM_LIBS) \
>>+    $(SSSD_LIBS) \
>>+    $(CARES_LIBS) \
>>+    $(SSSD_INTERNAL_LTLIBS)
>>+libdlopen_test_providers_la_LDFLAGS = \
>>+    -module \
>>+    -avoid-version \
>>+    -Wl,--version-script,$(srcdir)/src/providers/sssd_be.exports \
>>+    -rpath $(abs_top_builddir) \
>>+    -export-dynamic
>>+
>>+dlopen_tests_SOURCES = \
>>+    src/tests/dlopen-tests.c
>>+dlopen_tests_CFLAGS = \
>>+    $(AM_CFLAGS) \
>>+    $(CHECK_CFLAGS)
>>+dlopen_tests_LDADD = \
>>+    -ldl \
Please use $(LIBADD_DL) instead of "-ldl"
I did not noticed it first time.

"-ldl" was relaced in commit "AUTOMAKE: Use portable way to link with dlopen"
ccf340e56364851f2e5b75e52d3d63701b662954

LS

>>+    $(CHECK_LIBS)
>>+

>>     $(ldblib_LTLIBRARIES)
>> sysdb_tests_SOURCES = \
>>diff --git a/configure.ac b/configure.ac
>>index 511e8d6c8afbfb0cbf5aca076b460aa4c881e252..586fe0bf9dbda5c65175d491e0044f8f09650b39 100644
>>--- a/configure.ac
>>+++ b/configure.ac
>>@@ -227,6 +227,7 @@ if test x$HAVE_PYTHON_BINDINGS != x; then
>>     AM_PATH_PYTHON([2.4])
>>     AM_CHECK_PYTHON_HEADERS([],
>>                             AC_MSG_ERROR([Could not find python headers]))
>>+    AC_DEFINE_UNQUOTED(HAVE_PYTHON_BINDINGS, 1, [Build with python bindings])
>I would prefer to have this line in the file conf_macros.m4
>directly after the line
>   src/conf_macros.m4:333: AM_CONDITIONAL([BUILD_PYTHON_BINDINGS],
>
>In this file, there is a lot of similar patterns
>
>
>>     AM_PYTHON_CONFIG
>>     AM_CHECK_PYTHON_COMPAT
>> fi
>>diff --git a/src/external/krb5.m4 b/src/external/krb5.m4
>>index bdbd71ba1f395eb4f78047f75841729e0289ae81..a0da44a887329fbb2173a5849497d0b2cf1729a7 100644
>>--- a/src/external/krb5.m4
>>+++ b/src/external/krb5.m4
>>@@ -90,4 +90,5 @@ AC_CHECK_HEADER([krb5/locate_plugin.h],
>>                 ])
>> AM_CONDITIONAL([BUILD_KRB5_LOCATOR_PLUGIN],
>>                [test x$have_locate_plugin = xyes -a x$build_locator = xyes])
>>-
>>+AM_COND_IF([BUILD_KRB5_LOCATOR_PLUGIN],
>>+           [AC_DEFINE_UNQUOTED(HAVE_KRB5_LOCATOR_PLUGIN, 1, [Build with krb5 locator plugin])])
>>diff --git a/src/tests/dlopen-tests.c b/src/tests/dlopen-tests.c
>>new file mode 100644
>>index 0000000000000000000000000000000000000000..608b6106d6584a528a819cc2abdf1e21469d8d38
>>--- /dev/null
>>+++ b/src/tests/dlopen-tests.c
>>@@ -0,0 +1,155 @@
>>+/*
>>+    SSSD
>>+
>>+    debug-tests.c
>>+
>>+    Authors:
>>+        Simo Sorce <simo at redhat.com>
>>+
>>+    Copyright (C) 2013 Red Hat
>>+
>>+    This program is free software; you can redistribute it and/or modify
>>+    it under the terms of the GNU General Public License as published by
>>+    the Free Software Foundation; either version 3 of the License, or
>>+    (at your option) any later version.
>>+
>>+    This program is distributed in the hope that it will be useful,
>>+    but WITHOUT ANY WARRANTY; without even the implied warranty of
>>+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>+    GNU General Public License for more details.
>>+
>>+    You should have received a copy of the GNU General Public License
>>+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>+*/
>>+
>>+#define _GNU_SOURCE
>>+#include <stdbool.h>
>>+#include <dlfcn.h>
>>+#include <stdio.h>
>>+#include <stdlib.h>
>>+#include <limits.h>
>>+#include <check.h>
>>+#include "tests/common.h"
>>+
>>+#define LIBPFX ABS_BUILD_DIR"/.libs/"
>>+
>>+struct so {
>>+    const char *name;
>>+    const char *libs[6];
>>+} so[] = {
>>+    { "libsss_debug.so", { LIBPFX"libsss_debug.so", NULL } },
>>+    { "libsss_sudo.so", { LIBPFX"libsss_sudo.so", NULL } },
>add check for BUILD_SUDO
>>+    { "libipa_hbac.so", { LIBPFX"libipa_hbac.so", NULL } },
>>+    { "libsss_autofs.so", { LIBPFX"libsss_autofs.so", NULL } },
>add check for BUILD_AUTOFS
>>+    { "libsss_idmap.so", { LIBPFX"libsss_idmap.so", NULL } },
>>+    { "libsss_nss_idmap.so", { LIBPFX"libsss_nss_idmap.so", NULL } },
>>+    { "libnss_sss.so", { LIBPFX"libnss_sss.so", NULL } },
>>+    { "pam_sss.so", { LIBPFX"pam_sss.so", NULL } },
>>+#ifdef HAVE_KRB5_LOCATOR_PLUGIN
>>+    { "sssd_krb5_locator_plugin.so", { LIBPFX"sssd_krb5_locator_plugin.so",
>>+                                       NULL } },
>>+#endif
>>+#ifdef HAVE_PAC_RESPONDER
>>+    { "sssd_pac_plugin.so", { LIBPFX"sssd_pac_plugin.so", NULL } },
>>+#endif
>>+    { "memberof.so", { LIBPFX"memberof.so", NULL } },
>>+    { "libsss_child.so", { "libtevent.so",
>>+                           LIBPFX"libsss_debug.so",
>>+                           LIBPFX"libsss_crypt.so",
>>+                           LIBPFX"libsss_util.so",
>>+                           LIBPFX"libsss_child.so", NULL } },
>>+    { "libsss_crypt.so", { "libtalloc.so",
>>+                           LIBPFX"libsss_debug.so",
>>+                           LIBPFX"libsss_crypt.so", NULL } },
>>+    { "libsss_util.so", { "libtalloc.so",
>>+                           LIBPFX"libsss_debug.so",
>>+                           LIBPFX"libsss_crypt.so",
>>+                           LIBPFX"libsss_util.so", NULL } },
>>+    { "libsss_simple.so", { LIBPFX"libdlopen_test_providers.so",
>>+                            LIBPFX"libsss_simple.so", NULL } },
>>+    { "libsss_ad.so", { LIBPFX"libdlopen_test_providers.so",
>>+                        LIBPFX"libsss_ad.so", NULL } },
>>+    { "libsss_ipa.so", { LIBPFX"libdlopen_test_providers.so",
>>+                         LIBPFX"libsss_ipa.so", NULL } },
>>+    { "libsss_krb5.so", { LIBPFX"libdlopen_test_providers.so",
>>+                          LIBPFX"libsss_krb5.so", NULL } },
>>+    { "libsss_ldap.so", { LIBPFX"libdlopen_test_providers.so",
>>+                          LIBPFX"libsss_ldap.so", NULL } },
>>+    { "libsss_proxy.so", { LIBPFX"libdlopen_test_providers.so",
>>+                           LIBPFX"libsss_proxy.so", NULL } },
>>+#ifdef BUILD_PYTHON_BINDINGS
>You defined macro HAVE_PYTHON_BINDINGS, but BUILD_PYTHON_BINDINGS is used here.
>This is a dead code.
>>+    { "pyhbac.so", { LIBPFX"pyhbac.so", NULL } },
>>+    { "pysss.so", { LIBPFX"pysss.so", NULL } },
>>+    { "pysss_murmur.so", { LIBPFX"pysss_murmur.so", NULL } },
>>+    { "pysss_nss_idmap.so", { LIBPFX"pysss_nss_idmap.so", NULL } },
>>+#endif
>
>btw, BUILD_AUTOFS is used elsewhere in code.
>  src/tests/sysdb-tests.c:4707:#ifdef BUILD_AUTOFS
>  src/tests/sysdb-tests.c:4913:#endif /* BUILD_AUTOFS */
>  src/tests/sysdb-tests.c:5224:#ifdef BUILD_AUTOFS
>
>So we can use either BUILD_PYTHON_BINDINGS, BUILD_KRB5_LOCATOR_PLUGIN
>or we should file a ticket to be consistent.
>
>LS
>_______________________________________________
>sssd-devel mailing list
>sssd-devel at lists.fedorahosted.org
>https://lists.fedorahosted.org/mailman/listinfo/sssd-devel



More information about the sssd-devel mailing list