nfs-utils: nfsconf cli tool and code tests
Subsystems (3)
cover nfs-utils: nfsconf cli tool and code tests
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 extension 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 implement config setting via the cli tool in order to provide an api for automation and management tools
PATCH 1/7 nfs-utils: Fix minor memory leaks
Fix some minor memory leaks, a null pointer reference and a typo.
--- support/nfs/conffile.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
support/nfs/conffile.c +6 -2
@@ -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 ;
@@ -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)
@@ -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)
Signature · 1 line
1.8.3.1
PATCH 2/7 nfs-utils: Make config includes relative to current config
In reply to Justin MitchellWhen 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.
--- support/nfs/conffile.c | 50 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 41 insertions(+), 9 deletions(-)
support/nfs/conffile.c +41 -9
@@ -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,40 @@ 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 ; + +if (!newfile)return strdup (oldfile); +if (newfile[0 ] =='/' )return strdup (newfile); + + tmpcopy =strdup (oldfile); +if (!tmpcopy) goto mem_err; + + dir =dirname (tmpcopy); +size_t 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,10 +395,12 @@ conf_parse_line(int trans, char *line, int lineno, char **section, char **subsecif (strcasecmp (line,"include" )==0 ) {/* load and parse subordinate config files */ -char * subconf =conf_readfile (val); +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 %s" , + filename, lineno, relpath); +if (relpath)free (relpath);return ; }
@@ -383,10 +414,11 @@ conf_parse_line(int trans, char *line, int lineno, char **section, char **subsec 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 (relpath)free (relpath);free (subconf); }else {/* XXX Perhaps should we not ignore errors? */
@@ -396,7 +428,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 +445,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 ; } }
@@ -508,7 +540,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, §ion, &subsection); +conf_parse (trans, conf_data, §ion, &subsection, conf_file);if (section)free (section);if (subsection)free (subsection);free (conf_data);
Signature · 1 line
1.8.3.1
Re: [PATCH 2/7] nfs-utils: Make config includes relative to current config
Hi Justin,
Sorry for taking so long to get to this...
Most of my comments is about the style you are using.
I would like to keep the coding style somewhat the same and in a few places that is not just not happening...
Quoted 31 lines · 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 <[email protected]>--- support/nfs/conffile.c | 50 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 41 insertions(+), 9 deletions(-)support/nfs/conffile.c +2
-1@@ -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;
support/nfs/conffile.c conf_set_now(const char *section, const char *arg, const char *tag, · line 216
Quoted 10 lines
support/nfs/conffile.c +7
-0@@ -206,12 +207,40 @@ 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 ;
The space between the '*' and variable is not need.
support/nfs/conffile.c conf_set_now(const char *section, const char *arg, const char *tag, · line 219
Quoted 3 lines
support/nfs/conffile.c +3
-0+ +if (!newfile)return strdup (oldfile); +if (newfile[0 ] =='/' )return strdup (newfile);
Go a head a put a newline and tab after the if statement.
support/nfs/conffile.c conf_set_now(const char *section, const char *arg, const char *tag, · line 228
Quoted 9 lines
support/nfs/conffile.c +9
-0+ + tmpcopy =strdup (oldfile); +if (!tmpcopy) goto mem_err; + + dir =dirname (tmpcopy); +size_t pathlen =strlen (dir)+strlen (newfile)+2 ; + relpath =calloc (1 , pathlen); +if (!relpath) goto mem_err; +snprintf (relpath, pathlen,"%s/%s" , dir, newfile);
Same here... That is just hard to read IMHO...
Also put all variable at the top of the routine.
Quoted 20 lines
support/nfs/conffile.c +10
-1+ +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;
support/nfs/conffile.c conf_parse_line(int trans, char *line, int lineno, char **section, char **subsec · line 399
Quoted 6 lines
support/nfs/conffile.c +2
-1@@ -366,10 +395,12 @@ conf_parse_line(int trans, char *line, int lineno, char **section, char **subsecif (strcasecmp (line,"include" )==0 ) {/* load and parse subordinate config files */ -char * subconf =conf_readfile (val); +char * relpath =relative_path (filename, val); +char * subconf =conf_readfile (relpath);
Same here no space also isn't subconf being over written?
Quoted 10 lines
support/nfs/conffile.c +3
-2if (subconf ==NULL ) { -xlog_warn ("config file error: line %d: " -"error loading included config" , lineno); +xlog_warn ("%s:%d: error loading included config %s" , + filename, lineno, relpath); +if (relpath)free (relpath);return ; }
support/nfs/conffile.c conf_parse_line(int trans, char *line, int lineno, char **section, char **subsec · line 421
Quoted 9 lines
support/nfs/conffile.c +2
-1@@ -383,10 +414,11 @@ conf_parse_line(int trans, char *line, int lineno, char **section, char **subsec 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 (relpath)free (relpath);
Are these if even needed? Won't these either be NULL or allocated memory?
Again... I'm trying to is keep the coding similar which hopefully will make the code easier to read...
Quoted 22 lines
support/nfs/conffile.c +2
-2free (subconf); }else {/* XXX Perhaps should we not ignore errors? */ @@ -396,7 +428,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 +445,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 ; } }
support/nfs/conffile.c conf_load_file(const char *conf_file) · line 545
Quoted 7 lines
support/nfs/conffile.c +1
-1@@ -508,7 +540,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, §ion, &subsection); +conf_parse (trans, conf_data, §ion, &subsection, conf_file);if (section)free (section);if (subsection)free (subsection);
Same here...
steved.
Quoted 2 lines
support/nfs/conffile.c +0
-0free (conf_data);
Re: [PATCH 2/7] nfs-utils: Make config includes relative to current config
Quoted 8 lines · Steve Dickson
Hi Justin,
Sorry for taking so long to get to this...
Most of my comments is about the style you are using.
I would like to keep the coding style somewhat the same and in a few places that is not just not happening...
Yeah sorry I should have paid more attention to maintaining style, will fix it all up and repost.
Quoted 10 lines
Quoted 10 lines
/*
* Parse the line LINE of SZ bytes. Skip Comments, recognize section
* headers and feed tag-value pairs into our configuration database. */
static voidsupport/nfs/conffile.c +1
-1-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;
Quoted 8 lines
support/nfs/conffile.c conf_parse_line(int trans, char *line, int lineno, char **section, char **subsec · line 399
Quoted 7 lines
support/nfs/conffile.c +2
-1@@ -366,10 +395,12 @@ conf_parse_line(int trans, char *line, int lineno, char **section, char **subsecif (strcasecmp (line,"include" )==0 ) {/* load and parse subordinate config files */ -char * subconf =conf_readfile (val); +char * relpath =relative_path (filename, val); +char * subconf =conf_readfile (relpath);Same here no space also isn't subconf being over written?
nope, conf_readfile() returns a malloced chunk that gets freed at the end of the include handler
Quoted 11 lines
support/nfs/conffile.c conf_parse_line(int trans, char *line, int lineno, char **section, char **subsec · line 421
Quoted 10 lines
support/nfs/conffile.c +2
-1@@ -383,10 +414,11 @@ conf_parse_line(int trans, char *line, int lineno, char **section, char **subsec 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 (relpath)free (relpath);Are these if even needed? Won't these either be NULL or allocated memory?
yes, inc_section and inc_subsection are strdup()ed copies of the current section titles because conf_parse() will alter them if any new section headers are encountered, yet we needed to know what section we were in when we entered this included conf file.
the way it is written will correctly handle recursive includes.
in general all my code changes are avoiding any use of globals and statics in the hope of eventually making the code base thread-safe and suitable for use as a public library, so that tends to mean more freeing of short lived malloced chunks.
PATCH 3/7 nfs-utils: Use config file name in error messages
In reply to Justin MitchellImprove readability of error configuration error messages by including the filename currently being read.
--- support/nfs/conffile.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-)
support/nfs/conffile.c +19 -19
@@ -276,8 +276,8 @@ 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 ; }
@@ -302,7 +302,7 @@ 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 ; }
@@ -312,14 +312,14 @@ 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 ; }
@@ -330,15 +330,15 @@ 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 ; }
@@ -355,8 +355,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' ;
@@ -365,8 +365,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' ;
@@ -383,13 +383,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 ; }
@@ -398,7 +398,7 @@ 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 %s" , +xlog_warn ("config error at %s:%d: error loading included config %s" , filename, lineno, relpath);if (relpath)free (relpath);return ;
Signature · 1 line
1.8.3.1
PATCH 4/7 nfs-utils: Indicate if config file was missing
In reply to Justin MitchellReturn an indication that the config file could not be loaded for processes that want to differentiate this from empty config
--- support/include/conffile.h | 2 +- support/nfs/conffile.c | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-)
support/include/conffile.h +1 -1
@@ -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 *);
support/nfs/conffile.c +5 -5
@@ -522,7 +522,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;
@@ -532,7 +532,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 ();
@@ -550,10 +550,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;
@@ -564,7 +564,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); }/*
Signature · 1 line
1.8.3.1
PATCH 5/7 nfs-utils: tidy up output of conf_report
In reply to Justin MitchellPreviously unused conf_report() function is modified to output an alphabetically sorted version of the current configuration to the nominated FILE handle.
--- support/include/conffile.h | 3 +- support/nfs/conffile.c | 237 ++++++++++++++++++++++++++++----------------- 2 files changed, 151 insertions(+), 89 deletions(-)
support/include/conffile.h +2 -1
@@ -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
support/nfs/conffile.c +149 -88
266 lines not on this page · raw message
Signature · 1 line
1.8.3.1
Re: [PATCH 5/7] nfs-utils: tidy up output of conf_report
More styling comments...
Quoted 59 lines · 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 <[email protected]>--- support/include/conffile.h | 3 +- support/nfs/conffile.c | 237 ++++++++++++++++++++++++++++----------------- 2 files changed, 151 insertions(+), 89 deletions(-)support/include/conffile.h +2
-1@@ -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 support/nfs/conffile.c +16
-1@@ -1055,117 +1055,178 @@ 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; }; +static int dumper_section_compare (const struct dumper *nodea,const struct dumper *nodeb) +{ +int ret; + +// missing node, shouldnt happen +if (!nodea || !nodeb)return -1 ; +// 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 ; +// no section names at all +if (!nodea->section && !nodeb->section)return 0 ; +// two section names, do they match
support/nfs/conffile.c conf_end(int transaction, int commit) · line 1086
Quoted 10 lines
support/nfs/conffile.c +10
-0+ 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 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;
Please use normal comment braces and this code is just very hard to read do to one-lining your if statements and spacing.
support/nfs/conffile.c conf_end(int transaction, int commit) · line 1099
Quoted 13 lines
support/nfs/conffile.c +13
-0+ +return 0 ; +} + +static bool +should_escape (const char * text) +{ +int len; +if (!text)return false; +if (isspace (text[0 ]))return true; + len =strlen (text); +if (isspace (text[len-1 ]))return true; +return false;
Again... hard to read... New lines are a good thing! :-)
Quoted 92 lines
support/nfs/conffile.c +62
-22+} +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) { +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); + } +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); +/* compare two dumper nodes for sorting */ +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 ; + + 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 -1 ; +if ( nodea->tag && !nodeb->tag)return 1 ; +if (!nodea->tag && !nodeb->tag)return 0 ; + +// last test, compare the tags directly + ret =strcmp (nodea->tag, nodeb->tag); +return ret; } -void -conf_report (void ) +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 ; - dumper = dnode = (struct dumper *)calloc (1 ,sizeof *dumper); -if (!dumper) - goto mem_fail; +// how long is this list +for (node=start; node!=NULL ; node=node->next) count++; + +// no need to sort a list of 1 or less +if (count <2 )return start; + +// make an array of all the node + list =calloc (count,sizeof (struct dumper *)); +if (!list) goto mem_err;
support/nfs/conffile.c conf_end(int transaction, int commit) · line 1179
Quoted 10 lines
support/nfs/conffile.c +10
-0+ +unsigned int i=0 ; +for (node=start; node!=NULL ; node=node->next) { + list[i++] = node; + } + +// sort the array +qsort (list, count,sizeof (struct dumper *), dumper_compare); + +// turn sorted array back into a linked list
Normal comments braces... please..
steved.
Quoted 119 lines
support/nfs/conffile.c +38
-65+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 cleanup + node = list[0 ]; +free (list); + +// return the new head of list +return node; + +mem_err: +free (list); +return NULL ; +} + +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++) +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 ; + dumper =conf_report_sort (dumper); +conf_report_dump_text (dumper, outfile); + goto cleanup; mem_fail:xlog_warn ("conf_report: malloc/calloc failed" ); +cleanup:while ((dnode = dumper) !=0 ) { dumper = dumper->next; -if (dnode->s) -free (dnode->s);free (dnode); }return ;
PATCH 6/7 nfs-utils: Add nfsconftool cli
In reply to Justin MitchellThis tool uses the conffile facilities to allow commandline querying of configuration settings and to dump the current config for diagnosis and testing
--- Makefile.am | 2 +- configure.ac | 1 + tools/Makefile.am | 2 + tools/nfsconf/Makefile.am | 9 +++ tools/nfsconf/nfsconfcli.c | 144 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 tools/nfsconf/Makefile.am create mode 100644 tools/nfsconf/nfsconfcli.c
Makefile.am +1 -1
@@ -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
configure.ac +1 -0
@@ -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
tools/Makefile.am +2 -0
@@ -6,6 +6,8 @@ if CONFIG_RPCGEN OPTDIRS += rpcgen endif +OPTDIRS += nfsconf + SUBDIRS = locktest rpcdebug nlmtest mountstats nfs-iostat $(OPTDIRS) MAINTAINERCLEANFILES = Makefile.in
tools/nfsconf/Makefile.am new file +9 -0
@@ -0,0 +1,9 @@ +## Process this file with automake to produce Makefile.in + +bin_PROGRAMS = nfsconftool + +nfsconftool_SOURCES = nfsconfcli.c +nfsconftool_LDADD = ../../support/nfs/libnfsconf.la + +MAINTAINERCLEANFILES = Makefile.in +
tools/nfsconf/nfsconfcli.c new file +144 -0
144 lines not on this page · raw message
Signature · 1 line
1.8.3.1
Re: [PATCH 6/7] nfs-utils: Add nfsconftool cli
Quoted 155 lines · 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 <[email protected]>--- Makefile.am | 2 +- configure.ac | 1 + tools/Makefile.am | 2 + tools/nfsconf/Makefile.am | 9 +++ tools/nfsconf/nfsconfcli.c | 144 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 tools/nfsconf/Makefile.am create mode 100644 tools/nfsconf/nfsconfcli.cMakefile.am +1
-1@@ -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.inconfigure.ac +1
-0@@ -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/Makefiletools/Makefile.am +2
-0@@ -6,6 +6,8 @@ if CONFIG_RPCGEN OPTDIRS += rpcgen endif +OPTDIRS += nfsconf + SUBDIRS = locktest rpcdebug nlmtest mountstats nfs-iostat $(OPTDIRS) MAINTAINERCLEANFILES = Makefile.intools/nfsconf/Makefile.am new file +9
-0@@ -0,0 +1,9 @@ +## Process this file with automake to produce Makefile.in + +bin_PROGRAMS = nfsconftool + +nfsconftool_SOURCES = nfsconfcli.c +nfsconftool_LDADD = ../../support/nfs/libnfsconf.la + +MAINTAINERCLEANFILES = Makefile.in +tools/nfsconf/nfsconfcli.c new file +81
-0@@ -0,0 +1,144 @@ +#include <stdlib.h> +#include <unistd.h> +#include <stdio.h> +#include <getopt.h> +#include <string.h> +#include <errno.h> +#include "config.h" +#include "conffile.h" +#include "xlog.h" + +typedef enum { +MODE_NONE , +MODE_GET , +MODE_ISSET , +MODE_DUMP +}confmode_t ; + +static void usage (const char *name) +{ +fprintf (stderr,"Usage: %s [-v] [--file filename.conf] ...\n" , name); +fprintf (stderr,"Options:\n" ); +fprintf (stderr," -v Increase Verbosity\n" ); +fprintf (stderr," --file filename.conf Load this config file\n" ); +fprintf (stderr," (Default config file: " NFS_CONFFILE "\n" ); +fprintf (stderr,"Modes:\n" ); +fprintf (stderr," --dump [outputfile]\n" ); +fprintf (stderr," Outputs the configuration to the named file\n" ); +fprintf (stderr," --get [--arg subsection] {section} {tag}\n" ); +fprintf (stderr," Output one specific config value\n" ); +fprintf (stderr," --isset [--arg subsection] {section} {tag}\n" ); +fprintf (stderr," Return code indicates if config value is present\n" ); +} + +int main (int argc,char **argv) +{ +const char * val; +char * confpath =NFS_CONFFILE ; +char * arg =NULL ; +int verbose=0 ; +int ret =0 ; +char * dumpfile =NULL ; + +confmode_t mode =MODE_NONE ; + +while (1 ) { +int c; +int index =0 ; +struct option long_options[] = { + {"get" , no_argument,0 ,'g' }, + {"arg" , required_argument,0 ,'a' }, + {"isset" , no_argument,0 ,'i' }, + {"dump" , optional_argument,0 ,'d' }, + {"file" , required_argument,0 ,'f' }, + {"verbose" , no_argument,0 ,'v' }, + {NULL ,0 ,0 ,0 } + }; + + c =getopt_long (argc, argv,"ga:id::f:v" , long_options, &index); +if (c ==-1 )break ; + +switch (c) { +case 0 : +break ; +case 'f' : + confpath = optarg; +break ; +case 'a' : + arg = optarg; +break ; +case 'v' : + verbose++; +break ; +case 'g' : + mode =MODE_GET ; +break ; +case 'i' : + mode =MODE_ISSET ; +break ; +case 'd' : +if (optarg==NULL && argv[optind]!=NULL && argv[optind][0 ]!='-' ) + optarg = argv[optind++];
tools/nfsconf/nfsconfcli.c line 91
Quoted 10 lines
tools/nfsconf/nfsconfcli.c new file +10
-0+ mode =MODE_DUMP ; + dumpfile = optarg; +break ; +default : +usage (argv[0 ]); +return 1 ; + } + } + +if (verbose)xlog_config (D_ALL ,1 );
New line please..
Quoted 7 lines
tools/nfsconf/nfsconfcli.c new file +7
-0+xlog_stderr (1 ); +xlog_syslog (0 ); +xlog_open ("nfsconf" ); + +if (mode ==MODE_NONE ) { +fprintf (stderr,"Error: No MODE selected.\n" ); +usage (argv[0 ]);
tools/nfsconf/nfsconfcli.c line 108
Quoted 10 lines
tools/nfsconf/nfsconfcli.c new file +10
-0+return 1 ; + } + +if (conf_init_file (confpath)) { +if (verbose || mode !=MODE_ISSET )fprintf (stderr,"Error loading config file %s\n" , confpath); +if (mode !=MODE_ISSET )return 1 ; + } + +if (mode ==MODE_DUMP ) { +FILE *out = stdout;
Declarations up to[ please... and I don't understand why this assignment is even needed
tools/nfsconf/nfsconfcli.c line 118
Quoted 10 lines
tools/nfsconf/nfsconfcli.c new file +10
-0+if (dumpfile) { +if ((out=fopen (dumpfile,"w" ))==NULL ) { +fprintf (stderr,"Error opening dumpfile %s: %s\n" , dumpfile,strerror (errno)); + ret =2 ; + goto cleanup; + } +if (verbose)printf ("Dumping config to %s\n" , dumpfile); + } +conf_report (out); +if (dumpfile)fclose (out);
closing stdout??
tools/nfsconf/nfsconfcli.c line 128
Quoted 10 lines
tools/nfsconf/nfsconfcli.c new file +10
-0+ }else +if (mode ==MODE_GET || mode ==MODE_ISSET ) { +if (optind+1 >= argc) { +fprintf (stderr,"Error: insufficient arguments for mode\n" ); +usage (argv[0 ]); + ret =2 ; + goto cleanup; + } +char * section = argv[optind++]; +char * tag = argv[optind++];
Up top please...
tools/nfsconf/nfsconfcli.c line 135
Quoted 7 lines
tools/nfsconf/nfsconfcli.c new file +7
-0+ +if ((val=conf_get_section (section, arg, tag))!=NULL ) { +if (mode ==MODE_GET )printf ("%s\n" , val); + }else { +if (mode ==MODE_GET && verbose)fprintf (stderr,"Tag '%s' not found\n" , tag); + ret =1 ; + }
There is a lot going on here... having a few newlines and spacing would help a lot.
steved.
Quoted 10 lines
tools/nfsconf/nfsconfcli.c new file +9
-0+ }else { +fprintf (stderr,"Mode not yet implimented.\n" ); + ret =2 ; + } + +cleanup: +conf_cleanup (); +return ret; +}
PATCH 7/7 nfs-utils: use nfsconftool cli to test library function
In reply to Justin MitchellUses the nfsconftool cli to test that the configuration library functions are operating as expected
--- 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
tests/Makefile.am +2 -1
@@ -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)
tests/nfsconf/01-errors.conf new file +20 -0
@@ -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 +
tests/nfsconf/01-errors.exp new file +14 -0
@@ -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
tests/nfsconf/02-valid.conf new file +25 -0
@@ -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 " +
tests/nfsconf/02-valid.exp new file +26 -0
@@ -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
tests/nfsconf/02-valid.sub new file +7 -0
@@ -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
tests/t0002-nfsconf.sh new file +38 -0
@@ -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
Signature · 1 line
1.8.3.1