[pacman-dev] [PATCH 0/9] extract ini parser from parseconfig

newer
[pacman-dev] [PATCH] doc/repo-add:...

Andrew Gregory

22 Jul 2013 22 Jul '13

6:46 a.m.

This creates a generic ini parser than can be reused for hooks or other configuration files. I took a very different approach from the previous patchset by utilizing a callback that processes the options as the file is parsed. This should have a much lower memory footprint and is closer to how we were already parsing config files. Parsing differences: * Include directives can be outside a section. They can include their own sections so it made little sense to require that they be in one already. * Errors in included files are now fatal. Andrew Gregory (9): conf.c: add parse_options to section_t conf.c: move repo parsing out of _parseconfig conf.c: move directive parsing out of _parseconfig conf.c: move section handling out of _parseconfig conf.c: pass _parse_directive as a callback conf.c: extract ini parsing code to separate files ini.c: move recursion limit to a macro ini.c: reuse line buffer ini.c: make errors in includes fatal src/pacman/Makefile.am | 1 + src/pacman/conf.c | 257 ++++++++++++++----------------------------------- src/pacman/ini.c | 217 +++++++++++++++++++++++++++++++++++++++++ src/pacman/ini.h | 30 ++++++ 4 files changed, 319 insertions(+), 186 deletions(-) create mode 100644 src/pacman/ini.c create mode 100644 src/pacman/ini.h -- 1.8.3.3

Show replies by date

Andrew Gregory

22 Jul 22 Jul

6:46 a.m.

New subject: [pacman-dev] [PATCH 1/9] conf.c: add parse_options to section_t

This consolidates all of our state information into a single variable. Signed-off-by: Andrew Gregory --- src/pacman/conf.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/pacman/conf.c b/src/pacman/conf.c index 2985aba..2f21864 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -746,6 +746,7 @@ struct section_t { /* useful for all sections */ char *name; int is_options; + int parse_options; /* db section option gathering */ alpm_siglevel_t siglevel; alpm_list_t *servers; @@ -760,7 +761,7 @@ struct section_t { * @param parse_options whether we are parsing options or repo data * @return 0 on success, 1 on failure */ -static int finish_section(struct section_t *section, int parse_options) +static int finish_section(struct section_t *section) { int ret = 0; alpm_list_t *i; @@ -769,7 +770,7 @@ static int finish_section(struct section_t *section, int parse_options) pm_printf(ALPM_LOG_DEBUG, "config: finish section '%s'\n", section->name); /* parsing options (or nothing)- nothing to do except free the pieces */ - if(!section->name || parse_options || section->is_options) { + if(!section->name || section->parse_options || section->is_options) { goto cleanup; } @@ -809,13 +810,10 @@ cleanup: * within ourself on an include. * @param file path to the config file * @param section the current active section - * @param parse_options whether to parse and call methods for the options - * section; if 0, parse and call methods for the repos sections * @param depth the current recursion depth * @return 0 on success, 1 on failure */ -static int _parseconfig(const char *file, struct section_t *section, - int parse_options, int depth) +static int _parseconfig(const char *file, struct section_t *section, int depth) { FILE *fp = NULL; char line[PATH_MAX]; @@ -869,7 +867,7 @@ static int _parseconfig(const char *file, struct section_t *section, name = strdup(line + 1); name[line_len - 2] = '\0'; /* we're at a new section; perform any post-actions for the prior */ - if(finish_section(section, parse_options)) { + if(finish_section(section)) { ret = 1; goto cleanup; } @@ -934,19 +932,19 @@ static int _parseconfig(const char *file, struct section_t *section, for(gindex = 0; gindex < globbuf.gl_pathc; gindex++) { pm_printf(ALPM_LOG_DEBUG, "config file %s, line %d: including %s\n", file, linenum, globbuf.gl_pathv[gindex]); - _parseconfig(globbuf.gl_pathv[gindex], section, parse_options, depth + 1); + _parseconfig(globbuf.gl_pathv[gindex], section, depth + 1); } break; } globfree(&globbuf); continue; } - if(parse_options && section->is_options) { + if(section->parse_options && section->is_options) { /* we are either in options ... */ if((ret = _parse_options(key, value, file, linenum)) != 0) { goto cleanup; } - } else if(!parse_options && !section->is_options) { + } else if(!section->parse_options && !section->is_options) { /* ... or in a repo section */ if(strcmp(key, "Server") == 0) { if(value == NULL) { @@ -979,7 +977,7 @@ static int _parseconfig(const char *file, struct section_t *section, } if(depth == 0) { - ret = finish_section(section, parse_options); + ret = finish_section(section); } cleanup: @@ -1006,7 +1004,8 @@ int parseconfig(const char *file) /* call the real parseconfig function with a null section & db argument */ pm_printf(ALPM_LOG_DEBUG, "parseconfig: options pass\n"); - if((ret = _parseconfig(file, §ion, 1, 0))) { + section.parse_options = 1; + if((ret = _parseconfig(file, §ion, 0))) { return ret; } if((ret = setup_libalpm())) { @@ -1014,7 +1013,8 @@ int parseconfig(const char *file) } /* second pass, repo section parsing */ pm_printf(ALPM_LOG_DEBUG, "parseconfig: repo pass\n"); - return _parseconfig(file, §ion, 0, 0); + section.parse_options = 0; + return _parseconfig(file, §ion, 0); } /* vim: set ts=2 sw=2 noet: */ -- 1.8.3.3

Andrew Gregory

6:46 a.m.

New subject: [pacman-dev] [PATCH 2/9] conf.c: move repo parsing out of _parseconfig

Signed-off-by: Andrew Gregory --- src/pacman/conf.c | 65 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 39 insertions(+), 26 deletions(-) diff --git a/src/pacman/conf.c b/src/pacman/conf.c index 2f21864..765f369 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -752,6 +752,43 @@ struct section_t { alpm_list_t *servers; }; +static int _parse_repo(const char *key, char *value, const char *file, + int line, struct section_t *section) +{ + int ret = 0; + + if(strcmp(key, "Server") == 0) { + if(!value) { + pm_printf(ALPM_LOG_ERROR, _("config file %s, line %d: directive '%s' needs a value\n"), + file, line, key); + ret = 1; + } else { + section->servers = alpm_list_add(section->servers, strdup(value)); + } + } else if(strcmp(key, "SigLevel") == 0) { + if(!value) { + pm_printf(ALPM_LOG_ERROR, _("config file %s, line %d: directive '%s' needs a value\n"), + file, line, key); + } else { + alpm_list_t *values = NULL; + setrepeatingoption(value, "SigLevel", &values); + if(values) { + if(section->siglevel == ALPM_SIG_USE_DEFAULT) { + section->siglevel = config->siglevel; + } + ret = process_siglevel(values, §ion->siglevel, file, line); + FREELIST(values); + } + } + } else { + pm_printf(ALPM_LOG_WARNING, + _("config file %s, line %d: directive '%s' in section '%s' not recognized.\n"), + file, line, key, section->name); + } + + return ret; +} + /** * Wrap up a section once we have reached the end of it. This should be called * when a subsequent section is encountered, or when we have reached the end of @@ -946,32 +983,8 @@ static int _parseconfig(const char *file, struct section_t *section, int depth) } } else if(!section->parse_options && !section->is_options) { /* ... or in a repo section */ - if(strcmp(key, "Server") == 0) { - if(value == NULL) { - pm_printf(ALPM_LOG_ERROR, _("config file %s, line %d: directive '%s' needs a value\n"), - file, linenum, key); - ret = 1; - goto cleanup; - } - section->servers = alpm_list_add(section->servers, strdup(value)); - } else if(strcmp(key, "SigLevel") == 0) { - alpm_list_t *values = NULL; - setrepeatingoption(value, "SigLevel", &values); - if(values) { - if(section->siglevel == ALPM_SIG_USE_DEFAULT) { - section->siglevel = config->siglevel; - } - if(process_siglevel(values, §ion->siglevel, file, linenum)) { - FREELIST(values); - ret = 1; - goto cleanup; - } - FREELIST(values); - } - } else { - pm_printf(ALPM_LOG_WARNING, - _("config file %s, line %d: directive '%s' in section '%s' not recognized.\n"), - file, linenum, key, section->name); + if((ret = _parse_repo(key, value, file, linenum, section)) != 0) { + goto cleanup; } } } -- 1.8.3.3

Andrew Gregory

6:46 a.m.

New subject: [pacman-dev] [PATCH 3/9] conf.c: move directive parsing out of _parseconfig

Include directives no longer have to be within a section. Signed-off-by: Andrew Gregory --- src/pacman/conf.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/src/pacman/conf.c b/src/pacman/conf.c index 765f369..caee55f 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -841,6 +841,25 @@ cleanup: return ret; } +static int _parse_directive(const char *file, int linenum, + char *key, char *value, struct section_t *section) +{ + if(section->name == NULL) { + pm_printf(ALPM_LOG_ERROR, _("config file %s, line %d: All directives must belong to a section.\n"), + file, linenum); + return 1; + } + + if(section->parse_options && section->is_options) { + /* we are either in options ... */ + return _parse_options(key, value, file, linenum); + } else if(!section->parse_options && !section->is_options) { + /* ... or in a repo section */ + return _parse_repo(key, value, file, linenum, section); + } + return 0; +} + /** The "real" parseconfig. Each "Include" directive will recall this method so * recursion and stack depth are limited to 10 levels. The publicly visible * parseconfig calls this with a NULL section argument so we can recall from @@ -928,13 +947,6 @@ static int _parseconfig(const char *file, struct section_t *section, int depth) ret = 1; goto cleanup; } - /* For each directive, compare to the camelcase string. */ - if(section->name == NULL) { - pm_printf(ALPM_LOG_ERROR, _("config file %s, line %d: All directives must belong to a section.\n"), - file, linenum); - ret = 1; - goto cleanup; - } /* Include is allowed in both options and repo sections */ if(strcmp(key, "Include") == 0) { glob_t globbuf; @@ -976,16 +988,8 @@ static int _parseconfig(const char *file, struct section_t *section, int depth) globfree(&globbuf); continue; } - if(section->parse_options && section->is_options) { - /* we are either in options ... */ - if((ret = _parse_options(key, value, file, linenum)) != 0) { - goto cleanup; - } - } else if(!section->parse_options && !section->is_options) { - /* ... or in a repo section */ - if((ret = _parse_repo(key, value, file, linenum, section)) != 0) { - goto cleanup; - } + if((ret = _parse_directive(file, linenum, key, value, section)) != 0) { + goto cleanup; } } -- 1.8.3.3

Andrew Gregory

6:46 a.m.

New subject: [pacman-dev] [PATCH 4/9] conf.c: move section handling out of _parseconfig

_parseconfig now tracks the current section name directly so that the name stored in the section struct is just a pointer to the one stored by _parseconfig. Signed-off-by: Andrew Gregory --- src/pacman/conf.c | 49 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/src/pacman/conf.c b/src/pacman/conf.c index caee55f..ab8706c 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -744,7 +744,7 @@ static int setup_libalpm(void) */ struct section_t { /* useful for all sections */ - char *name; + const char *name; int is_options; int parse_options; /* db section option gathering */ @@ -836,14 +836,25 @@ cleanup: alpm_list_free(section->servers); section->servers = NULL; section->siglevel = ALPM_SIG_USE_DEFAULT; - free(section->name); section->name = NULL; return ret; } -static int _parse_directive(const char *file, int linenum, +static int _parse_directive(const char *file, int linenum, const char *name, char *key, char *value, struct section_t *section) { + if(!key && !value) { + int ret = finish_section(section); + pm_printf(ALPM_LOG_DEBUG, "config: new section '%s'\n", name); + section->name = name; + if(name && strcmp(name, "options") == 0) { + section->is_options = 1; + } else { + section->is_options = 0; + } + return ret; + } + if(section->name == NULL) { pm_printf(ALPM_LOG_ERROR, _("config file %s, line %d: All directives must belong to a section.\n"), file, linenum); @@ -857,6 +868,7 @@ static int _parse_directive(const char *file, int linenum, /* ... or in a repo section */ return _parse_repo(key, value, file, linenum, section); } + return 0; } @@ -869,7 +881,8 @@ static int _parse_directive(const char *file, int linenum, * @param depth the current recursion depth * @return 0 on success, 1 on failure */ -static int _parseconfig(const char *file, struct section_t *section, int depth) +static int _parseconfig(const char *file, struct section_t *section, + char **section_name, int depth) { FILE *fp = NULL; char line[PATH_MAX]; @@ -922,14 +935,14 @@ static int _parseconfig(const char *file, struct section_t *section, int depth) /* new config section, skip the '[' */ name = strdup(line + 1); name[line_len - 2] = '\0'; - /* we're at a new section; perform any post-actions for the prior */ - if(finish_section(section)) { - ret = 1; + + ret = _parse_directive(file, linenum, name, NULL, NULL, section); + free(*section_name); + *section_name = name; + + if(ret) { goto cleanup; } - pm_printf(ALPM_LOG_DEBUG, "config: new section '%s'\n", name); - section->name = name; - section->is_options = (strcmp(name, "options") == 0); continue; } @@ -981,26 +994,31 @@ static int _parseconfig(const char *file, struct section_t *section, int depth) for(gindex = 0; gindex < globbuf.gl_pathc; gindex++) { pm_printf(ALPM_LOG_DEBUG, "config file %s, line %d: including %s\n", file, linenum, globbuf.gl_pathv[gindex]); - _parseconfig(globbuf.gl_pathv[gindex], section, depth + 1); + _parseconfig(globbuf.gl_pathv[gindex], section, + section_name, depth + 1); } break; } globfree(&globbuf); continue; } - if((ret = _parse_directive(file, linenum, key, value, section)) != 0) { + if((ret = _parse_directive(file, linenum, *section_name, key, value, section)) != 0) { goto cleanup; } } if(depth == 0) { - ret = finish_section(section); + ret = _parse_directive(NULL, 0, NULL, NULL, NULL, section); } cleanup: if(fp) { fclose(fp); } + if(depth == 0) { + free(*section_name); + *section_name = NULL; + } pm_printf(ALPM_LOG_DEBUG, "config: finished parsing %s\n", file); return ret; } @@ -1013,6 +1031,7 @@ int parseconfig(const char *file) { int ret; struct section_t section; + char *section_name = NULL; memset(§ion, 0, sizeof(struct section_t)); section.siglevel = ALPM_SIG_USE_DEFAULT; /* the config parse is a two-pass affair. We first parse the entire thing for @@ -1022,7 +1041,7 @@ int parseconfig(const char *file) /* call the real parseconfig function with a null section & db argument */ pm_printf(ALPM_LOG_DEBUG, "parseconfig: options pass\n"); section.parse_options = 1; - if((ret = _parseconfig(file, §ion, 0))) { + if((ret = _parseconfig(file, §ion, §ion_name, 0))) { return ret; } if((ret = setup_libalpm())) { @@ -1031,7 +1050,7 @@ int parseconfig(const char *file) /* second pass, repo section parsing */ pm_printf(ALPM_LOG_DEBUG, "parseconfig: repo pass\n"); section.parse_options = 0; - return _parseconfig(file, §ion, 0); + return _parseconfig(file, §ion, §ion_name, 0); } /* vim: set ts=2 sw=2 noet: */ -- 1.8.3.3

Andrew Gregory

6:46 a.m.

New subject: [pacman-dev] [PATCH 5/9] conf.c: pass _parse_directive as a callback

This will allow passing arbitrary key/value handlers. Signed-off-by: Andrew Gregory --- src/pacman/conf.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/pacman/conf.c b/src/pacman/conf.c index ab8706c..704ce13 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -841,8 +841,9 @@ cleanup: } static int _parse_directive(const char *file, int linenum, const char *name, - char *key, char *value, struct section_t *section) + char *key, char *value, void *data) { + struct section_t *section = data; if(!key && !value) { int ret = finish_section(section); pm_printf(ALPM_LOG_DEBUG, "config: new section '%s'\n", name); @@ -872,6 +873,9 @@ static int _parse_directive(const char *file, int linenum, const char *name, return 0; } +typedef int (ini_parser_fn)(const char *file, int line, const char *section, + char *key, char *value, void *data); + /** The "real" parseconfig. Each "Include" directive will recall this method so * recursion and stack depth are limited to 10 levels. The publicly visible * parseconfig calls this with a NULL section argument so we can recall from @@ -881,7 +885,7 @@ static int _parse_directive(const char *file, int linenum, const char *name, * @param depth the current recursion depth * @return 0 on success, 1 on failure */ -static int _parseconfig(const char *file, struct section_t *section, +static int _parseconfig(const char *file, ini_parser_fn cb, void *data, char **section_name, int depth) { FILE *fp = NULL; @@ -936,7 +940,7 @@ static int _parseconfig(const char *file, struct section_t *section, name = strdup(line + 1); name[line_len - 2] = '\0'; - ret = _parse_directive(file, linenum, name, NULL, NULL, section); + ret = cb(file, linenum, name, NULL, NULL, data); free(*section_name); *section_name = name; @@ -994,7 +998,7 @@ static int _parseconfig(const char *file, struct section_t *section, for(gindex = 0; gindex < globbuf.gl_pathc; gindex++) { pm_printf(ALPM_LOG_DEBUG, "config file %s, line %d: including %s\n", file, linenum, globbuf.gl_pathv[gindex]); - _parseconfig(globbuf.gl_pathv[gindex], section, + _parseconfig(globbuf.gl_pathv[gindex], cb, data, section_name, depth + 1); } break; @@ -1002,13 +1006,13 @@ static int _parseconfig(const char *file, struct section_t *section, globfree(&globbuf); continue; } - if((ret = _parse_directive(file, linenum, *section_name, key, value, section)) != 0) { + if((ret = cb(file, linenum, *section_name, key, value, data)) != 0) { goto cleanup; } } if(depth == 0) { - ret = _parse_directive(NULL, 0, NULL, NULL, NULL, section); + ret = cb(NULL, 0, NULL, NULL, NULL, data); } cleanup: @@ -1041,7 +1045,7 @@ int parseconfig(const char *file) /* call the real parseconfig function with a null section & db argument */ pm_printf(ALPM_LOG_DEBUG, "parseconfig: options pass\n"); section.parse_options = 1; - if((ret = _parseconfig(file, §ion, §ion_name, 0))) { + if((ret = _parseconfig(file, _parse_directive, §ion, §ion_name, 0))) { return ret; } if((ret = setup_libalpm())) { @@ -1050,7 +1054,7 @@ int parseconfig(const char *file) /* second pass, repo section parsing */ pm_printf(ALPM_LOG_DEBUG, "parseconfig: repo pass\n"); section.parse_options = 0; - return _parseconfig(file, §ion, §ion_name, 0); + return _parseconfig(file, _parse_directive, §ion, §ion_name, 0); } /* vim: set ts=2 sw=2 noet: */ -- 1.8.3.3

Allan McRae

29 Jul 29 Jul

4:18 a.m.

New subject: [pacman-dev] [PATCH 5/9] conf.c: pass _parse_directive as a callback

On 22/07/13 16:46, Andrew Gregory wrote:

...

This will allow passing arbitrary key/value handlers.

I have now gone through this entire patchset and this is the only patch that confuses me. What do we gain by doing this? Are you envisioning having _parse_directive and _parse_hook or something? Will that be needed? Allan

...

Signed-off-by: Andrew Gregory --- src/pacman/conf.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/src/pacman/conf.c b/src/pacman/conf.c index ab8706c..704ce13 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -841,8 +841,9 @@ cleanup: }

static int _parse_directive(const char *file, int linenum, const char *name, - char *key, char *value, struct section_t *section) + char *key, char *value, void *data) { + struct section_t *section = data; if(!key && !value) { int ret = finish_section(section); pm_printf(ALPM_LOG_DEBUG, "config: new section '%s'\n", name); @@ -872,6 +873,9 @@ static int _parse_directive(const char *file, int linenum, const char *name, return 0; }

+typedef int (ini_parser_fn)(const char *file, int line, const char *section, + char *key, char *value, void *data); + /** The "real" parseconfig. Each "Include" directive will recall this method so * recursion and stack depth are limited to 10 levels. The publicly visible * parseconfig calls this with a NULL section argument so we can recall from @@ -881,7 +885,7 @@ static int _parse_directive(const char *file, int linenum, const char *name, * @param depth the current recursion depth * @return 0 on success, 1 on failure */ -static int _parseconfig(const char *file, struct section_t *section, +static int _parseconfig(const char *file, ini_parser_fn cb, void *data, char **section_name, int depth) { FILE *fp = NULL; @@ -936,7 +940,7 @@ static int _parseconfig(const char *file, struct section_t *section, name = strdup(line + 1); name[line_len - 2] = '\0';

- ret = _parse_directive(file, linenum, name, NULL, NULL, section); + ret = cb(file, linenum, name, NULL, NULL, data); free(*section_name); *section_name = name;

@@ -994,7 +998,7 @@ static int _parseconfig(const char *file, struct section_t *section, for(gindex = 0; gindex < globbuf.gl_pathc; gindex++) { pm_printf(ALPM_LOG_DEBUG, "config file %s, line %d: including %s\n", file, linenum, globbuf.gl_pathv[gindex]); - _parseconfig(globbuf.gl_pathv[gindex], section, + _parseconfig(globbuf.gl_pathv[gindex], cb, data, section_name, depth + 1); } break; @@ -1002,13 +1006,13 @@ static int _parseconfig(const char *file, struct section_t *section, globfree(&globbuf); continue; } - if((ret = _parse_directive(file, linenum, *section_name, key, value, section)) != 0) { + if((ret = cb(file, linenum, *section_name, key, value, data)) != 0) { goto cleanup; } }

if(depth == 0) { - ret = _parse_directive(NULL, 0, NULL, NULL, NULL, section); + ret = cb(NULL, 0, NULL, NULL, NULL, data); }

cleanup: @@ -1041,7 +1045,7 @@ int parseconfig(const char *file) /* call the real parseconfig function with a null section & db argument */ pm_printf(ALPM_LOG_DEBUG, "parseconfig: options pass\n"); section.parse_options = 1; - if((ret = _parseconfig(file, §ion, §ion_name, 0))) { + if((ret = _parseconfig(file, _parse_directive, §ion, §ion_name, 0))) { return ret; } if((ret = setup_libalpm())) { @@ -1050,7 +1054,7 @@ int parseconfig(const char *file) /* second pass, repo section parsing */ pm_printf(ALPM_LOG_DEBUG, "parseconfig: repo pass\n"); section.parse_options = 0; - return _parseconfig(file, §ion, §ion_name, 0); + return _parseconfig(file, _parse_directive, §ion, §ion_name, 0); }

/* vim: set ts=2 sw=2 noet: */

Andrew Gregory

1:19 p.m.

New subject: [pacman-dev] [PATCH 5/9] conf.c: pass _parse_directive as a callback

On 07/29/13 at 02:18pm, Allan McRae wrote:

...

On 22/07/13 16:46, Andrew Gregory wrote:

...
This will allow passing arbitrary key/value handlers.

I have now gone through this entire patchset and this is the only patch that confuses me. What do we gain by doing this? Are you envisioning having _parse_directive and _parse_hook or something? Will that be needed?

Allan

Yes, _parse_hook is exactly what I was envisioning. This patch completely decouples the ini parser from the handling of the key/value pairs, making it easy to reuse for any file that uses our format simply by passing a different key/value handler. Aside from hooks, I also hope to eventually make the parser available to other pacman-related projects so that they can all easily use the same config format. apg

...

...
Signed-off-by: Andrew Gregory --- src/pacman/conf.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/src/pacman/conf.c b/src/pacman/conf.c index ab8706c..704ce13 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -841,8 +841,9 @@ cleanup: }

static int _parse_directive(const char *file, int linenum, const char *name, - char *key, char *value, struct section_t *section) + char *key, char *value, void *data) { + struct section_t *section = data; if(!key && !value) { int ret = finish_section(section); pm_printf(ALPM_LOG_DEBUG, "config: new section '%s'\n", name); @@ -872,6 +873,9 @@ static int _parse_directive(const char *file, int linenum, const char *name, return 0; }

+typedef int (ini_parser_fn)(const char *file, int line, const char *section, + char *key, char *value, void *data); + /** The "real" parseconfig. Each "Include" directive will recall this method so * recursion and stack depth are limited to 10 levels. The publicly visible * parseconfig calls this with a NULL section argument so we can recall from @@ -881,7 +885,7 @@ static int _parse_directive(const char *file, int linenum, const char *name, * @param depth the current recursion depth * @return 0 on success, 1 on failure */ -static int _parseconfig(const char *file, struct section_t *section, +static int _parseconfig(const char *file, ini_parser_fn cb, void *data, char **section_name, int depth) { FILE *fp = NULL; @@ -936,7 +940,7 @@ static int _parseconfig(const char *file, struct section_t *section, name = strdup(line + 1); name[line_len - 2] = '\0';

- ret = _parse_directive(file, linenum, name, NULL, NULL, section); + ret = cb(file, linenum, name, NULL, NULL, data); free(*section_name); *section_name = name;

@@ -994,7 +998,7 @@ static int _parseconfig(const char *file, struct section_t *section, for(gindex = 0; gindex < globbuf.gl_pathc; gindex++) { pm_printf(ALPM_LOG_DEBUG, "config file %s, line %d: including %s\n", file, linenum, globbuf.gl_pathv[gindex]); - _parseconfig(globbuf.gl_pathv[gindex], section, + _parseconfig(globbuf.gl_pathv[gindex], cb, data, section_name, depth + 1); } break; @@ -1002,13 +1006,13 @@ static int _parseconfig(const char *file, struct section_t *section, globfree(&globbuf); continue; } - if((ret = _parse_directive(file, linenum, *section_name, key, value, section)) != 0) { + if((ret = cb(file, linenum, *section_name, key, value, data)) != 0) { goto cleanup; } }

if(depth == 0) { - ret = _parse_directive(NULL, 0, NULL, NULL, NULL, section); + ret = cb(NULL, 0, NULL, NULL, NULL, data); }

cleanup: @@ -1041,7 +1045,7 @@ int parseconfig(const char *file) /* call the real parseconfig function with a null section & db argument */ pm_printf(ALPM_LOG_DEBUG, "parseconfig: options pass\n"); section.parse_options = 1; - if((ret = _parseconfig(file, §ion, §ion_name, 0))) { + if((ret = _parseconfig(file, _parse_directive, §ion, §ion_name, 0))) { return ret; } if((ret = setup_libalpm())) { @@ -1050,7 +1054,7 @@ int parseconfig(const char *file) /* second pass, repo section parsing */ pm_printf(ALPM_LOG_DEBUG, "parseconfig: repo pass\n"); section.parse_options = 0; - return _parseconfig(file, §ion, §ion_name, 0); + return _parseconfig(file, _parse_directive, §ion, §ion_name, 0); }

/* vim: set ts=2 sw=2 noet: */

Allan McRae

2:17 p.m.

New subject: [pacman-dev] [PATCH 5/9] conf.c: pass _parse_directive as a callback

On 29/07/13 23:19, Andrew Gregory wrote:

...

On 07/29/13 at 02:18pm, Allan McRae wrote:

...
On 22/07/13 16:46, Andrew Gregory wrote:

...
This will allow passing arbitrary key/value handlers.

I have now gone through this entire patchset and this is the only patch that confuses me. What do we gain by doing this? Are you envisioning having _parse_directive and _parse_hook or something? Will that be needed?

Allan

Yes, _parse_hook is exactly what I was envisioning. This patch completely decouples the ini parser from the handling of the key/value pairs, making it easy to reuse for any file that uses our format simply by passing a different key/value handler. Aside from hooks, I also hope to eventually make the parser available to other pacman-related projects so that they can all easily use the same config format.

Ah - this all makes sense to me now, and I think it would have earlier if _parse_directive had not been such a generic name that I thought it to be used for every ini parsing. Of course, looking at the code, it is not. I was going to suggest a less generic name, but once _parseconfig is moved and turned into parse_ini at next, and noting it is in conf.c, it is really quite clear. So the patch is all good to me. Allan

Andrew Gregory

22 Jul 22 Jul

6:46 a.m.

New subject: [pacman-dev] [PATCH 6/9] conf.c: extract ini parsing code to separate files

Move _parseconfig to ini.c as _parse_ini and create a convenient wrapper for the public API. Signed-off-by: Andrew Gregory --- src/pacman/Makefile.am | 1 + src/pacman/conf.c | 161 +------------------------------------ src/pacman/ini.c | 211 +++++++++++++++++++++++++++++++++++++++++++++++++ src/pacman/ini.h | 30 +++++++ 4 files changed, 245 insertions(+), 158 deletions(-) create mode 100644 src/pacman/ini.c create mode 100644 src/pacman/ini.h diff --git a/src/pacman/Makefile.am b/src/pacman/Makefile.am index ed51573..f7272c6 100644 --- a/src/pacman/Makefile.am +++ b/src/pacman/Makefile.am @@ -31,6 +31,7 @@ pacman_SOURCES = \ conf.h conf.c \ database.c \ deptest.c \ + ini.h ini.c \ package.h package.c \ pacman.h pacman.c \ query.c \ diff --git a/src/pacman/conf.c b/src/pacman/conf.c index 704ce13..77dc659 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -19,7 +19,6 @@ */ #include -#include #include #include /* setlocale */ #include /* open */ @@ -33,6 +32,7 @@ /* pacman */ #include "conf.h" +#include "ini.h" #include "util.h" #include "pacman.h" #include "callback.h" @@ -873,160 +873,6 @@ static int _parse_directive(const char *file, int linenum, const char *name, return 0; } -typedef int (ini_parser_fn)(const char *file, int line, const char *section, - char *key, char *value, void *data); - -/** The "real" parseconfig. Each "Include" directive will recall this method so - * recursion and stack depth are limited to 10 levels. The publicly visible - * parseconfig calls this with a NULL section argument so we can recall from - * within ourself on an include. - * @param file path to the config file - * @param section the current active section - * @param depth the current recursion depth - * @return 0 on success, 1 on failure - */ -static int _parseconfig(const char *file, ini_parser_fn cb, void *data, - char **section_name, int depth) -{ - FILE *fp = NULL; - char line[PATH_MAX]; - int linenum = 0; - int ret = 0; - const int max_depth = 10; - - if(depth >= max_depth) { - pm_printf(ALPM_LOG_ERROR, - _("config parsing exceeded max recursion depth of %d.\n"), max_depth); - ret = 1; - goto cleanup; - } - - pm_printf(ALPM_LOG_DEBUG, "config: attempting to read file %s\n", file); - fp = fopen(file, "r"); - if(fp == NULL) { - pm_printf(ALPM_LOG_ERROR, _("config file %s could not be read: %s\n"), - file, strerror(errno)); - ret = 1; - goto cleanup; - } - - while(fgets(line, PATH_MAX, fp)) { - char *key, *value, *ptr; - size_t line_len; - - linenum++; - - /* ignore whole line and end of line comments */ - if((ptr = strchr(line, '#'))) { - *ptr = '\0'; - } - - line_len = strtrim(line); - - if(line_len == 0) { - continue; - } - - if(line[0] == '[' && line[line_len - 1] == ']') { - char *name; - /* only possibility here is a line == '[]' */ - if(line_len <= 2) { - pm_printf(ALPM_LOG_ERROR, _("config file %s, line %d: bad section name.\n"), - file, linenum); - ret = 1; - goto cleanup; - } - /* new config section, skip the '[' */ - name = strdup(line + 1); - name[line_len - 2] = '\0'; - - ret = cb(file, linenum, name, NULL, NULL, data); - free(*section_name); - *section_name = name; - - if(ret) { - goto cleanup; - } - continue; - } - - /* directive */ - /* strsep modifies the 'line' string: 'key \0 value' */ - key = line; - value = line; - strsep(&value, "="); - strtrim(key); - strtrim(value); - - if(key == NULL) { - pm_printf(ALPM_LOG_ERROR, _("config file %s, line %d: syntax error in config file- missing key.\n"), - file, linenum); - ret = 1; - goto cleanup; - } - /* Include is allowed in both options and repo sections */ - if(strcmp(key, "Include") == 0) { - glob_t globbuf; - int globret; - size_t gindex; - - if(value == NULL) { - pm_printf(ALPM_LOG_ERROR, _("config file %s, line %d: directive '%s' needs a value\n"), - file, linenum, key); - ret = 1; - goto cleanup; - } - /* Ignore include failures... assume non-critical */ - globret = glob(value, GLOB_NOCHECK, NULL, &globbuf); - switch(globret) { - case GLOB_NOSPACE: - pm_printf(ALPM_LOG_DEBUG, - "config file %s, line %d: include globbing out of space\n", - file, linenum); - break; - case GLOB_ABORTED: - pm_printf(ALPM_LOG_DEBUG, - "config file %s, line %d: include globbing read error for %s\n", - file, linenum, value); - break; - case GLOB_NOMATCH: - pm_printf(ALPM_LOG_DEBUG, - "config file %s, line %d: no include found for %s\n", - file, linenum, value); - break; - default: - for(gindex = 0; gindex < globbuf.gl_pathc; gindex++) { - pm_printf(ALPM_LOG_DEBUG, "config file %s, line %d: including %s\n", - file, linenum, globbuf.gl_pathv[gindex]); - _parseconfig(globbuf.gl_pathv[gindex], cb, data, - section_name, depth + 1); - } - break; - } - globfree(&globbuf); - continue; - } - if((ret = cb(file, linenum, *section_name, key, value, data)) != 0) { - goto cleanup; - } - } - - if(depth == 0) { - ret = cb(NULL, 0, NULL, NULL, NULL, data); - } - -cleanup: - if(fp) { - fclose(fp); - } - if(depth == 0) { - free(*section_name); - *section_name = NULL; - } - pm_printf(ALPM_LOG_DEBUG, "config: finished parsing %s\n", file); - return ret; -} - /** Parse a configuration file. * @param file path to the config file * @return 0 on success, non-zero on error @@ -1035,7 +881,6 @@ int parseconfig(const char *file) { int ret; struct section_t section; - char *section_name = NULL; memset(§ion, 0, sizeof(struct section_t)); section.siglevel = ALPM_SIG_USE_DEFAULT; /* the config parse is a two-pass affair. We first parse the entire thing for @@ -1045,7 +890,7 @@ int parseconfig(const char *file) /* call the real parseconfig function with a null section & db argument */ pm_printf(ALPM_LOG_DEBUG, "parseconfig: options pass\n"); section.parse_options = 1; - if((ret = _parseconfig(file, _parse_directive, §ion, §ion_name, 0))) { + if((ret = parse_ini(file, _parse_directive, §ion))) { return ret; } if((ret = setup_libalpm())) { @@ -1054,7 +899,7 @@ int parseconfig(const char *file) /* second pass, repo section parsing */ pm_printf(ALPM_LOG_DEBUG, "parseconfig: repo pass\n"); section.parse_options = 0; - return _parseconfig(file, _parse_directive, §ion, §ion_name, 0); + return parse_ini(file, _parse_directive, §ion); } /* vim: set ts=2 sw=2 noet: */ diff --git a/src/pacman/ini.c b/src/pacman/ini.c new file mode 100644 index 0000000..4d25f58 --- /dev/null +++ b/src/pacman/ini.c @@ -0,0 +1,211 @@ +/* + * ini.c + * + * Copyright (c) 2013 Pacman Development Team + * + * 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 2 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/. + */ + +#include +#include +#include +#include /* strdup */ + +#include + +#include "ini.h" +#include "util.h" + +/** + * @brief INI parser backend. + * + * @param file path to the config file + * @param cb callback for key/value pairs + * @param data caller defined data to be passed to the callback + * @param section_name the name of the current section + * @param depth recursion depth, should initially be 0 + * + * @return 0 on success, 1 on parsing errors, the callback return value + * otherwise + */ +static int _parse_ini(const char *file, ini_parser_fn cb, void *data, + char **section_name, int depth) +{ + FILE *fp = NULL; + char line[PATH_MAX]; + int linenum = 0; + int ret = 0; + const int max_depth = 10; + + if(depth >= max_depth) { + pm_printf(ALPM_LOG_ERROR, + _("config parsing exceeded max recursion depth of %d.\n"), max_depth); + ret = 1; + goto cleanup; + } + + pm_printf(ALPM_LOG_DEBUG, "config: attempting to read file %s\n", file); + fp = fopen(file, "r"); + if(fp == NULL) { + pm_printf(ALPM_LOG_ERROR, _("config file %s could not be read: %s\n"), + file, strerror(errno)); + ret = 1; + goto cleanup; + } + + while(fgets(line, PATH_MAX, fp)) { + char *key, *value, *ptr; + size_t line_len; + + linenum++; + + /* ignore whole line and end of line comments */ + if((ptr = strchr(line, '#'))) { + *ptr = '\0'; + } + + line_len = strtrim(line); + + if(line_len == 0) { + continue; + } + + if(line[0] == '[' && line[line_len - 1] == ']') { + char *name; + /* only possibility here is a line == '[]' */ + if(line_len <= 2) { + pm_printf(ALPM_LOG_ERROR, _("config file %s, line %d: bad section name.\n"), + file, linenum); + ret = 1; + goto cleanup; + } + /* new config section, skip the '[' */ + name = strdup(line + 1); + name[line_len - 2] = '\0'; + + ret = cb(file, linenum, name, NULL, NULL, data); + free(*section_name); + *section_name = name; + + /* we're at a new section; perform any post-actions for the prior */ + if(ret) { + goto cleanup; + } + continue; + } + + /* directive */ + /* strsep modifies the 'line' string: 'key \0 value' */ + key = line; + value = line; + strsep(&value, "="); + strtrim(key); + strtrim(value); + + if(key == NULL) { + pm_printf(ALPM_LOG_ERROR, _("config file %s, line %d: syntax error in config file- missing key.\n"), + file, linenum); + ret = 1; + goto cleanup; + } + /* Include is allowed in both options and repo sections */ + if(strcmp(key, "Include") == 0) { + glob_t globbuf; + int globret; + size_t gindex; + + if(value == NULL) { + pm_printf(ALPM_LOG_ERROR, _("config file %s, line %d: directive '%s' needs a value\n"), + file, linenum, key); + ret = 1; + goto cleanup; + } + /* Ignore include failures... assume non-critical */ + globret = glob(value, GLOB_NOCHECK, NULL, &globbuf); + switch(globret) { + case GLOB_NOSPACE: + pm_printf(ALPM_LOG_DEBUG, + "config file %s, line %d: include globbing out of space\n", + file, linenum); + break; + case GLOB_ABORTED: + pm_printf(ALPM_LOG_DEBUG, + "config file %s, line %d: include globbing read error for %s\n", + file, linenum, value); + break; + case GLOB_NOMATCH: + pm_printf(ALPM_LOG_DEBUG, + "config file %s, line %d: no include found for %s\n", + file, linenum, value); + break; + default: + for(gindex = 0; gindex < globbuf.gl_pathc; gindex++) { + pm_printf(ALPM_LOG_DEBUG, "config file %s, line %d: including %s\n", + file, linenum, globbuf.gl_pathv[gindex]); + _parse_ini(globbuf.gl_pathv[gindex], cb, data, + section_name, depth + 1); + } + break; + } + globfree(&globbuf); + continue; + } + if((ret = cb(file, linenum, *section_name, key, value, data)) != 0) { + goto cleanup; + } + } + + if(depth == 0) { + ret = cb(NULL, 0, NULL, NULL, NULL, data); + } + +cleanup: + if(fp) { + fclose(fp); + } + if(depth == 0) { + free(*section_name); + *section_name = NULL; + } + pm_printf(ALPM_LOG_DEBUG, "config: finished parsing %s\n", file); + return ret; +} + +/** + * @brief Parse a pacman-style INI config file. + * + * @param file path to the config file + * @param cb callback for key/value pairs + * @param data caller defined data to be passed to the callback + * + * @return 0 on success, 1 on parsing errors, the callback return value + * otherwise + * + * @note The callback will be called at the beginning of each section with an + * empty key and value, for each key/value pair, and when parsing is complete + * with all arguments except @a data empty. + * + * @note The @a key and @a value passed to @ cb will be overwritten between + * calls. The section name will remain valid until after @a cb is called to + * begin a new section. + * + * @note Parsing will immediately stop if the callback returns non-zero. + */ +int parse_ini(const char *file, ini_parser_fn cb, void *data) +{ + char *section_name = NULL; + return _parse_ini(file, cb, data, §ion_name, 0); +} + +/* vim: set ts=2 sw=2 noet: */ diff --git a/src/pacman/ini.h b/src/pacman/ini.h new file mode 100644 index 0000000..4d403c1 --- /dev/null +++ b/src/pacman/ini.h @@ -0,0 +1,30 @@ +/* + * ini.h + * + * Copyright (c) 2013 Pacman Development Team + * + * 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 2 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/. + */ + +#ifndef _PM_INI_H +#define _PM_INI_H + +typedef int (ini_parser_fn)(const char *file, int line, const char *section, + char *key, char *value, void *data); + +int parse_ini(const char *file, ini_parser_fn cb, void *data); + +#endif /* _PM_CONF_H */ + +/* vim: set ts=2 sw=2 noet: */ -- 1.8.3.3

Andrew Gregory

6:46 a.m.

New subject: [pacman-dev] [PATCH 7/9] ini.c: move recursion limit to a macro

Signed-off-by: Andrew Gregory --- src/pacman/ini.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/pacman/ini.c b/src/pacman/ini.c index 4d25f58..1c37f5c 100644 --- a/src/pacman/ini.c +++ b/src/pacman/ini.c @@ -27,6 +27,8 @@ #include "ini.h" #include "util.h" +#define INI_MAX_RECURSION 10 + /** * @brief INI parser backend. * @@ -46,11 +48,11 @@ static int _parse_ini(const char *file, ini_parser_fn cb, void *data, char line[PATH_MAX]; int linenum = 0; int ret = 0; - const int max_depth = 10; - if(depth >= max_depth) { + if(INI_MAX_RECURSION && depth >= INI_MAX_RECURSION) { pm_printf(ALPM_LOG_ERROR, - _("config parsing exceeded max recursion depth of %d.\n"), max_depth); + _("config parsing exceeded max recursion depth of %d.\n"), + INI_MAX_RECURSION); ret = 1; goto cleanup; } -- 1.8.3.3

Dave Reisner

12:41 p.m.

New subject: [pacman-dev] [PATCH 7/9] ini.c: move recursion limit to a macro

On Jul 22, 2013 2:48 AM, "Andrew Gregory" wrote:

...

Signed-off-by: Andrew Gregory ---

OK... I'll bite. Why? Is this really going to be used elsewhere? Seems like an error case that should be handled by the like any other.

...

src/pacman/ini.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/pacman/ini.c b/src/pacman/ini.c index 4d25f58..1c37f5c 100644 --- a/src/pacman/ini.c +++ b/src/pacman/ini.c @@ -27,6 +27,8 @@ #include "ini.h" #include "util.h"

+#define INI_MAX_RECURSION 10 + /** * @brief INI parser backend. * @@ -46,11 +48,11 @@ static int _parse_ini(const char *file, ini_parser_fn cb, void *data, char line[PATH_MAX]; int linenum = 0; int ret = 0; - const int max_depth = 10;

- if(depth >= max_depth) { + if(INI_MAX_RECURSION && depth >= INI_MAX_RECURSION) { pm_printf(ALPM_LOG_ERROR, - _("config parsing exceeded max recursion depth of %d.\n"), max_depth); + _("config parsing exceeded max recursion depth of %d.\n"), + INI_MAX_RECURSION); ret = 1; goto cleanup; } -- 1.8.3.3

Andrew Gregory

2:58 p.m.

New subject: [pacman-dev] [PATCH 7/9] ini.c: move recursion limit to a macro

On 07/22/13 at 08:41am, Dave Reisner wrote:

...

On Jul 22, 2013 2:48 AM, "Andrew Gregory" wrote:

...
Signed-off-by: Andrew Gregory ---

OK... I'll bite. Why? Is this really going to be used elsewhere? Seems like an error case that should be handled by the like any other.

My goal wasn't reusability so much as visibility. It's an artificial limitation we impose to prevent memory exhaustion in a recursive call, not a true error condition. I also did this with the thought of eventually trying to make the parser usable for pacman-related programs outside of pacman proper and making the limit configurable. I don't know if a macro is the best way to do that, though. apg

Dave Reisner

3:02 p.m.

New subject: [pacman-dev] [PATCH 7/9] ini.c: move recursion limit to a macro

On Mon, Jul 22, 2013 at 10:58:55AM -0400, Andrew Gregory wrote:

...

On 07/22/13 at 08:41am, Dave Reisner wrote:

...
On Jul 22, 2013 2:48 AM, "Andrew Gregory" wrote:

...
Signed-off-by: Andrew Gregory ---

OK... I'll bite. Why? Is this really going to be used elsewhere? Seems like an error case that should be handled by the like any other.

My goal wasn't reusability so much as visibility. It's an artificial limitation we impose to prevent memory exhaustion in a recursive call, not a true error condition. I also did this with the thought of eventually trying to make the parser usable for pacman-related programs outside of pacman proper and making the limit configurable. I don't know if a macro is the best way to do that, though.

apg

Although minor, I'm against this. If you want this to be extensible and useful outside of pacman proper (and this is indeed a noble goal), then you need to create an opaque context that users would instantiate similar to alpm_handle_t. The recursion limit would have get/set methods exposed in the API.

Allan McRae

29 Jul 29 Jul

3:35 a.m.

New subject: [pacman-dev] [PATCH 7/9] ini.c: move recursion limit to a macro

On 23/07/13 01:02, Dave Reisner wrote:

...

On Mon, Jul 22, 2013 at 10:58:55AM -0400, Andrew Gregory wrote:

...
On 07/22/13 at 08:41am, Dave Reisner wrote:

...
On Jul 22, 2013 2:48 AM, "Andrew Gregory" wrote:

...
Signed-off-by: Andrew Gregory ---

OK... I'll bite. Why? Is this really going to be used elsewhere? Seems like an error case that should be handled by the like any other.

My goal wasn't reusability so much as visibility. It's an artificial limitation we impose to prevent memory exhaustion in a recursive call, not a true error condition. I also did this with the thought of eventually trying to make the parser usable for pacman-related programs outside of pacman proper and making the limit configurable. I don't know if a macro is the best way to do that, though.

apg

Although minor, I'm against this.

If you want this to be extensible and useful outside of pacman proper (and this is indeed a noble goal), then you need to create an opaque context that users would instantiate similar to alpm_handle_t. The recursion limit would have get/set methods exposed in the API.

I do like the idea of increasing the visibility of this limitation. How about instead of using a macro, just make it a "static const int" at the top of the file? A

Andrew Gregory

2 Aug 2 Aug

12:29 p.m.

New subject: [pacman-dev] [PATCH] ini.c: give recursion limit file scope

The recursion limit is an artificial limitation imposed to prevent memory exhaustion in a recursive function. Giving it file-level scope increases its visibility. Signed-off-by: Andrew Gregory --- src/pacman/ini.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/pacman/ini.c b/src/pacman/ini.c index 2e677e0..cd7741d 100644 --- a/src/pacman/ini.c +++ b/src/pacman/ini.c @@ -27,6 +27,8 @@ #include "ini.h" #include "util.h" +static const int ini_max_recursion = 10; + /** * @brief INI parser backend. * @@ -46,11 +48,11 @@ static int _parse_ini(const char *file, ini_parser_fn cb, void *data, FILE *fp = NULL; int linenum = 0; int ret = 0; - const int max_depth = 10; - if(depth >= max_depth) { + if(depth >= ini_max_recursion) { pm_printf(ALPM_LOG_ERROR, - _("config parsing exceeded max recursion depth of %d.\n"), max_depth); + _("config parsing exceeded max recursion depth of %d.\n"), + ini_max_recursion); ret = 1; goto cleanup; } -- 1.8.3.4

Andrew Gregory

22 Jul 22 Jul

6:46 a.m.

New subject: [pacman-dev] [PATCH 8/9] ini.c: reuse line buffer

By the time we make the recursive call we have already finished with the line buffer, making it safe to reuse. Signed-off-by: Andrew Gregory --- src/pacman/ini.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/pacman/ini.c b/src/pacman/ini.c index 1c37f5c..95c85a5 100644 --- a/src/pacman/ini.c +++ b/src/pacman/ini.c @@ -36,16 +36,16 @@ * @param cb callback for key/value pairs * @param data caller defined data to be passed to the callback * @param section_name the name of the current section + * @param line buffer to read into, must be at least PATH_MAX long * @param depth recursion depth, should initially be 0 * * @return 0 on success, 1 on parsing errors, the callback return value * otherwise */ static int _parse_ini(const char *file, ini_parser_fn cb, void *data, - char **section_name, int depth) + char **section_name, char *line, int depth) { FILE *fp = NULL; - char line[PATH_MAX]; int linenum = 0; int ret = 0; @@ -156,7 +156,7 @@ static int _parse_ini(const char *file, ini_parser_fn cb, void *data, pm_printf(ALPM_LOG_DEBUG, "config file %s, line %d: including %s\n", file, linenum, globbuf.gl_pathv[gindex]); _parse_ini(globbuf.gl_pathv[gindex], cb, data, - section_name, depth + 1); + section_name, line, depth + 1); } break; } @@ -206,8 +206,8 @@ cleanup: */ int parse_ini(const char *file, ini_parser_fn cb, void *data) { - char *section_name = NULL; - return _parse_ini(file, cb, data, §ion_name, 0); + char *section_name = NULL, line[PATH_MAX]; + return _parse_ini(file, cb, data, §ion_name, line, 0); } /* vim: set ts=2 sw=2 noet: */ -- 1.8.3.3

Andrew Gregory

6:46 a.m.

New subject: [pacman-dev] [PATCH 9/9] ini.c: make errors in includes fatal

If an error in the main file would be fatal there is little reason to ignore the error in an included file. Signed-off-by: Andrew Gregory --- src/pacman/ini.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/pacman/ini.c b/src/pacman/ini.c index 95c85a5..c2399ba 100644 --- a/src/pacman/ini.c +++ b/src/pacman/ini.c @@ -155,8 +155,12 @@ static int _parse_ini(const char *file, ini_parser_fn cb, void *data, for(gindex = 0; gindex < globbuf.gl_pathc; gindex++) { pm_printf(ALPM_LOG_DEBUG, "config file %s, line %d: including %s\n", file, linenum, globbuf.gl_pathv[gindex]); - _parse_ini(globbuf.gl_pathv[gindex], cb, data, + ret =_parse_ini(globbuf.gl_pathv[gindex], cb, data, section_name, line, depth + 1); + if(ret) { + globfree(&globbuf); + goto cleanup; + } } break; } -- 1.8.3.3

Allan McRae

7 Aug 7 Aug

12:57 p.m.

On 22/07/13 16:46, Andrew Gregory wrote:

...

This creates a generic ini parser than can be reused for hooks or other configuration files. I took a very different approach from the previous patchset by utilizing a callback that processes the options as the file is parsed. This should have a much lower memory footprint and is closer to how we were already parsing config files.

Parsing differences: * Include directives can be outside a section. They can include their own sections so it made little sense to require that they be in one already. * Errors in included files are now fatal.

These patches look fine to me and it sets us up for parsing ini based hook configuration files. Any other comments before I commit these? Allan

3929

Age (days ago)

3945

Last active (days ago)

List overview

Download

18 comments

3 participants

participants (3)

Allan McRae
Andrew Gregory
Dave Reisner

[pacman-dev] [PATCH 0/9] extract ini parser from parseconfig

tags

participants (3)