[PATCH v2 0/7] nfs-utils: nfsconf cli tool and code tests

[PATCH v2 0/7] nfs-utils: nfsconf cli tool and code tests

[Justin Mitchell]

v2
Updated style and commenting to make the code clearer

v1
This series introduces a new cli tool to nfs-utils which provides
for the dumping of the current configuration, querying of individual
settings in a scripting friendly manner and by extention the 
scripted testing and verification of nfsconf functionality.

The previously unreferenced conf_report() is modified to produce
a sorted output, which allows you to --dump the current config in
a way thats easier to diff

As well as some improved error messages a script is included which
tests the current config parsing code by supplying various example
config files and comparing the results of the --dump output.

The cli tool also allows a simple test to check if a config item
has been set, and to retrieve the current value in a way that is
easy to call from a shell script.

As the next stage I hope to impliment config setting via the cli 
tool in order to provide an api for automation and management tools

Signed-off-by: Justin Mitchell <jumitche@redhat.com>

[PATCH v2 1/7] nfs-utils: Fix minor memory leaks

[Justin Mitchell]

Fix some minor memory leaks, typos, and trailing whitespace

Signed-off-by: Justin Mitchell <jumitche@redhat.com>
---
 support/nfs/conffile.c | 42 +++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index 29f132d..28e29b7 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -57,9 +57,9 @@
 
 static void conf_load_defaults(void);
 static char * conf_readfile(const char *path);
-static int conf_set(int , const char *, const char *, const char *, 
+static int conf_set(int , const char *, const char *, const char *,
 	const char *, int , int );
-static void conf_parse(int trans, char *buf, 
+static void conf_parse(int trans, char *buf,
 	char **section, char **subsection);
 
 struct conf_trans {
@@ -177,7 +177,7 @@ conf_remove_section_now(const char *section)
  * into SECTION of our configuration database.
  */
 static int
-conf_set_now(const char *section, const char *arg, const char *tag, 
+conf_set_now(const char *section, const char *arg, const char *tag,
 	const char *value, int override, int is_default)
 {
 	struct conf_binding *node = 0;
@@ -186,7 +186,7 @@ conf_set_now(const char *section, const char *arg, const char *tag,
 		conf_remove_now(section, tag);
 	else if (conf_get_section(section, arg, tag)) {
 		if (!is_default) {
-			xlog(LOG_INFO, "conf_set: duplicate tag [%s]:%s, ignoring...\n", 
+			xlog(LOG_INFO, "conf_set: duplicate tag [%s]:%s, ignoring...",
 				section, tag);
 		}
 		return 1;
@@ -220,7 +220,7 @@ conf_parse_line(int trans, char *line, int lineno, char **section, char **subsec
 		return;
 
 	/* Strip off any leading blanks */
-	while (isblank(*line)) 
+	while (isblank(*line))
 		line++;
 
 	/* Lines starting with '#' or ';' are comments.  */
@@ -241,7 +241,7 @@ conf_parse_line(int trans, char *line, int lineno, char **section, char **subsec
 		}
 
 		/* Strip off any blanks after '[' */
-		while (isblank(*line)) 
+		while (isblank(*line))
 			line++;
 
 		/* find the closing ] */
@@ -256,7 +256,7 @@ conf_parse_line(int trans, char *line, int lineno, char **section, char **subsec
 		*(ptr--) = '\0';
 
 		/* Strip off any blanks before ']' */
-		while (ptr >= line && isblank(*ptr)) 
+		while (ptr >= line && isblank(*ptr))
 			*(ptr--)='\0';
 
 		/* look for an arg to split from the section name */
@@ -289,7 +289,7 @@ conf_parse_line(int trans, char *line, int lineno, char **section, char **subsec
 		}
 		*ptr = '\0';
 		*subsection = strdup(val);
-		if (!*subsection) 
+		if (!*subsection)
 			xlog_warn("conf_parse_line: %d: malloc arg failed", lineno);
 		return;
 	}
@@ -535,8 +535,8 @@ conf_init_file(const char *conf_file)
 	conf_load_file(conf_file);
 }
 
-/* 
- * Empty the config and free up any used memory 
+/*
+ * Empty the config and free up any used memory
  */
 void
 conf_cleanup(void)
@@ -618,7 +618,7 @@ conf_match_num(const char *section, const char *tag, int x)
 		xlog(LOG_INFO, "conf_match_num: %s:%s %d==%d?", section, tag, val, x);
 		return x == val;
 	case 3:
-		xlog(LOG_INFO, "conf_match_num: %s:%s %d<=%d<=%d?", section, 
+		xlog(LOG_INFO, "conf_match_num: %s:%s %d<=%d<=%d?", section,
 			tag, min, x, max);
 		return min <= x && max >= x;
 	default:
@@ -642,7 +642,7 @@ char *
 conf_get_str_with_def(const char *section, const char *tag, char *def)
 {
 	char * result = conf_get_section(section, NULL, tag);
-	if (!result) 
+	if (!result)
 		return def;
 	return result;
 }
@@ -659,7 +659,7 @@ retry:
 	for (; cb; cb = LIST_NEXT (cb, link)) {
 		if (strcasecmp(section, cb->section) != 0)
 			continue;
-		if (arg && strcasecmp(arg, cb->arg) != 0)
+		if (arg && (cb->arg == NULL || strcasecmp(arg, cb->arg) != 0))
 			continue;
 		if (strcasecmp(tag, cb->tag) != 0)
 			continue;
@@ -917,6 +917,8 @@ conf_set(int transaction, const char *section, const char *arg,
 fail:
 	if (node->tag)
 		free(node->tag);
+	if (node->arg)
+		free(node->arg);
 	if (node->section)
 		free(node->section);
 	if (node)
@@ -987,8 +989,8 @@ conf_end(int transaction, int commit)
 			if (commit) {
 				switch (node->op) {
 				case CONF_SET:
-					conf_set_now(node->section, node->arg, 
-						node->tag, node->value, node->override, 
+					conf_set_now(node->section, node->arg,
+						node->tag, node->value, node->override,
 						node->is_default);
 					break;
 				case CONF_REMOVE:
@@ -1004,6 +1006,8 @@ conf_end(int transaction, int commit)
 			TAILQ_REMOVE (&conf_trans_queue, node, link);
 			if (node->section)
 				free(node->section);
+			if (node->arg)
+				free(node->arg);
 			if (node->tag)
 				free(node->tag);
 			if (node->value)
@@ -1066,7 +1070,7 @@ conf_report (void)
 					diff_arg = 1;
 				}
 				/* Dump this entry.  */
-				if (!current_section || strcmp(cb->section, current_section) 
+				if (!current_section || strcmp(cb->section, current_section)
 							|| diff_arg) {
 					if (current_section || diff_arg) {
 						len = strlen (current_section) + 3;
@@ -1077,19 +1081,19 @@ conf_report (void)
 							goto mem_fail;
 
 						if (current_arg)
-							snprintf(dnode->s, len, "[%s \"%s\"]", 
+							snprintf(dnode->s, len, "[%s \"%s\"]",
 								current_section, current_arg);
 						else
 							snprintf(dnode->s, len, "[%s]", current_section);
 
-						dnode->next = 
+						dnode->next =
 							(struct dumper *)calloc(1, sizeof (struct dumper));
 						dnode = dnode->next;
 						if (!dnode)
 							goto mem_fail;
 
 						dnode->s = "";
-						dnode->next = 
+						dnode->next =
 							(struct dumper *)calloc(1, sizeof (struct dumper));
 						dnode = dnode->next;
 						if (!dnode)
-- 
1.8.3.1

[PATCH v2 2/7] nfs-utils: Make config includes relative to current config

[Justin Mitchell]

When including additional config files with the include directive
make relative paths be relative to the current config file instead
of to the process's cwd.

Signed-off-by: Justin Mitchell <jumitche@redhat.com>
---
 support/nfs/conffile.c | 76 +++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 63 insertions(+), 13 deletions(-)

diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index 28e29b7..95dda99 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -49,6 +49,7 @@
 #include <errno.h>
 #include <err.h>
 #include <syslog.h>
+#include <libgen.h>
 
 #include "conffile.h"
 #include "xlog.h"
@@ -60,7 +61,7 @@ static char * conf_readfile(const char *path);
 static int conf_set(int , const char *, const char *, const char *,
 	const char *, int , int );
 static void conf_parse(int trans, char *buf,
-	char **section, char **subsection);
+	char **section, char **subsection, const char *filename);
 
 struct conf_trans {
 	TAILQ_ENTRY (conf_trans) link;
@@ -206,12 +207,49 @@ conf_set_now(const char *section, const char *arg, const char *tag,
 	return 0;
 }
 
+/* Attempt to construct a relative path to the new file */
+static char *
+relative_path(const char *oldfile, const char *newfile)
+{
+	char *tmpcopy = NULL;
+	char *dir = NULL;
+	char *relpath = NULL;
+	size_t pathlen;
+
+	if (!newfile)
+		return strdup(oldfile);
+
+	if (newfile[0] == '/')
+		return strdup(newfile);
+
+	tmpcopy = strdup(oldfile);
+	if (!tmpcopy)
+		goto mem_err;
+
+	dir = dirname(tmpcopy);
+
+	pathlen = strlen(dir) + strlen(newfile) + 2;
+	relpath = calloc(1, pathlen);
+	if (!relpath)
+		goto mem_err;
+
+	snprintf(relpath, pathlen, "%s/%s", dir, newfile);
+
+	free(tmpcopy);
+	return relpath;
+mem_err:
+	if (tmpcopy)
+		free(tmpcopy);
+	return NULL;
+}
+
+
 /*
  * Parse the line LINE of SZ bytes.  Skip Comments, recognize section
  * headers and feed tag-value pairs into our configuration database.
  */
 static void
-conf_parse_line(int trans, char *line, int lineno, char **section, char **subsection)
+conf_parse_line(int trans, char *line, const char *filename, int lineno, char **section, char **subsection)
 {
 	char *val, *ptr;
 
@@ -366,27 +404,34 @@ conf_parse_line(int trans, char *line, int lineno, char **section, char **subsec
 
 	if (strcasecmp(line, "include")==0) {
 		/* load and parse subordinate config files */
-		char * subconf = conf_readfile(val);
+		char *inc_section = NULL;
+		char *inc_subsection = NULL;
+		char *relpath = relative_path(filename, val);
+		char *subconf = conf_readfile(relpath);
 		if (subconf == NULL) {
-			xlog_warn("config file error: line %d: "
-			"error loading included config", lineno);
+			xlog_warn("%s:%d: error loading included config",
+				  filename, lineno);
+			if (relpath)
+				free(relpath);
 			return;
 		}
 
 		/* copy the section data so the included file can inherit it
 		 * without accidentally changing it for us */
-		char * inc_section = NULL;
-		char * inc_subsection = NULL;
 		if (*section != NULL) {
 			inc_section = strdup(*section);
 			if (*subsection != NULL)
 				inc_subsection = strdup(*subsection);
 		}
 
-		conf_parse(trans, subconf, &inc_section, &inc_subsection);
+		conf_parse(trans, subconf, &inc_section, &inc_subsection, relpath);
 
-		if (inc_section) free(inc_section);
-		if (inc_subsection) free(inc_subsection);
+		if (inc_section)
+			free(inc_section);
+		if (inc_subsection)
+			free(inc_subsection);
+		if (relpath)
+			free(relpath);
 		free(subconf);
 	} else {
 		/* XXX Perhaps should we not ignore errors?  */
@@ -396,7 +441,7 @@ conf_parse_line(int trans, char *line, int lineno, char **section, char **subsec
 
 /* Parse the mapped configuration file.  */
 static void
-conf_parse(int trans, char *buf, char **section, char **subsection)
+conf_parse(int trans, char *buf, char **section, char **subsection, const char *filename)
 {
 	char *cp = buf;
 	char *bufend = NULL;
@@ -413,7 +458,7 @@ conf_parse(int trans, char *buf, char **section, char **subsection)
 			else {
 				*cp = '\0';
 				lineno++;
-				conf_parse_line(trans, line, lineno, section, subsection);
+				conf_parse_line(trans, line, filename, lineno, section, subsection);
 				line = cp + 1;
 			}
 		}
@@ -434,6 +479,11 @@ static char *
 conf_readfile(const char *path)
 {
 	struct stat sb;
+	if (!path) {
+		xlog_err("conf_readfile: no path given");
+		return NULL;
+	}
+
 	if ((stat (path, &sb) == 0) || (errno != ENOENT)) {
 		char *new_conf_addr = NULL;
 		size_t sz = sb.st_size;
@@ -508,7 +558,7 @@ conf_load_file(const char *conf_file)
 	/* Parse config contents into the transaction queue */
 	char *section = NULL;
 	char *subsection = NULL;
-	conf_parse(trans, conf_data, &section, &subsection);
+	conf_parse(trans, conf_data, &section, &subsection, conf_file);
 	if (section) free(section);
 	if (subsection) free(subsection);
 	free(conf_data);
-- 
1.8.3.1

[PATCH v2 3/7] nfs-utils: Use config file name in error messages

[Justin Mitchell]

Improve readability of error configuration error messages
by including the filename currently being read.

Signed-off-by: Justin Mitchell <jumitche@redhat.com>
---
 support/nfs/conffile.c | 45 ++++++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index 95dda99..5fb58ea 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -285,8 +285,9 @@ conf_parse_line(int trans, char *line, const char *filename, int lineno, char **
 		/* find the closing ] */
 		ptr = strchr(line, ']');
 		if (ptr == NULL) {
-			xlog_warn("config file error: line %d: "
- 				"non-matched ']', ignoring until next section", lineno);
+			xlog_warn("config error at %s:%d: "
+				  "non-matched ']', ignoring until next section",
+				  filename, lineno);
 			return;
 		}
 
@@ -311,7 +312,8 @@ conf_parse_line(int trans, char *line, const char *filename, int lineno, char **
 		/* copy the section name */
 		*section = strdup(line);
 		if (!*section) {
-			xlog_warn("conf_parse_line: %d: malloc failed", lineno);
+			xlog_warn("config error at %s:%d:"
+				  "malloc failed", filename, lineno);
 			return;
 		}
 
@@ -321,14 +323,16 @@ conf_parse_line(int trans, char *line, const char *filename, int lineno, char **
 		/* check for the closing " */
 		ptr = strchr(val, '"');
 		if (ptr == NULL) {
-			xlog_warn("config file error: line %d: "
- 				"non-matched '\"', ignoring until next section", lineno);
+			xlog_warn("config error at %s:%d: "
+				  "non-matched '\"', ignoring until next section",
+				filename, lineno);
 			return;
 		}
 		*ptr = '\0';
 		*subsection = strdup(val);
 		if (!*subsection)
-			xlog_warn("conf_parse_line: %d: malloc arg failed", lineno);
+			xlog_warn("config error at %s:%d: "
+				  "malloc failed", filename, lineno);
 		return;
 	}
 
@@ -339,15 +343,17 @@ conf_parse_line(int trans, char *line, const char *filename, int lineno, char **
 	if (ptr == NULL) {
 		/* Other non-empty lines are weird.  */
 		if (line[strspn(line, " \t")])
-			xlog_warn("config file error: line %d: "
-				"line not empty and not an assignment", lineno);
+			xlog_warn("config error at %s:%d: "
+				"line not empty and not an assignment",
+				filename, lineno);
 		return;
 	}
 
 	/* If no section, we are ignoring the line.  */
 	if (!*section) {
-		xlog_warn("config file error: line %d: "
-			"ignoring line due to no section", lineno);
+		xlog_warn("config error at %s:%d: "
+			  "ignoring line not in a section",
+			  filename, lineno);
 		return;
 	}
 
@@ -364,8 +370,8 @@ conf_parse_line(int trans, char *line, const char *filename, int lineno, char **
 		val++;
 		ptr = strchr(val, '"');
 		if (ptr == NULL) {
-			xlog_warn("config file error: line %d: "
-				"unmatched quotes", lineno);
+			xlog_warn("config error at %s:%d: "
+				  "unmatched quotes",filename, lineno);
 			return;
 		}
 		*ptr = '\0';
@@ -374,8 +380,8 @@ conf_parse_line(int trans, char *line, const char *filename, int lineno, char **
 		val++;
 		ptr = strchr(val, '\'');
 		if (ptr == NULL) {
-			xlog_warn("config file error: line %d: "
-				"unmatched quotes", lineno);
+			xlog_warn("config error at %s:%d: "
+				  "unmatched quotes", filename, lineno);
 			return;
 		}
 		*ptr = '\0';
@@ -392,13 +398,13 @@ conf_parse_line(int trans, char *line, const char *filename, int lineno, char **
 	}
 
 	if (*line == '\0') {
-		xlog_warn("config file error: line %d: "
-			"missing tag in assignment", lineno);
+		xlog_warn("config error at %s:%d: "
+			  "missing tag in assignment", filename, lineno);
 		return;
 	}
 	if (*val == '\0') {
-		xlog_warn("config file error: line %d: "
-			"missing value in assignment", lineno);
+		xlog_warn("config error at %s:%d: "
+			  "missing value in assignment", filename, lineno);
 		return;
 	}
 
@@ -409,7 +415,8 @@ conf_parse_line(int trans, char *line, const char *filename, int lineno, char **
 		char *relpath = relative_path(filename, val);
 		char *subconf = conf_readfile(relpath);
 		if (subconf == NULL) {
-			xlog_warn("%s:%d: error loading included config",
+			xlog_warn("config error at %s:%d: "
+				  "error loading included config",
 				  filename, lineno);
 			if (relpath)
 				free(relpath);
-- 
1.8.3.1

[PATCH v2 4/7] nfs-utils: Indicate if config file was missing

[Justin Mitchell]

Return an indication that the config file could not be loaded
for processes that want to differentiate this from empty config

Signed-off-by: Justin Mitchell <jumitche@redhat.com>
---
 support/include/conffile.h |  2 +-
 support/nfs/conffile.c     | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/support/include/conffile.h b/support/include/conffile.h
index ad20067..6baaf9a 100644
--- a/support/include/conffile.h
+++ b/support/include/conffile.h
@@ -60,7 +60,7 @@ extern _Bool    conf_get_bool(const char *, const char *, _Bool);
 extern char    *conf_get_str(const char *, const char *);
 extern char    *conf_get_str_with_def(const char *, const char *, char *);
 extern char    *conf_get_section(const char *, const char *, const char *);
-extern void     conf_init_file(const char *);
+extern int      conf_init_file(const char *);
 extern void     conf_cleanup(void);
 extern int      conf_match_num(const char *, const char *, int);
 extern int      conf_remove(int, const char *, const char *);
diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index 5fb58ea..e65caaf 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -547,7 +547,7 @@ static void conf_free_bindings(void)
 }
 
 /* Open the config file and map it into our address space, then parse it.  */
-static void
+static int
 conf_load_file(const char *conf_file)
 {
 	int trans;
@@ -557,7 +557,7 @@ conf_load_file(const char *conf_file)
 	conf_data = conf_readfile(conf_file);
 
 	if (conf_data == NULL)
-		return;
+		return 1;
 
 	/* Load default configuration values.  */
 	conf_load_defaults();
@@ -575,10 +575,10 @@ conf_load_file(const char *conf_file)
 
 	/* Apply the new configuration values */
 	conf_end(trans, 1);
-	return;
+	return 0;
 }
 
-void
+int
 conf_init_file(const char *conf_file)
 {
 	unsigned int i;
@@ -589,7 +589,7 @@ conf_init_file(const char *conf_file)
 	TAILQ_INIT (&conf_trans_queue);
 
 	if (conf_file == NULL) conf_file=NFS_CONFFILE;
-	conf_load_file(conf_file);
+	return conf_load_file(conf_file);
 }
 
 /*
-- 
1.8.3.1

[PATCH v2 5/7] nfs-utils: tidy up output of conf_report

[Justin Mitchell]

Previously unused conf_report() function is modified to output
an alphabetically sorted version of the current configuration
to the nominated FILE handle.

Signed-off-by: Justin Mitchell <jumitche@redhat.com>
---
 support/include/conffile.h |   3 +-
 support/nfs/conffile.c     | 293 +++++++++++++++++++++++++++++++--------------
 2 files changed, 207 insertions(+), 89 deletions(-)

diff --git a/support/include/conffile.h b/support/include/conffile.h
index 6baaf9a..bc2d61f 100644
--- a/support/include/conffile.h
+++ b/support/include/conffile.h
@@ -37,6 +37,7 @@
 #include <ctype.h>
 #include <stdint.h>
 #include <stdbool.h>
+#include <stdio.h>
 
 struct conf_list_node {
 	TAILQ_ENTRY(conf_list_node) link;
@@ -65,7 +66,7 @@ extern void     conf_cleanup(void);
 extern int      conf_match_num(const char *, const char *, int);
 extern int      conf_remove(int, const char *, const char *);
 extern int      conf_remove_section(int, const char *);
-extern void     conf_report(void);
+extern void     conf_report(FILE *);
 
 /*
  * Convert letter from upper case to lower case
diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index e65caaf..1e8034a 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -1080,117 +1080,234 @@ conf_end(int transaction, int commit)
  * Configuration is "stored in reverse order", so reverse it again.
  */
 struct dumper {
-	char *s, *v;
+	char *section;
+	char *arg;
+	char *tag;
+	char *value;
 	struct dumper *next;
 };
 
+/*
+ * Test if two nodes belong to the same (sub)sections
+ */
+static int
+dumper_section_compare(const struct dumper *nodea, const struct dumper *nodeb)
+{
+	int ret;
+
+	/* missing node, shouldnt happen */
+	if (!nodea || !nodeb)
+		return -1;
+
+	/* no section names at all, they are equal */
+	if (!nodea->section && !nodeb->section)
+		return 0;
+
+	/* if only one has a section name, the blank one goes first */
+	if (!nodea->section && nodeb->section)
+		return -1;
+
+	if (nodea->section && !nodeb->section)
+		return 1;
+
+	/* both have section names, but do they match */
+	ret = strcmp(nodea->section, nodeb->section);
+
+	/* section names differ, that was easy */
+	if (ret != 0)
+		return ret;
+
+	/* sections matched, but how about sub-sections,
+	 * again, if only one has a value the blank goes first
+	 */
+	if (!nodea->arg && nodeb->arg)
+		return -1;
+
+	if (nodea->arg && !nodeb->arg)
+		return  1;
+
+	/* both have sub-section args and they differ */
+	if (nodea->arg && nodeb->arg
+	&& (ret=strcmp(nodea->arg, nodeb->arg))!=0)
+		return ret;
+
+	return 0;
+}
+
+/* If a string starts or ends with a space it should be quoted */
+static bool
+should_escape(const char *text)
+{
+	int len;
+
+	/* no string, no escaping needed */
+	if (!text)
+		return false;
+
+	/* first character is a space */
+	if (isspace(text[0]))
+		return true;
+
+	/* last character is a space */
+	len = strlen(text);
+	if (isspace(text[len-1]))
+		return true;
+
+	return false;
+}
+
 static void
-conf_report_dump(struct dumper *node)
+conf_report_dump_text(struct dumper *head, FILE *ff)
 {
-	/* Recursive, cleanup when we're done.  */
-	if (node->next)
-		conf_report_dump(node->next);
-
-	if (node->v)
-		xlog(LOG_INFO, "%s=\t%s", node->s, node->v);
-	else if (node->s) {
-		xlog(LOG_INFO, "%s", node->s);
-		if (strlen(node->s) > 0)
-			free(node->s);
+	const struct dumper *node = head;
+	const struct dumper *last = NULL;
+
+	for (node=head; node!=NULL; node=node->next) {
+		/* starting a new section, print the section header */
+		if (dumper_section_compare(last, node)!=0) {
+			if (node != head)
+				fprintf(ff, "\n");
+			if (node->arg)
+				fprintf(ff, "[%s \"%s\"]\n", node->section, node->arg);
+			else
+				fprintf(ff, "[%s]\n", node->section);
+		}
+
+		/* now print the tag and its value */
+		fprintf(ff, " %s", node->tag);
+		if (node->value) {
+			if (should_escape(node->value))
+				fprintf(ff, " = \"%s\"", node->value);
+			else
+				fprintf(ff, " = %s", node->value);
+		}
+		fprintf(ff, "\n");
+
+		last = node;
 	}
+}
 
-	free (node);
+/* sort by tag compare function */
+static int
+dumper_compare(const void *a, const void *b)
+{
+	const struct dumper *nodea = *(struct dumper **)a;
+	const struct dumper *nodeb = *(struct dumper **)b;
+	int ret;
+
+	/* missing node, shouldnt happen */
+	if (!nodea || !nodeb)
+		return -1;
+
+	/* are the two nodes in different (sub)sections */
+	ret = dumper_section_compare(nodea, nodeb);
+	if (ret != 0)
+		return ret;
+
+	/* sub-sections match (both blank, or both same)
+	 * so we compare the tag names
+	 */
+
+	/* blank tags shouldnt happen, but paranoia */
+	if (!nodea->tag && !nodeb->tag)
+		return  0;
+
+	/* still shouldnt happen, but use the blank-goes-first logic */
+	if (!nodea->tag &&  nodeb->tag)
+		return -1;
+	if ( nodea->tag && !nodeb->tag)
+		return  1;
+
+	/* last test, compare the tags directly */
+	ret = strcmp(nodea->tag, nodeb->tag);
+	return ret;
 }
 
-void
-conf_report (void)
+/* sort all of the report nodes */
+static struct dumper *
+conf_report_sort(struct dumper *start)
 {
-	struct conf_binding *cb, *last = 0;
-	unsigned int i, len, diff_arg = 0;
-	char *current_section = (char *)0;
-	char *current_arg = (char *)0;
-	struct dumper *dumper, *dnode;
+	struct dumper **list;
+	struct dumper *node;
+	unsigned int count = 0;
+	unsigned int i=0;
+
+	/* how long is this list */
+	for (node=start; node!=NULL; node=node->next)
+		count++;
+
+	/* no need to sort a list with less than 2 items */
+	if (count < 2)
+		return start;
+
+	/* build an array of all the nodes */
+	list = calloc(count, sizeof(struct dumper *));
+	if (!list)
+		goto mem_err;
+
+	for (node=start,i=0; node!=NULL; node=node->next) {
+		list[i++] = node;
+	}
+
+	/* sort the array alphabetically by section and tag */
+	qsort(list, count, sizeof(struct dumper *), dumper_compare);
+
+	/* rebuild the linked list in sorted order */
+	for (i=0; i<count-1; i++) {
+		list[i]->next = list[i+1];
+	}
+	list[count-1]->next = NULL;
+
+	/* remember the new head of list and discard the sorting array */
+	node = list[0];
+	free(list);
 
-	dumper = dnode = (struct dumper *)calloc(1, sizeof *dumper);
-	if (!dumper)
-		goto mem_fail;
+	/* return the new head of list */
+	return node;
+
+mem_err:
+	free(list);
+	return NULL;
+}
+
+/* Output a copy of the current configuration to file */
+void
+conf_report(FILE *outfile)
+{
+	struct conf_binding *cb = NULL;
+	unsigned int i;
+	struct dumper *dumper = NULL, *dnode = NULL;
 
 	xlog(LOG_INFO, "conf_report: dumping running configuration");
 
-	for (i = 0; i < sizeof conf_bindings / sizeof conf_bindings[0]; i++)
+	/* build a linked list of all the config nodes */
+	for (i = 0; i < sizeof conf_bindings / sizeof conf_bindings[0]; i++) {
 		for (cb = LIST_FIRST(&conf_bindings[i]); cb; cb = LIST_NEXT(cb, link)) {
-			if (!cb->is_default) {
-				/* Make sure the Section arugment is the same */
-				if (current_arg && current_section && cb->arg) {
-					if (strcmp(cb->section, current_section) == 0 &&
-						strcmp(cb->arg, current_arg) != 0)
-					diff_arg = 1;
-				}
-				/* Dump this entry.  */
-				if (!current_section || strcmp(cb->section, current_section)
-							|| diff_arg) {
-					if (current_section || diff_arg) {
-						len = strlen (current_section) + 3;
-						if (current_arg)
-							len += strlen(current_arg) + 3;
-						dnode->s = malloc(len);
-						if (!dnode->s)
-							goto mem_fail;
-
-						if (current_arg)
-							snprintf(dnode->s, len, "[%s \"%s\"]",
-								current_section, current_arg);
-						else
-							snprintf(dnode->s, len, "[%s]", current_section);
-
-						dnode->next =
-							(struct dumper *)calloc(1, sizeof (struct dumper));
-						dnode = dnode->next;
-						if (!dnode)
-							goto mem_fail;
-
-						dnode->s = "";
-						dnode->next =
-							(struct dumper *)calloc(1, sizeof (struct dumper));
-						dnode = dnode->next;
-						if (!dnode)
-						goto mem_fail;
-					}
-					current_section = cb->section;
-					current_arg = cb->arg;
-					diff_arg = 0;
-				}
-				dnode->s = cb->tag;
-				dnode->v = cb->value;
-				dnode->next = (struct dumper *)calloc (1, sizeof (struct dumper));
-				dnode = dnode->next;
-				if (!dnode)
-					goto mem_fail;
-				last = cb;
+			struct dumper *newnode = calloc(1, sizeof (struct dumper));
+			if (!newnode)
+				goto mem_fail;
+
+			newnode->next = dumper;
+			dumper = newnode;
+
+			newnode->section = cb->section;
+			newnode->arg = cb->arg;
+			newnode->tag = cb->tag;
+			newnode->value = cb->value;
 		}
 	}
 
-	if (last) {
-		len = strlen(last->section) + 3;
-		if (last->arg)
-			len += strlen(last->arg) + 3;
-		dnode->s = malloc(len);
-		if (!dnode->s)
-			goto mem_fail;
-		if (last->arg)
-			snprintf(dnode->s, len, "[%s \"%s\"]", last->section, last->arg);
-		else
-			snprintf(dnode->s, len, "[%s]", last->section);
-	}
-	conf_report_dump(dumper);
-	return;
+	/* sort the list then print it */
+	dumper = conf_report_sort(dumper);
+	conf_report_dump_text(dumper, outfile);
+	goto cleanup;
 
 mem_fail:
 	xlog_warn("conf_report: malloc/calloc failed");
+cleanup:
+	/* traverse the linked list freeing all the nodes */
 	while ((dnode = dumper) != 0) {
 		dumper = dumper->next;
-		if (dnode->s)
-			free(dnode->s);
 		free(dnode);
 	}
 	return;
-- 
1.8.3.1

[PATCH v2 6/7] nfs-utils: Add nfsconftool cli

[Justin Mitchell]

This tool uses the conffile facilities to allow commandline
querying of configuration settings and to dump the current
config for diagnosis and testing

Signed-off-by: Justin Mitchell <jumitche@redhat.com>
---
 Makefile.am       | 2 +-
 configure.ac      | 1 +
 tools/Makefile.am | 2 ++
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index e1f39aa..0022084 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2,7 +2,7 @@
 
 AUTOMAKE_OPTIONS = foreign
 
-SUBDIRS = tools support utils linux-nfs tests systemd
+SUBDIRS = support tools utils linux-nfs tests systemd
 
 MAINTAINERCLEANFILES = Makefile.in
 
diff --git a/configure.ac b/configure.ac
index 5a11636..b925666 100644
--- a/configure.ac
+++ b/configure.ac
@@ -616,6 +616,7 @@ AC_CONFIG_FILES([
 	tools/rpcgen/Makefile
 	tools/mountstats/Makefile
 	tools/nfs-iostat/Makefile
+	tools/nfsconf/Makefile
 	utils/Makefile
 	utils/blkmapd/Makefile
 	utils/nfsdcltrack/Makefile
diff --git a/tools/Makefile.am b/tools/Makefile.am
index f2ce282..4266da4 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -6,6 +6,8 @@ if CONFIG_RPCGEN
 OPTDIRS += rpcgen
 endif
 
+OPTDIRS += nfsconf
+
 SUBDIRS = locktest rpcdebug nlmtest mountstats nfs-iostat $(OPTDIRS)
 
 MAINTAINERCLEANFILES = Makefile.in
-- 
1.8.3.1

[PATCH v2 7/7] nfs-utils: use nfsconftool cli to test library function

[Justin Mitchell]

Uses the nfsconftool cli to test that the configuration
library functions are operating as expected

Signed-off-by: Justin Mitchell <jumitche@redhat.com>
---
 tests/Makefile.am            |  3 ++-
 tests/nfsconf/01-errors.conf | 20 ++++++++++++++++++++
 tests/nfsconf/01-errors.exp  | 14 ++++++++++++++
 tests/nfsconf/02-valid.conf  | 25 +++++++++++++++++++++++++
 tests/nfsconf/02-valid.exp   | 26 ++++++++++++++++++++++++++
 tests/nfsconf/02-valid.sub   |  7 +++++++
 tests/t0002-nfsconf.sh       | 38 ++++++++++++++++++++++++++++++++++++++
 7 files changed, 132 insertions(+), 1 deletion(-)
 create mode 100644 tests/nfsconf/01-errors.conf
 create mode 100644 tests/nfsconf/01-errors.exp
 create mode 100644 tests/nfsconf/02-valid.conf
 create mode 100644 tests/nfsconf/02-valid.exp
 create mode 100644 tests/nfsconf/02-valid.sub
 create mode 100755 tests/t0002-nfsconf.sh

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1f96264..60691cc 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -10,5 +10,6 @@ SUBDIRS = nsm_client
 
 MAINTAINERCLEANFILES = Makefile.in
 
-TESTS = t0001-statd-basic-mon-unmon.sh
+TESTS = t0001-statd-basic-mon-unmon.sh \
+	t0002-nfsconf.sh
 EXTRA_DIST = test-lib.sh $(TESTS)
diff --git a/tests/nfsconf/01-errors.conf b/tests/nfsconf/01-errors.conf
new file mode 100644
index 0000000..ca64d2c
--- /dev/null
+++ b/tests/nfsconf/01-errors.conf
@@ -0,0 +1,20 @@
+# file of deliberate errors
+[default]
+
+
+[ one
+
+[ two " foo ]
+aa = foo
+
+[ three 
+val = none
+
+[four]
+one
+ = two
+three =
+four = foo = bar
+five = " nothing
+six = normal
+
diff --git a/tests/nfsconf/01-errors.exp b/tests/nfsconf/01-errors.exp
new file mode 100644
index 0000000..2bf1b8c
--- /dev/null
+++ b/tests/nfsconf/01-errors.exp
@@ -0,0 +1,14 @@
+nfsconf: config error at 01-errors.conf:5: non-matched ']', ignoring until next section
+nfsconf: config error at 01-errors.conf:7: non-matched '"', ignoring until next section
+nfsconf: config error at 01-errors.conf:10: non-matched ']', ignoring until next section
+nfsconf: config error at 01-errors.conf:11: ignoring line not in a section
+nfsconf: config error at 01-errors.conf:14: line not empty and not an assignment
+nfsconf: config error at 01-errors.conf:15: missing tag in assignment
+nfsconf: config error at 01-errors.conf:16: missing value in assignment
+nfsconf: config error at 01-errors.conf:18: unmatched quotes
+[four]
+ four = foo = bar
+ six = normal
+
+[two]
+ aa = foo
diff --git a/tests/nfsconf/02-valid.conf b/tests/nfsconf/02-valid.conf
new file mode 100644
index 0000000..ca8ccab
--- /dev/null
+++ b/tests/nfsconf/02-valid.conf
@@ -0,0 +1,25 @@
+[environment]
+one = 1
+two = 2
+three = 3
+
+[section_one]
+one = 11
+two = 22
+three = $three
+ four   =   "six  "
+ five six = seven eight
+
+[section_two "two"]
+one = Un
+two = Dau
+include = "02-valid.sub"
+
+[section_two "one"]
+one = Un
+two = Deux
+
+[section_two "two"]
+four = Pedwar
+   five   =    " Pump "
+
diff --git a/tests/nfsconf/02-valid.exp b/tests/nfsconf/02-valid.exp
new file mode 100644
index 0000000..379d7a4
--- /dev/null
+++ b/tests/nfsconf/02-valid.exp
@@ -0,0 +1,26 @@
+[environment]
+ one = 1
+ three = 3
+ two = 2
+
+[extra_section]
+ bar = baz
+ foo = bar
+
+[section_one]
+ five six = seven eight
+ four = "six  "
+ one = 11
+ three = $three
+ two = 22
+
+[section_two "one"]
+ one = Un
+ two = Deux
+
+[section_two "two"]
+ five = " Pump "
+ four = Pedwar
+ one = Un
+ three = Tri
+ two = Dau
diff --git a/tests/nfsconf/02-valid.sub b/tests/nfsconf/02-valid.sub
new file mode 100644
index 0000000..ab7beda
--- /dev/null
+++ b/tests/nfsconf/02-valid.sub
@@ -0,0 +1,7 @@
+# Included configs don't need a section, it is inherited
+three=Tri
+
+# But if they do, that works also
+[extra_section]
+foo = bar
+bar = baz
diff --git a/tests/t0002-nfsconf.sh b/tests/t0002-nfsconf.sh
new file mode 100755
index 0000000..511d248
--- /dev/null
+++ b/tests/t0002-nfsconf.sh
@@ -0,0 +1,38 @@
+#!/bin/bash
+
+TESTFILES=nfsconf
+DUMPER=`realpath ../tools/nfsconf/nfsconftool`
+
+BASEDIR=`dirname "$0"`
+pushd $BASEDIR/$TESTFILES
+
+rm -f *.out
+
+TCOUNT=0
+TPASS=0
+
+for i in *.conf
+do
+TNAME=`basename "$i" .conf`
+
+echo "Running test $TNAME"
+TCOUNT=$((TCOUNT + 1))
+
+if ! $DUMPER --file "$i" --dump - > "$TNAME.out" 2>&1
+then
+echo "Error running test $TNAME"
+elif ! diff -u "$TNAME.exp" "$TNAME.out"
+then
+echo "FAIL differences detected in test $TNAME"
+else
+echo "PASS $TNAME"
+TPASS=$((TPASS + 1))
+fi
+
+done
+
+echo "nfsconf tests complete. $TPASS of $TCOUNT tests passed"
+
+if [ $TPASS -lt $TCOUNT ]; then
+exit 1
+fi
-- 
1.8.3.1