On Thu, Sep 05, 2013 at 09:48:27PM -0400, Simo Sorce wrote:
While reviewing the recent big patchset I sent Sumit found out I did
not
add some .c files to the ldap provider (which I didn't test as I was
working on the krb5 provider), and that broke the provider's shared
library as one symbols was missing.
We allow modules to have unresolved symbols because a number of the
symbols they use come from our sssd_be binary so they can't be fully
resolved at build time, only at runtime.
The attached patch adds a test that simulates loading most of the shared
modules we build in order to verify that there are no unresolved
symbols, that is done by manually dlopen()ing eash .so library with
RTLD_NOW so all symbols are immediately resolved.
I tested that this test properly caught the issue Sumit found before
fixing it.
Simo.
Hi,
I think in general this is a great test to have, thank you. See some
minor comments below:
--
Simo Sorce * Red Hat, Inc * New York
From 53d8be3d2b543fec3aa83a7cfa86dcd87c7425d7 Mon Sep 17 00:00:00
2001
From: Simo Sorce <simo(a)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 | 23 ++++++++++
src/tests/dlopen-tests.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 135 insertions(+)
create mode 100644 src/tests/dlopen-tests.c
diff --git a/Makefile.am b/Makefile.am
index 05da9a14d82d4d0c11ec2eb888e6af46c6e48ff7..6aad7530120e783c9a1c937110b22b49cf6139eb
100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -114,6 +114,7 @@ endif
if HAVE_CHECK
non_interactive_check_based_tests = \
+ dlopen-tests \
Mixed tab and space
sysdb-tests \
strtonum-tests \
resolv-tests \
index
0000000000000000000000000000000000000000..4067162eb63feb0ebdf21b250f83bdfcb4654182
--- /dev/null
+++ b/src/tests/dlopen-tests.c
@@ -0,0 +1,112 @@
+/*
+ SSSD
+
+ debug-tests.c
+
+ Authors:
+ Pavel Březina <pbrezina(a)redhat.com>
You should credit yourself :)
+
+ Copyright (C) 2011 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 DLOPEN_TESTS_BUILDDIR"/.libs/"
We talked about this hardcoded directory on IRC and I can't see another
way to autodetect this either.
+
+const char *sonames[] = {
+ "libipa_hbac.so",
+ "libsss_ad.so",
+ "libsss_child.so",
+ "libsss_debug.so",
+ "libsss_ipa.so",
+ "libsss_krb5.so",
+ "libsss_ldap.so",
+ "libsss_proxy.so",
+ "libsss_sudo.so",
+ "memberof.so",
+ "pyhbac.so",
+ "pysss_nss_idmap.so",
+ "sssd_krb5_locator_plugin.so",
+ "libnss_sss.so",
+ "libsss_autofs.so",
+ "libsss_crypt.so",
+ "libsss_idmap.so",
+ "libsss_nss_idmap.so",
+ "libsss_simple.so",
+ "libsss_util.so",
+ "pam_sss.so",
+ "sssd_pac_plugin.so",
+ NULL
+};
I wonder if there's any way of autogenerating this maybe from
LTLIBRARIES? But definitely not something worth fixing in this patch
instance.
+
+START_TEST(test_dlopen_base)
+{
+ void *handle;
+ char *soname;
+ int ret, i;
+
+ for (i = 0; sonames[i] != NULL; i++) {
+ ret = asprintf(&soname, LIBPFX"%s", sonames[i]);
+ fail_unless(ret != -1, "Failed to construct soname");
+
+ handle = dlopen(soname, RTLD_NOW);
+ fail_unless(handle != NULL,
+ "dlopen() failed for %s: [%s]", soname, dlerror());
+ dlclose(handle);
+ free(soname);
+ }
+}
+END_TEST
+
+Suite *dlopen_suite(void)
+{
+ Suite *s = suite_create("dlopen");
+
+ TCase *tc_dlopen = tcase_create("dlopen");
+
+ tcase_add_test(tc_dlopen, test_dlopen_base);
+ tcase_set_timeout(tc_dlopen, 10);
+
+ suite_add_tcase(s, tc_dlopen);
+
+ return s;
+}
+
+int main(int argc, const char *argv[])
+{
+ int number_failed;
+
+ tests_set_cwd();
+
I think this test is a special case where we actually should not call
tests_set_cwd(). Calling it breaks the parallel build for me.
+ Suite *s = dlopen_suite();
+ SRunner *sr = srunner_create(s);
+
+ srunner_run_all(sr, CK_NORMAL);
+ number_failed = srunner_ntests_failed(sr);
+ srunner_free(sr);
+
+ if (number_failed == 0)
+ return EXIT_SUCCESS;
+
+ return EXIT_FAILURE;
+}
--
1.8.3.1
Otherwise looks good to me!