On Mon, May 13, 2013 at 05:00:42PM +0200, Ondrej Kos wrote:
Hi,
Attached is patch for the following trac issue
https://fedorahosted.org/sssd/ticket/1785
Please feel free to correct my wording of the syslog messages.
See some comments inline.
--- a/configure.ac
+++ b/configure.ac
@@ -166,6 +166,10 @@ fi
WITH_LIBNL
+if test x$have_nscd; then
+ WITH_NSCD_CONF
+fi
+
Where does $have_nscd come from? git grep have_nscd returns nothing.
WITH_INITSCRIPT
if test x$initscript = xsystemd; then
WITH_SYSTEMD_UNIT_DIR
diff --git a/src/conf_macros.m4 b/src/conf_macros.m4
index 26eb4acccc4895e961ecc50d490a3cdd5716d085..1315fc9da8761a62402692ae6f272236955c2d1c
100644
--- a/src/conf_macros.m4
+++ b/src/conf_macros.m4
@@ -352,6 +352,18 @@ AC_DEFUN([WITH_NSCD],
AC_DEFINE_UNQUOTED(HAVE_NSCD, $NSCD_PATH, [flush nscd cache after local domain
operations])
])
+AC_DEFUN([WITH_NSCD_CONF],
+ [ AC_ARG_WITH([nscd_conf],
+ [AC_HELP_STRING([--with-nscd-conf=PATH],
+ [Path to nscd.conf file [/etc/nscd.conf]])])
+ NSCD_CONF_PATH="/etc/nscd.conf"
+ if test x"$with_nscd_conf" != x; then
+ NSCD_CONF_PATH=$with_nscd_conf
+ AC_SUBST(NSCD_CONF_PATH)
Why is AC_SUBST used here? I don't see any @NSCD_CONF_PATH@.
+ fi
+ AC_DEFINE_UNQUOTED([NSCD_CONF_PATH], ["$NSCD_CONF_PATH"], [NSCD
configuration file])
+ ])
+
AC_DEFUN([WITH_SEMANAGE],
[ AC_ARG_WITH([semanage],
[AC_HELP_STRING([--with-semanage],
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index bd22a951d3b48d0ea367e36f628b07e2783d469a..6a69a76fe49a4d6d19561b5d4ae5bed97932b396
100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -2754,12 +2754,29 @@ int main(int argc, const char *argv[])
/* Warn if nscd seems to be running */
ret = check_file(NSCD_SOCKET_PATH, -1, -1, -1, CHECK_SOCK, NULL, false);
if (ret == EOK) {
- sss_log(SSS_LOG_NOTICE,
- "nscd socket was detected. Nscd caching capabilities "
- "may conflict with SSSD for users and groups. It is "
- "recommended not to run nscd in parallel with SSSD, unless "
- "nscd is configured not to cache the passwd, group and "
- "netgroup nsswitch maps.");
+ ret = sss_nscd_parse_conf();
+
+ switch (ret) {
+ case ENOENT:
+ sss_log(SSS_LOG_NOTICE,
+ "NSCD socket was detected, but NSCD configuration "
+ "file was not found in the specified path [%s] to "
+ "perform check of its caching settings.",
+ NSCD_CONF_PATH);
I would prefer the original sss_log message here instead of an error
message.
diff --git a/src/util/util.c b/src/util/util.c
index ba85e0da2a2e03784c5bb9abbbd038ddaeb3455c..8034ee63c632538af74ff7a3238f69cf914a6895
100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -688,3 +688,84 @@ void safezero(void *data, size_t size)
*p++ = 0;
}
}
+
+/* NSCD config file parse and check */
+
+#define NSCD_LINE_BUFF 120
+
+enum nscd_service_type {
+ NSCD_PASSWD = 0,
+ NSCD_GROUP,
+ NSCD_NETGROUP,
+ NSCD_SERVICES
+};
+
+unsigned int nscd_service_flag[4] = {
+ 0x0001, /* passwd */
+ 0x0010, /* group */
+ 0x0100, /* netgroup */
+ 0x1000 /* service */
+};
+
+const char *svc_type_name[] = {
+ "passwd",
+ "group",
+ "netgroup",
+ "services"
+};
+
There could be only one structure instead:
struct sss_nscd_db {
const char *svc_type_name;
int nscd_service_flag;
};
The in sss_nscd_check_service you'd have:
struct sss_nscd_db db[] = {
{ passwd, 0x0001 },
...
{ NULL, NULL }
};
And the for loop would iterate until db[i].name != NULL.
This way you don't have to sync three structures. The function could
even be inside the function namespace.
+unsigned int sss_nscd_check_service(char* svc_name)
+{
+ int i;
+
+ unsigned int ret = 0;
+
+ for (i = NSCD_PASSWD; i <= NSCD_SERVICES; i++) {
+ if (!strcmp(svc_type_name[i], svc_name)) {
+
+ ret |= nscd_service_flag[i];
+ break;
+ }
+ }
+
+ return ret;
+}
+
+errno_t sss_nscd_parse_conf(void)
+{
+ FILE *fp;
+ unsigned int occured = 0;
+ char line[NSCD_LINE_BUFF];
+ char part1[40], part2[40], part3[40];
+
+ fp = fopen(NSCD_CONF_PATH, "r");
+
+ if (fp == NULL) {
+ DEBUG(SSSDBG_MINOR_FAILURE, ("Couldn't open NSCD configuration "
+ "file [%s]\n", NSCD_CONF_PATH));
+ return ENOENT;
+ }
+
+ while (fgets(line, NSCD_LINE_BUFF, fp) != NULL) {
What about getline instead of gets?
+
+ if (line[0] != '#') {
+
It's more readable to put the main ("success") flow of the program in
the outmost block. So instead of:
if not_a_comment:
process()
it is more readable:
if comment:
continue
process()
+ sscanf(line, "%s %s %s", part1, part2, part3);
Do you know if the parts can only be separated by spaces? I think also
tabs could do.
Also if any part was larger than 40 chars, you'd get a segfault here.
In glibc, the parsing is done in nscd/nscd_conf.c, mostly in
nscd_parse_file() if you'd like to take a look.