From 8069b3b14b1906bf98811a0d5fa2ca068e947933 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Mon, 29 Jul 2013 18:05:33 +0200 Subject: [PATCH 01/27] array: Allocate initial data properly if esize is 0 --- src/libstrongswan/collections/array.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstrongswan/collections/array.c b/src/libstrongswan/collections/array.c index 75efb85bf..8d619116a 100644 --- a/src/libstrongswan/collections/array.c +++ b/src/libstrongswan/collections/array.c @@ -168,7 +168,7 @@ array_t *array_create(u_int esize, u_int8_t reserve) ); if (array->tail) { - array->data = malloc(array->tail * array->esize); + array->data = malloc(get_size(array, array->tail)); } return array; } From b9b1114ab1f53fc6afa93d01718b93de6326f358 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 4 Mar 2014 14:18:42 +0100 Subject: [PATCH 02/27] settings: Move to a separate folder --- src/libstrongswan/Android.mk | 6 +++--- src/libstrongswan/Makefile.am | 11 +++++------ src/libstrongswan/library.h | 2 +- src/libstrongswan/{utils => settings}/settings.c | 0 src/libstrongswan/{utils => settings}/settings.h | 7 +++++-- src/libstrongswan/tests/suites/test_settings.c | 2 +- 6 files changed, 15 insertions(+), 13 deletions(-) rename src/libstrongswan/{utils => settings}/settings.c (100%) rename src/libstrongswan/{utils => settings}/settings.h (98%) diff --git a/src/libstrongswan/Android.mk b/src/libstrongswan/Android.mk index 24f0bc552..5f22bf114 100644 --- a/src/libstrongswan/Android.mk +++ b/src/libstrongswan/Android.mk @@ -32,12 +32,12 @@ networking/streams/stream_service.c networking/streams/stream_manager.c \ pen/pen.c plugins/plugin_loader.c plugins/plugin_feature.c processing/jobs/job.c \ processing/jobs/callback_job.c processing/processor.c processing/scheduler.c \ processing/watcher.c resolver/resolver_manager.c resolver/rr_set.c \ -selectors/traffic_selector.c threading/thread.c threading/thread_value.c \ +selectors/traffic_selector.c settings/settings.c \ +threading/thread.c threading/thread_value.c \ threading/mutex.c threading/semaphore.c threading/rwlock.c threading/spinlock.c \ utils/utils.c utils/chunk.c utils/debug.c utils/enum.c utils/identification.c \ utils/lexparser.c utils/optionsfrom.c utils/capabilities.c utils/backtrace.c \ -utils/printf_hook/printf_hook_builtin.c utils/settings.c utils/test.c \ -utils/utils/strerror.c +utils/printf_hook/printf_hook_builtin.c utils/test.c utils/utils/strerror.c # adding the plugin source files diff --git a/src/libstrongswan/Makefile.am b/src/libstrongswan/Makefile.am index 3462d2ffc..d202b023d 100644 --- a/src/libstrongswan/Makefile.am +++ b/src/libstrongswan/Makefile.am @@ -30,12 +30,12 @@ networking/streams/stream_service.c networking/streams/stream_manager.c \ pen/pen.c plugins/plugin_loader.c plugins/plugin_feature.c processing/jobs/job.c \ processing/jobs/callback_job.c processing/processor.c processing/scheduler.c \ processing/watcher.c resolver/resolver_manager.c resolver/rr_set.c \ -selectors/traffic_selector.c threading/thread.c threading/thread_value.c \ +selectors/traffic_selector.c settings/settings.c \ +threading/thread.c threading/thread_value.c \ threading/mutex.c threading/semaphore.c threading/rwlock.c threading/spinlock.c \ utils/utils.c utils/chunk.c utils/debug.c utils/enum.c utils/identification.c \ utils/lexparser.c utils/optionsfrom.c utils/capabilities.c utils/backtrace.c \ -utils/settings.c utils/test.c \ -utils/utils/strerror.c +utils/test.c utils/utils/strerror.c if USE_DEV_HEADERS strongswan_includedir = ${dev_headers} @@ -75,15 +75,14 @@ resolver/rr.h resolver/resolver_manager.h \ plugins/plugin_loader.h plugins/plugin.h plugins/plugin_feature.h \ processing/jobs/job.h processing/jobs/callback_job.h processing/processor.h \ processing/scheduler.h processing/watcher.h selectors/traffic_selector.h \ -threading/thread.h threading/thread_value.h \ +settings/settings.h threading/thread.h threading/thread_value.h \ threading/mutex.h threading/condvar.h threading/spinlock.h threading/semaphore.h \ threading/rwlock.h threading/rwlock_condvar.h threading/lock_profiler.h \ utils/utils.h utils/chunk.h utils/debug.h utils/enum.h utils/identification.h \ utils/lexparser.h utils/optionsfrom.h utils/capabilities.h utils/backtrace.h \ utils/leak_detective.h utils/printf_hook/printf_hook.h \ utils/printf_hook/printf_hook_vstr.h utils/printf_hook/printf_hook_builtin.h \ -utils/settings.h utils/test.h utils/integrity_checker.h \ -utils/utils/strerror.h +utils/test.h utils/integrity_checker.h utils/utils/strerror.h endif library.lo : $(top_builddir)/config.status diff --git a/src/libstrongswan/library.h b/src/libstrongswan/library.h index 4125328b7..37a83fa2f 100644 --- a/src/libstrongswan/library.h +++ b/src/libstrongswan/library.h @@ -113,8 +113,8 @@ #include "utils/capabilities.h" #include "utils/integrity_checker.h" #include "utils/leak_detective.h" -#include "utils/settings.h" #include "plugins/plugin_loader.h" +#include "settings/settings.h" typedef struct library_t library_t; diff --git a/src/libstrongswan/utils/settings.c b/src/libstrongswan/settings/settings.c similarity index 100% rename from src/libstrongswan/utils/settings.c rename to src/libstrongswan/settings/settings.c diff --git a/src/libstrongswan/utils/settings.h b/src/libstrongswan/settings/settings.h similarity index 98% rename from src/libstrongswan/utils/settings.h rename to src/libstrongswan/settings/settings.h index 46403c4d3..23f3a8ead 100644 --- a/src/libstrongswan/utils/settings.h +++ b/src/libstrongswan/settings/settings.h @@ -16,7 +16,10 @@ /** * @defgroup settings settings - * @{ @ingroup utils + * @ingroup libstrongswan + * + * @defgroup settings_t settings + * @{ @ingroup settings */ #ifndef SETTINGS_H_ @@ -24,7 +27,7 @@ typedef struct settings_t settings_t; -#include "utils.h" +#include "utils/utils.h" #include "collections/enumerator.h" /** diff --git a/src/libstrongswan/tests/suites/test_settings.c b/src/libstrongswan/tests/suites/test_settings.c index 096465191..8a08e360c 100644 --- a/src/libstrongswan/tests/suites/test_settings.c +++ b/src/libstrongswan/tests/suites/test_settings.c @@ -17,7 +17,7 @@ #include -#include +#include #include #include #include From 96de74b879f478955246aeee382afb5476187228 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 4 Sep 2013 18:14:29 +0200 Subject: [PATCH 03/27] enumerator: Add enumerator to enumerate files matching a pattern This enumerator is a wrapper around glob(3). If that function is not supported NULL is returned. If no files match or an error occurs during the pattern expansion an error is logged and the enumerator simply returns no items. RFC: if GLOB_ERR is not supplied glob returns GLOB_NOMATCH if e.g. the base directory of the pattern does not exist, which would otherwise result in an error. This way there is at least a clear error message in case of a typo. --- src/libstrongswan/collections/enumerator.c | 106 ++++++++++++++++++++- src/libstrongswan/collections/enumerator.h | 37 ++++++- 2 files changed, 140 insertions(+), 3 deletions(-) diff --git a/src/libstrongswan/collections/enumerator.c b/src/libstrongswan/collections/enumerator.c index 8049ac016..0c97f796f 100644 --- a/src/libstrongswan/collections/enumerator.c +++ b/src/libstrongswan/collections/enumerator.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2008 Tobias Brunner + * Copyright (C) 2008-2013 Tobias Brunner * Copyright (C) 2007 Martin Willi * Hochschule fuer Technik Rapperswil * @@ -25,6 +25,10 @@ #include #include +#ifdef HAVE_GLOB_H +#include +#endif /* HAVE_GLOB_H */ + #include /** @@ -157,8 +161,106 @@ enumerator_t* enumerator_create_directory(const char *path) return &this->public; } +#ifdef HAVE_GLOB_H + /** - * Enumerator implementation for directory enumerator + * Enumerator implementation for glob enumerator + */ +typedef struct { + /** implements enumerator_t */ + enumerator_t public; + /** glob data */ + glob_t glob; + /** current match */ + char **match; + /** absolute path of current file */ + char full[PATH_MAX]; +} glob_enum_t; + +/** + * Implementation of enumerator_create_glob().destroy + */ +static void destroy_glob_enum(glob_enum_t *this) +{ + globfree(&this->glob); + free(this); +} + +/** + * Implementation of enumerator_create_glob().enumerate + */ +static bool enumerate_glob_enum(glob_enum_t *this, char **file, struct stat *st) +{ + char *match = *(++this->match); + + if (!match) + { + return FALSE; + } + if (file) + { + *file = match; + } + if (st) + { + if (stat(match, st)) + { + DBG1(DBG_LIB, "stat() on '%s' failed: %s", match, + strerror(errno)); + return FALSE; + } + } + return TRUE; +} + +/** + * See header + */ +enumerator_t* enumerator_create_glob(const char *pattern) +{ + glob_enum_t *this; + int status; + + if (!pattern) + { + return enumerator_create_empty(); + } + + INIT(this, + .public = { + .enumerate = (void*)enumerate_glob_enum, + .destroy = (void*)destroy_glob_enum, + }, + .glob = { + .gl_offs = 1, /* reserve one slot so we can enumerate easily */ + } + ); + + status = glob(pattern, GLOB_DOOFFS | GLOB_ERR, NULL, &this->glob); + if (status == GLOB_NOMATCH) + { + DBG1(DBG_LIB, "no files found matching '%s'", pattern); + } + else if (status != 0) + { + DBG1(DBG_LIB, "expanding file pattern '%s' failed: %s", pattern, + strerror(errno)); + } + this->match = this->glob.gl_pathv; + return &this->public; +} + +#else /* HAVE_GLOB_H */ + +enumerator_t* enumerator_create_glob(const char *pattern) +{ + return NULL; +} + +#endif /* HAVE_GLOB_H */ + +/** + * Enumerator implementation for token enumerator */ typedef struct { /** implements enumerator_t */ diff --git a/src/libstrongswan/collections/enumerator.h b/src/libstrongswan/collections/enumerator.h index 299373a3e..55f8d83e6 100644 --- a/src/libstrongswan/collections/enumerator.h +++ b/src/libstrongswan/collections/enumerator.h @@ -1,4 +1,5 @@ /* + * Copyright (C) 2013 Tobias Brunner * Copyright (C) 2007 Martin Willi * Hochschule fuer Technik Rapperswil * @@ -69,7 +70,9 @@ enumerator_t *enumerator_create_single(void *item, void (*cleanup)(void *item)); * This enumerator_t.enumerate() function returns a (to the directory) relative * filename (as a char*), an absolute filename (as a char*) and a file status * (to a struct stat), which all may be NULL. "." and ".." entries are - * skipped. Example: + * skipped. + * + * Example: * * @code char *rel, *abs; @@ -95,6 +98,38 @@ enumerator_t *enumerator_create_single(void *item, void (*cleanup)(void *item)); */ enumerator_t* enumerator_create_directory(const char *path); +/** + * Create an enumerator over files/directories matching a file pattern. + * + * This enumerator_t.enumerate() function returns the filename (as a char*), + * and a file status (to a struct stat), which both may be NULL. + * + * Example: + * + * @code + char *file; + struct stat st; + enumerator_t *e; + + e = enumerator_create_glob("/etc/ipsec.*.conf"); + if (e) + { + while (e->enumerate(e, &file, &st)) + { + if (S_ISREG(st.st_mode)) + { + printf("%s\n", file); + } + } + e->destroy(e); + } + @endcode + * + * @param pattern file pattern to match + * @return the enumerator, NULL if not supported + */ +enumerator_t* enumerator_create_glob(const char *pattern); + /** * Create an enumerator over tokens of a string. * From 3784633fa5866bc53b47429753cb275f079c756d Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 4 Sep 2013 18:23:07 +0200 Subject: [PATCH 04/27] settings: Use glob enumerator to load included files --- src/libstrongswan/settings/settings.c | 45 ++++++++------------------- 1 file changed, 13 insertions(+), 32 deletions(-) diff --git a/src/libstrongswan/settings/settings.c b/src/libstrongswan/settings/settings.c index cf34fd1cf..acfa7c1ea 100644 --- a/src/libstrongswan/settings/settings.c +++ b/src/libstrongswan/settings/settings.c @@ -24,10 +24,6 @@ #include #include -#ifdef HAVE_GLOB_H -#include -#endif /* HAVE_GLOB_H */ - #include "settings.h" #include "collections/array.h" @@ -1284,8 +1280,9 @@ static bool parse_file(linked_list_t *contents, char *file, int level, static bool parse_files(linked_list_t *contents, char *file, int level, char *pattern, section_t *section) { + enumerator_t *enumerator; bool success = TRUE; - char pat[PATH_MAX]; + char pat[PATH_MAX], *expanded; if (level > MAX_INCLUSION_LEVEL) { @@ -1319,39 +1316,23 @@ static bool parse_files(linked_list_t *contents, char *file, int level, } free(dir); } -#ifdef HAVE_GLOB_H + enumerator = enumerator_create_glob(pat); + if (enumerator) { - int status; - glob_t buf; - - status = glob(pat, GLOB_ERR, NULL, &buf); - if (status == GLOB_NOMATCH) + while (enumerator->enumerate(enumerator, &expanded, NULL)) { - DBG1(DBG_LIB, "no files found matching '%s', ignored", pat); - } - else if (status != 0) - { - DBG1(DBG_LIB, "expanding file pattern '%s' failed", pat); - success = FALSE; - } - else - { - char **expanded; - for (expanded = buf.gl_pathv; *expanded != NULL; expanded++) + success &= parse_file(contents, expanded, level + 1, section); + if (!success) { - success &= parse_file(contents, *expanded, level + 1, section); - if (!success) - { - break; - } + break; } } - globfree(&buf); + enumerator->destroy(enumerator); + } + else + { /* if glob(3) is not available, try to load pattern directly */ + success = parse_file(contents, pat, level + 1, section); } -#else /* HAVE_GLOB_H */ - /* if glob(3) is not available, try to load pattern directly */ - success = parse_file(contents, pat, level + 1, section); -#endif /* HAVE_GLOB_H */ return success; } From 3cb8016f0e938e92ff680f8d6eef0c857dac5d75 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 7 Mar 2014 17:04:01 +0100 Subject: [PATCH 05/27] parser-helper: Add utility class for flex/bison based parsers --- src/libstrongswan/Android.mk | 3 +- src/libstrongswan/Makefile.am | 5 +- src/libstrongswan/utils/parser_helper.c | 230 ++++++++++++++++++++++++ src/libstrongswan/utils/parser_helper.h | 173 ++++++++++++++++++ 4 files changed, 408 insertions(+), 3 deletions(-) create mode 100644 src/libstrongswan/utils/parser_helper.c create mode 100644 src/libstrongswan/utils/parser_helper.h diff --git a/src/libstrongswan/Android.mk b/src/libstrongswan/Android.mk index 5f22bf114..ccff30bc0 100644 --- a/src/libstrongswan/Android.mk +++ b/src/libstrongswan/Android.mk @@ -37,7 +37,8 @@ threading/thread.c threading/thread_value.c \ threading/mutex.c threading/semaphore.c threading/rwlock.c threading/spinlock.c \ utils/utils.c utils/chunk.c utils/debug.c utils/enum.c utils/identification.c \ utils/lexparser.c utils/optionsfrom.c utils/capabilities.c utils/backtrace.c \ -utils/printf_hook/printf_hook_builtin.c utils/test.c utils/utils/strerror.c +utils/parser_helper.c utils/test.c utils/utils/strerror.c \ +utils/printf_hook/printf_hook_builtin.c # adding the plugin source files diff --git a/src/libstrongswan/Makefile.am b/src/libstrongswan/Makefile.am index d202b023d..e7f5ab05a 100644 --- a/src/libstrongswan/Makefile.am +++ b/src/libstrongswan/Makefile.am @@ -35,7 +35,7 @@ threading/thread.c threading/thread_value.c \ threading/mutex.c threading/semaphore.c threading/rwlock.c threading/spinlock.c \ utils/utils.c utils/chunk.c utils/debug.c utils/enum.c utils/identification.c \ utils/lexparser.c utils/optionsfrom.c utils/capabilities.c utils/backtrace.c \ -utils/test.c utils/utils/strerror.c +utils/parser_helper.c utils/test.c utils/utils/strerror.c if USE_DEV_HEADERS strongswan_includedir = ${dev_headers} @@ -82,7 +82,8 @@ utils/utils.h utils/chunk.h utils/debug.h utils/enum.h utils/identification.h \ utils/lexparser.h utils/optionsfrom.h utils/capabilities.h utils/backtrace.h \ utils/leak_detective.h utils/printf_hook/printf_hook.h \ utils/printf_hook/printf_hook_vstr.h utils/printf_hook/printf_hook_builtin.h \ -utils/test.h utils/integrity_checker.h utils/utils/strerror.h +utils/parser_helper.h utils/test.h utils/integrity_checker.h \ +utils/utils/strerror.h endif library.lo : $(top_builddir)/config.status diff --git a/src/libstrongswan/utils/parser_helper.c b/src/libstrongswan/utils/parser_helper.c new file mode 100644 index 000000000..fb549a572 --- /dev/null +++ b/src/libstrongswan/utils/parser_helper.c @@ -0,0 +1,230 @@ +/* + * Copyright (C) 2014 Tobias Brunner + * Hochschule fuer Technik Rapperswil + * + * 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. See . + * + * 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. + */ + +#include +#include + +#include "parser_helper.h" + +#include + +typedef struct private_parser_helper_t private_parser_helper_t; +typedef struct private_parser_helper_file_t private_parser_helper_file_t; + +struct private_parser_helper_t { + + /** + * Public interface. + */ + parser_helper_t public; + + /** + * Stack of included files, as private_parser_helper_file_t. + */ + array_t *files; + + /** + * Helper for parsing strings. + */ + bio_writer_t *writer; +}; + +struct private_parser_helper_file_t { + + /** + * File data. + */ + parser_helper_file_t public; + + /** + * Enumerator of paths matching the most recent inclusion pattern. + */ + enumerator_t *matches; +}; + +/** + * Destroy the given file data. + */ +static void parser_helper_file_destroy(private_parser_helper_file_t *this) +{ + if (this->public.file) + { + fclose(this->public.file); + } + free(this->public.name); + DESTROY_IF(this->matches); + free(this); +} + +METHOD(parser_helper_t, file_current, parser_helper_file_t*, + private_parser_helper_t *this) +{ + private_parser_helper_file_t *file; + + array_get(this->files, ARRAY_TAIL, &file); + if (file->public.name) + { + return &file->public; + } + return NULL; +} + +METHOD(parser_helper_t, file_next, parser_helper_file_t*, + private_parser_helper_t *this) +{ + private_parser_helper_file_t *file, *next; + char *name; + + array_get(this->files, ARRAY_TAIL, &file); + if (!file->matches) + { + array_remove(this->files, ARRAY_TAIL, NULL); + parser_helper_file_destroy(file); + /* continue with previous includes, if any */ + array_get(this->files, ARRAY_TAIL, &file); + } + if (file->matches) + { + while (file->matches->enumerate(file->matches, &name, NULL)) + { + INIT(next, + .public = { + .name = strdup(name), + .file = fopen(name, "r"), + }, + ); + + if (next->public.file) + { + array_insert(this->files, ARRAY_TAIL, next); + return &next->public; + } + PARSER_DBG1(&this->public, "unable to open '%s'", name); + parser_helper_file_destroy(next); + } + file->matches->destroy(file->matches); + file->matches = NULL; + } + return NULL; +} + +METHOD(parser_helper_t, file_include, void, + private_parser_helper_t *this, char *pattern) +{ + private_parser_helper_file_t *file; + char pat[PATH_MAX]; + + array_get(this->files, ARRAY_TAIL, &file); + if (!pattern || !*pattern) + { + PARSER_DBG1(&this->public, "no include pattern specified, ignored"); + file->matches = enumerator_create_empty(); + return; + } + + if (!file->public.name || pattern[0] == '/') + { /* absolute path */ + if (snprintf(pat, sizeof(pat), "%s", pattern) >= sizeof(pat)) + { + PARSER_DBG1(&this->public, "include pattern too long, ignored"); + file->matches = enumerator_create_empty(); + return; + } + } + else + { /* base relative paths to the directory of the current file */ + char *dir = path_dirname(file->public.name); + if (snprintf(pat, sizeof(pat), "%s/%s", dir, pattern) >= sizeof(pat)) + { + PARSER_DBG1(&this->public, "include pattern too long, ignored"); + free(dir); + file->matches = enumerator_create_empty(); + return; + } + free(dir); + } + + file->matches = enumerator_create_glob(pat); + if (!file->matches) + { /* if glob(3) is not available, try to load pattern directly */ + file->matches = enumerator_create_single(strdup(pat), free); + } +} + +METHOD(parser_helper_t, string_init, void, + private_parser_helper_t *this) +{ + chunk_t data; + + data = this->writer->extract_buf(this->writer); + chunk_free(&data); +} + +METHOD(parser_helper_t, string_add, void, + private_parser_helper_t *this, char *str) +{ + this->writer->write_data(this->writer, chunk_from_str(str)); +} + +METHOD(parser_helper_t, string_get, char*, + private_parser_helper_t *this) +{ + chunk_t data; + + this->writer->write_data(this->writer, chunk_from_chars('\0')); + data = this->writer->extract_buf(this->writer); + return data.ptr; +} + +METHOD(parser_helper_t, destroy, void, + private_parser_helper_t *this) +{ + array_destroy_function(this->files, (void*)parser_helper_file_destroy, NULL); + this->writer->destroy(this->writer); + free(this); +} + +/** + * Described in header + */ +parser_helper_t *parser_helper_create(void *context) +{ + private_parser_helper_t *this; + private_parser_helper_file_t *sentinel; + + INIT(this, + .public = { + .context = context, + .file_current = _file_current, + .file_include = _file_include, + .file_next = _file_next, + .string_init = _string_init, + .string_add = _string_add, + .string_get = _string_get, + .destroy = _destroy, + }, + .files = array_create(0, 0), + .writer = bio_writer_create(0), + ); + + INIT(sentinel, + .public = { + .name = NULL, + }, + ); + array_insert(this->files, ARRAY_TAIL, sentinel); + + return &this->public; +} diff --git a/src/libstrongswan/utils/parser_helper.h b/src/libstrongswan/utils/parser_helper.h new file mode 100644 index 000000000..741582d2c --- /dev/null +++ b/src/libstrongswan/utils/parser_helper.h @@ -0,0 +1,173 @@ +/* + * Copyright (C) 2014 Tobias Brunner + * Hochschule fuer Technik Rapperswil + * + * 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. See . + * + * 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. + */ + +/** + * @defgroup parser_helper parser_helper + * @{ @ingroup utils + */ + +#ifndef PARSER_HELPER_H_ +#define PARSER_HELPER_H_ + +#include +#include + +typedef struct parser_helper_t parser_helper_t; +typedef struct parser_helper_file_t parser_helper_file_t; + +/** + * Helper class for flex/bison based parsers. + * + * PREFIX equals whatever is configure with + * %option prefix resp. %name-prefix. + */ +struct parser_helper_t { + + /** + * A user defined parser context object. + */ + const void *context; + + /** + * Opaque object allocated by the lexer, should be set with: + * @code + * PREFIXlex_init_extra(helper, &helper->scanner). + * @endcode + */ + void *scanner; + + /** + * Function to determine the current line number (defined by the lexer). + * + * Basically, this should be assigned to PREFIXget_lineno. + * + * @param scanner the lexer + * @return current line number + */ + int (*get_lineno)(void *scanner); + + /** + * Get the current file. + * + * @return current file, or NULL + */ + parser_helper_file_t *(*file_current)(parser_helper_t *this); + + /** + * Resolves the given include pattern, relative to the location of the + * current file. + * + * Call file_next() to open the next file. + * + * @param pattern file pattern + */ + void (*file_include)(parser_helper_t *this, char *pattern); + + /** + * Get the next file to process. + * + * This will return NULL if all files matching the most recent pattern + * have been handled. If there are other patterns the next call will then + * return the next file matching the previous pattern. + * + * When hitting \<\\> first call + * @code + * PREFIXpop_buffer_state(yyscanner); + * @endcode + * then call this method to check if there are more files to include for + * the most recent call to file_include(), if so, call + * @code + * PREFIXset_in(file->file, helper->scanner); + * PREFIXpush_buffer_state(PREFIX_create_buffer(file->file, YY_BUF_SIZE, + * helper->scanner), helper->scanner); + * @endcode + * + * If there are no more files to process check + * YY_CURRENT_BUFFER and if it is FALSE call yyterminate(). + * + * @return next file to process, or NULL (see comment) + */ + parser_helper_file_t *(*file_next)(parser_helper_t *this); + + /** + * Start parsing a string, discards any currently stored data. + */ + void (*string_init)(parser_helper_t *this); + + /** + * Append the given string. + * + * @param str string to append + */ + void (*string_add)(parser_helper_t *this, char *str); + + /** + * Extract the current string buffer as null-terminated string. Can only + * be called once per string. + * + * @return allocated string + */ + char *(*string_get)(parser_helper_t *this); + + /** + * Destroy this instance. + */ + void (*destroy)(parser_helper_t *this); +}; + +struct parser_helper_file_t { + + /** + * File name + */ + char *name; + + /** + * File stream + */ + FILE *file; +}; + +/** + * Log the given message either as error or warning + * + * @param level log level + * @param ctx current parser context + * @param fmt error message format + * @param ... additional arguments + */ +#define parser_helper_log(level, ctx, fmt, ...) ({ \ + parser_helper_file_t *_file = (ctx)->file_current(ctx); \ + int _line = (ctx)->get_lineno ? (ctx)->get_lineno((ctx)->scanner) : 0; \ + if (_file) {\ + DBG##level(DBG_CFG, "%s:%d: " fmt, _file->name, _line, ##__VA_ARGS__); \ + } else { \ + DBG##level(DBG_CFG, fmt, ##__VA_ARGS__); \ + } \ +}) + +#define PARSER_DBG1(ctx, fmt, ...) parser_helper_log(1, ctx, fmt, ##__VA_ARGS__) +#define PARSER_DBG2(ctx, fmt, ...) parser_helper_log(2, ctx, fmt, ##__VA_ARGS__) +#define PARSER_DBG3(ctx, fmt, ...) parser_helper_log(3, ctx, fmt, ##__VA_ARGS__) + +/** + * Create a parser helper object + * + * @param context user defined parser context + * @return parser helper + */ +parser_helper_t *parser_helper_create(void *context); + +#endif /** PARSER_HELPER_H_ @}*/ From f99d1f7ba55337e5f4fae96dfc97bd463ceea831 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 7 Mar 2014 17:13:31 +0100 Subject: [PATCH 06/27] settings: Extract section and key/value pair types and helper functions This allows us to use them in the upcoming parser. --- src/libstrongswan/Android.mk | 6 +- src/libstrongswan/Makefile.am | 10 +- src/libstrongswan/settings/settings_types.c | 150 ++++++++++++++++++++ src/libstrongswan/settings/settings_types.h | 134 +++++++++++++++++ 4 files changed, 294 insertions(+), 6 deletions(-) create mode 100644 src/libstrongswan/settings/settings_types.c create mode 100644 src/libstrongswan/settings/settings_types.h diff --git a/src/libstrongswan/Android.mk b/src/libstrongswan/Android.mk index ccff30bc0..9dfd264f0 100644 --- a/src/libstrongswan/Android.mk +++ b/src/libstrongswan/Android.mk @@ -32,9 +32,9 @@ networking/streams/stream_service.c networking/streams/stream_manager.c \ pen/pen.c plugins/plugin_loader.c plugins/plugin_feature.c processing/jobs/job.c \ processing/jobs/callback_job.c processing/processor.c processing/scheduler.c \ processing/watcher.c resolver/resolver_manager.c resolver/rr_set.c \ -selectors/traffic_selector.c settings/settings.c \ -threading/thread.c threading/thread_value.c \ -threading/mutex.c threading/semaphore.c threading/rwlock.c threading/spinlock.c \ +selectors/traffic_selector.c settings/settings.c settings/settings_types.c \ +threading/thread.c threading/thread_value.c threading/mutex.c \ +threading/semaphore.c threading/rwlock.c threading/spinlock.c \ utils/utils.c utils/chunk.c utils/debug.c utils/enum.c utils/identification.c \ utils/lexparser.c utils/optionsfrom.c utils/capabilities.c utils/backtrace.c \ utils/parser_helper.c utils/test.c utils/utils/strerror.c \ diff --git a/src/libstrongswan/Makefile.am b/src/libstrongswan/Makefile.am index e7f5ab05a..de2c9508b 100644 --- a/src/libstrongswan/Makefile.am +++ b/src/libstrongswan/Makefile.am @@ -30,13 +30,17 @@ networking/streams/stream_service.c networking/streams/stream_manager.c \ pen/pen.c plugins/plugin_loader.c plugins/plugin_feature.c processing/jobs/job.c \ processing/jobs/callback_job.c processing/processor.c processing/scheduler.c \ processing/watcher.c resolver/resolver_manager.c resolver/rr_set.c \ -selectors/traffic_selector.c settings/settings.c \ -threading/thread.c threading/thread_value.c \ -threading/mutex.c threading/semaphore.c threading/rwlock.c threading/spinlock.c \ +selectors/traffic_selector.c settings/settings.c settings/settings_types.c \ +threading/thread.c threading/thread_value.c threading/mutex.c \ +threading/semaphore.c threading/rwlock.c threading/spinlock.c \ utils/utils.c utils/chunk.c utils/debug.c utils/enum.c utils/identification.c \ utils/lexparser.c utils/optionsfrom.c utils/capabilities.c utils/backtrace.c \ utils/parser_helper.c utils/test.c utils/utils/strerror.c +# private header files +noinst_HEADERS = \ +settings/settings_types.h + if USE_DEV_HEADERS strongswan_includedir = ${dev_headers} nobase_strongswan_include_HEADERS = \ diff --git a/src/libstrongswan/settings/settings_types.c b/src/libstrongswan/settings/settings_types.c new file mode 100644 index 000000000..015fd3f98 --- /dev/null +++ b/src/libstrongswan/settings/settings_types.c @@ -0,0 +1,150 @@ +/* + * Copyright (C) 2010-2014 Tobias Brunner + * Hochschule fuer Technik Rapperswil + * + * 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. See . + * + * 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. + */ + +#include "settings_types.h" + +/* + * Described in header + */ +kv_t *settings_kv_create(char *key, char *value) +{ + kv_t *this; + + INIT(this, + .key = key, + .value = value, + ); + return this; +} + +/* + * Described in header + */ +void settings_kv_destroy(kv_t *this) +{ + free(this->key); + free(this->value); + free(this); +} + +/* + * Described in header + */ +section_t *settings_section_create(char *name) +{ + section_t *this; + + INIT(this, + .name = name, + ); + return this; +} + +/* + * Described in header + */ +void settings_section_destroy(section_t *this) +{ + array_destroy_function(this->sections, (void*)settings_section_destroy, NULL); + array_destroy_function(this->kv, (void*)settings_kv_destroy, NULL); + array_destroy(this->fallbacks); + free(this->name); + free(this); +} + +/* + * Described in header + */ +void settings_section_extend(section_t *base, section_t *extension) +{ + enumerator_t *enumerator; + section_t *sec; + kv_t *kv; + + enumerator = array_create_enumerator(extension->sections); + while (enumerator->enumerate(enumerator, (void**)&sec)) + { + section_t *found; + if (array_bsearch(base->sections, sec->name, settings_section_find, + &found) != -1) + { + settings_section_extend(found, sec); + } + else + { + array_remove_at(extension->sections, enumerator); + array_insert_create(&base->sections, ARRAY_TAIL, sec); + array_sort(base->sections, settings_section_sort, NULL); + } + } + enumerator->destroy(enumerator); + + enumerator = array_create_enumerator(extension->kv); + while (enumerator->enumerate(enumerator, (void**)&kv)) + { + kv_t *found; + if (array_bsearch(base->kv, kv->key, settings_kv_find, &found) != -1) + { + free(found->value); + found->value = kv->value; + kv->value = NULL; + } + else + { + array_remove_at(extension->kv, enumerator); + array_insert_create(&base->kv, ARRAY_TAIL, kv); + array_sort(base->kv, settings_kv_sort, NULL); + } + } + enumerator->destroy(enumerator); +} + +/* + * Described in header + */ +int settings_section_find(const void *a, const void *b) +{ + const char *key = a; + const section_t *item = b; + return strcmp(key, item->name); +} + +/* + * Described in header + */ +int settings_section_sort(const void *a, const void *b, void *user) +{ + const section_t *sa = a, *sb = b; + return strcmp(sa->name, sb->name); +} + +/* + * Described in header + */ +int settings_kv_find(const void *a, const void *b) +{ + const char *key = a; + const kv_t *item = b; + return strcmp(key, item->key); +} + +/* + * Described in header + */ +int settings_kv_sort(const void *a, const void *b, void *user) +{ + const kv_t *kva = a, *kvb = b; + return strcmp(kva->key, kvb->key); +} diff --git a/src/libstrongswan/settings/settings_types.h b/src/libstrongswan/settings/settings_types.h new file mode 100644 index 000000000..79e8dbf5a --- /dev/null +++ b/src/libstrongswan/settings/settings_types.h @@ -0,0 +1,134 @@ +/* + * Copyright (C) 2010-2014 Tobias Brunner + * Hochschule fuer Technik Rapperswil + * + * 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. See . + * + * 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. + */ + +/** + * Internal data types and functions shared between the parser and t. + * + * @defgroup settings_types settings_types + * @{ @ingroup settings + */ + +#ifndef SETTINGS_TYPES_H_ +#define SETTINGS_TYPES_H_ + +typedef struct kv_t kv_t; +typedef struct section_t section_t; + +#include "collections/array.h" + +/** + * Key/value pair. + */ +struct kv_t { + + /** + * Key string, relative, not the full name. + */ + char *key; + + /** + * Value as string. + */ + char *value; +}; + +/** + * Section containing subsections and key value pairs. + */ +struct section_t { + + /** + * Name of the section. + */ + char *name; + + /** + * Fallback sections, as section_t. + */ + array_t *fallbacks; + + /** + * Subsections, as section_t. + */ + array_t *sections; + + /** + * Key value pairs, as kv_t. + */ + array_t *kv; +}; + +/** + * Create a key/value pair. + * + * @param key key (gets adopted) + * @param value value (gets adopted) + * @return allocated key/value pair + */ +kv_t *settings_kv_create(char *key, char *value); + +/** + * Destroy a key/value pair. + * + * @param this key/value pair to destroy + */ +void settings_kv_destroy(kv_t *this); + +/** + * Create a section with the given name. + * + * @param name name (gets adopted) + * @return allocated section + */ +section_t *settings_section_create(char *name); + +/** + * Destroy a section. + * + * @param this section to destroy + */ +void settings_section_destroy(section_t *this); + +/** + * Extend the first section with the values and sub-sections of the second + * section, from where they are consequently moved/removed (but there might + * still remain some leftovers). + * + * @param base base section to extend + * @param extension section whose data is extracted + */ +void settings_section_extend(section_t *base, section_t *extension); + +/** + * Callback to find a section by name + */ +int settings_section_find(const void *a, const void *b); + +/** + * Callback to sort sections by name + */ +int settings_section_sort(const void *a, const void *b, void *user); + +/** + * Callback to find a key/value pair by key + */ +int settings_kv_find(const void *a, const void *b); + +/** + * Callback to sort kv pairs by key + */ +int settings_kv_sort(const void *a, const void *b, void *user); + +#endif /** SETTINGS_TYPES_H_ @}*/ From 1f669078ac781c1e9a01cebe15490db8edc8d21c Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 7 Mar 2014 17:21:19 +0100 Subject: [PATCH 07/27] settings: Add flex/bison based parser for strongswan.conf This parser features several improvements over the existing one. For instance, quoted strings (with escape sequences), unlimited includes, relaxed newline handling (e.g. at the end of files or before/after { and }), and the difference between empty and unset values (key = vs. key = ""). It also complains a lot more about invalid syntax. The current one accepts pretty odd stuff (like settings or sections without name) without any errors or warnings. --- src/libstrongswan/Android.mk | 1 + src/libstrongswan/Makefile.am | 6 +- src/libstrongswan/settings/.gitignore | 3 + src/libstrongswan/settings/settings_lexer.l | 193 ++++++++++++ src/libstrongswan/settings/settings_parser.y | 299 +++++++++++++++++++ 5 files changed, 501 insertions(+), 1 deletion(-) create mode 100644 src/libstrongswan/settings/.gitignore create mode 100644 src/libstrongswan/settings/settings_lexer.l create mode 100644 src/libstrongswan/settings/settings_parser.y diff --git a/src/libstrongswan/Android.mk b/src/libstrongswan/Android.mk index 9dfd264f0..1840ad26e 100644 --- a/src/libstrongswan/Android.mk +++ b/src/libstrongswan/Android.mk @@ -33,6 +33,7 @@ pen/pen.c plugins/plugin_loader.c plugins/plugin_feature.c processing/jobs/job.c processing/jobs/callback_job.c processing/processor.c processing/scheduler.c \ processing/watcher.c resolver/resolver_manager.c resolver/rr_set.c \ selectors/traffic_selector.c settings/settings.c settings/settings_types.c \ +settings/settings_parser.c settings/settings_lexer.c \ threading/thread.c threading/thread_value.c threading/mutex.c \ threading/semaphore.c threading/rwlock.c threading/spinlock.c \ utils/utils.c utils/chunk.c utils/debug.c utils/enum.c utils/identification.c \ diff --git a/src/libstrongswan/Makefile.am b/src/libstrongswan/Makefile.am index de2c9508b..c4d1a5802 100644 --- a/src/libstrongswan/Makefile.am +++ b/src/libstrongswan/Makefile.am @@ -31,6 +31,7 @@ pen/pen.c plugins/plugin_loader.c plugins/plugin_feature.c processing/jobs/job.c processing/jobs/callback_job.c processing/processor.c processing/scheduler.c \ processing/watcher.c resolver/resolver_manager.c resolver/rr_set.c \ selectors/traffic_selector.c settings/settings.c settings/settings_types.c \ +settings/settings_parser.y settings/settings_lexer.l \ threading/thread.c threading/thread_value.c threading/mutex.c \ threading/semaphore.c threading/rwlock.c threading/spinlock.c \ utils/utils.c utils/chunk.c utils/debug.c utils/enum.c utils/identification.c \ @@ -107,6 +108,8 @@ AM_CFLAGS = \ AM_LDFLAGS = \ -no-undefined +AM_YFLAGS = -v -d + if USE_LEAK_DETECTIVE AM_CPPFLAGS += -DLEAK_DETECTIVE libstrongswan_la_SOURCES += utils/leak_detective.c @@ -148,7 +151,8 @@ Android.mk AndroidConfigLocal.h BUILT_SOURCES = \ $(srcdir)/asn1/oid.c $(srcdir)/asn1/oid.h \ -$(srcdir)/crypto/proposal/proposal_keywords_static.c +$(srcdir)/crypto/proposal/proposal_keywords_static.c \ +settings/settings_parser.h MAINTAINERCLEANFILES = \ $(srcdir)/asn1/oid.c $(srcdir)/asn1/oid.h \ diff --git a/src/libstrongswan/settings/.gitignore b/src/libstrongswan/settings/.gitignore new file mode 100644 index 000000000..a6e46817d --- /dev/null +++ b/src/libstrongswan/settings/.gitignore @@ -0,0 +1,3 @@ +settings_lexer.c +settings_parser.[ch] +settings_parser.output \ No newline at end of file diff --git a/src/libstrongswan/settings/settings_lexer.l b/src/libstrongswan/settings/settings_lexer.l new file mode 100644 index 000000000..f287761c1 --- /dev/null +++ b/src/libstrongswan/settings/settings_lexer.l @@ -0,0 +1,193 @@ +%{ +/* + * Copyright (C) 2014 Tobias Brunner + * Hochschule fuer Technik Rapperswil + * + * 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. See . + * + * 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. + */ + +#include "settings_parser.h" + +#include + +bool settings_parser_open_next_file(parser_helper_t *ctx); + +static void include_files(parser_helper_t *ctx); + +%} +%option debug +%option perf-report + +%option warn + +/* use start conditions stack */ +%option stack + +/* do not declare unneded functions */ +%option noinput noyywrap + +/* don't use global variables, and interact properly with bison */ +%option reentrant bison-bridge + +/* maintain the line number */ +%option yylineno + +/* don't generate a default rule */ +%option nodefault + +/* prefix function/variable declarations */ +%option prefix="settings_parser_" +/* don't change the name of the output file otherwise autotools has issues */ +%option outfile="lex.yy.c" + +/* type of our extra data */ +%option extra-type="parser_helper_t*" + +/* state used to scan include file patterns */ +%x inc +/* state used to scan quoted strings */ +%x str + +%% + +[\t ]*#[^\n]* /* eat comments */ +[\t ]+ /* eat whitespace */ +\n|#.*\n return NEWLINE; /* also eats comments at the end of a line */ + +"{" | +"}" | +"=" return yytext[0]; + +"include"[\t ]+/[^=] { + yyextra->string_init(yyextra); + yy_push_state(inc, yyscanner); +} + +"\"" { + yyextra->string_init(yyextra); + yy_push_state(str, yyscanner); +} + +[^#{}="\n\t ]+ { + yylval->s = strdup(yytext); + return NAME; +} + +{ + /* we allow all characters except #, } and spaces, they can be escaped */ + <> | + \n|#.*\n | + [\t ] { + if (*yytext && yytext[strlen(yytext) - 1] == '\n') + { /* put the newline back to fix the line numbers */ + unput('\n'); + yy_set_bol(0); + } + include_files(yyextra); + yy_pop_state(yyscanner); + } + "\"" { /* string include */ + yy_push_state(str, yyscanner); + } + \\ { + yyextra->string_add(yyextra, yytext); + } + \\["#} ] { + yyextra->string_add(yyextra, yytext+1); + } + [^"\\#}\n\t ]+ { + yyextra->string_add(yyextra, yytext); + } +} + +{ + "\"" | + <> | + \n | + \\ { + if (!streq(yytext, "\"")) + { + if (streq(yytext, "\n")) + { /* put the newline back to fix the line numbers */ + unput('\n'); + yy_set_bol(0); + } + PARSER_DBG1(yyextra, "unterminated string detected"); + } + if (yy_top_state(yyscanner) == inc) + { /* string include */ + include_files(yyextra); + yy_pop_state(yyscanner); + yy_pop_state(yyscanner); + } + else + { + yy_pop_state(yyscanner); + yylval->s = yyextra->string_get(yyextra); + return STRING; + } + } + \\n yyextra->string_add(yyextra, "\n"); + \\r yyextra->string_add(yyextra, "\r"); + \\t yyextra->string_add(yyextra, "\t"); + \\b yyextra->string_add(yyextra, "\b"); + \\f yyextra->string_add(yyextra, "\f"); + \\(.|\n) { + yyextra->string_add(yyextra, yytext+1); + } + [^\\\n"]+ { + yyextra->string_add(yyextra, yytext); + } +} + +<> { + settings_parser_pop_buffer_state(yyscanner); + if (!settings_parser_open_next_file(yyextra) && !YY_CURRENT_BUFFER) + { + yyterminate(); + } +} + +%% + +/** + * Open the next file, if any is queued and readable, otherwise returns FALSE. + */ +bool settings_parser_open_next_file(parser_helper_t *ctx) +{ + parser_helper_file_t *file; + + file = ctx->file_next(ctx); + if (!file) + { + return FALSE; + } + + settings_parser_set_in(file->file, ctx->scanner); + settings_parser_push_buffer_state( + settings_parser__create_buffer(file->file, YY_BUF_SIZE, + ctx->scanner), ctx->scanner); + return TRUE; +} + +/** + * Assumes that the file pattern to include is currently stored as string on + * the helper object. + */ +static void include_files(parser_helper_t *ctx) +{ + char *pattern = ctx->string_get(ctx); + + ctx->file_include(ctx, pattern); + free(pattern); + + settings_parser_open_next_file(ctx); +} diff --git a/src/libstrongswan/settings/settings_parser.y b/src/libstrongswan/settings/settings_parser.y new file mode 100644 index 000000000..3f0877ca6 --- /dev/null +++ b/src/libstrongswan/settings/settings_parser.y @@ -0,0 +1,299 @@ +%{ +/* + * Copyright (C) 2014 Tobias Brunner + * Hochschule fuer Technik Rapperswil + * + * 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. See . + * + * 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. + */ + +#define _GNU_SOURCE /* for asprintf() */ +#include + +#include "settings_parser.h" + +#include +#include +#include +#include + +#define YYDEBUG 1 + +/** + * Defined by the lexer + */ +int settings_parser_lex(YYSTYPE *lvalp, void *scanner); +int settings_parser_lex_init_extra(parser_helper_t *extra, void *scanner); +int settings_parser_lex_destroy(void *scanner); +int settings_parser_set_in(FILE *in, void *scanner); +void settings_parser_set_debug(int debug, void *scanner); +char *settings_parser_get_text(void *scanner); +int settings_parser_get_leng(void *scanner); +int settings_parser_get_lineno(void *scanner); +/* Custom functions in lexer */ +bool settings_parser_open_next_file(parser_helper_t *ctx); + +/** + * Forward declarations + */ +static void settings_parser_error(parser_helper_t *ctx, const char *s); +static section_t *push_section(parser_helper_t *ctx, char *name); +static section_t *pop_section(parser_helper_t *ctx); +static void add_section(parser_helper_t *ctx, section_t *section); +static void add_setting(parser_helper_t *ctx, kv_t *kv); + +/** + * Make sure the generated parser code passes the correct object to the lexer + */ +#define YYLEX_PARAM ctx->scanner + +%} +%debug + +/* generate verbose error messages */ +%error-verbose +/* generate a reentrant parser */ +%define api.pure +/* prefix function/variable declarations */ +%name-prefix "settings_parser_" + +/* interact properly with the reentrant lexer */ +%lex-param {void *scanner} +%parse-param {parser_helper_t *ctx} + +/* types for terminal symbols... (can't use the typedef'd types) */ +%union { + char *s; + struct section_t *sec; + struct kv_t *kv; +} +%token NAME STRING +%token NEWLINE + +/* ...and other symbols */ +%type value valuepart +%type section_start section +%type setting + +/* properly destroy string tokens that are strdup()ed on error */ +%destructor { free($$); } NAME STRING value valuepart +/* properly destroy parse results on error */ +%destructor { pop_section(ctx); settings_section_destroy($$); } section_start section +%destructor { settings_kv_destroy($$); } setting + +/* there are two shift/reduce conflicts because of the "NAME = NAME" and + * "NAME {" ambiguity, and the "NAME =" rule) */ +%expect 2 + +%% + +/** + * strongswan.conf grammar rules + */ +statements: + /* empty */ + | statements NEWLINE + | statements statement + ; + +statement: + section + { + add_section(ctx, $section); + } + | setting + { + add_setting(ctx, $setting); + } + ; + +section: + section_start statements '}' + { + pop_section(ctx); + $$ = $section_start; + } + ; + +section_start: + NAME '{' + { + $$ = push_section(ctx, $NAME); + } + | + NAME NEWLINE '{' + { + $$ = push_section(ctx, $NAME); + } + ; + +setting: + NAME '=' value + { + $$ = settings_kv_create($NAME, $value); + } + | + NAME '=' + { + $$ = settings_kv_create($NAME, NULL); + } + ; + +value: + valuepart + | value valuepart + { /* just put a single space between them, use strings for more */ + if (asprintf(&$$, "%s %s", $1, $2) < 0) + { + free($1); + free($2); + YYERROR; + } + free($1); + free($2); + } + ; + +valuepart: + NAME + | STRING + ; + +%% + +/** + * Referenced by the generated parser + */ +static void settings_parser_error(parser_helper_t *ctx, const char *s) +{ + char *text = settings_parser_get_text(ctx->scanner); + int len = settings_parser_get_leng(ctx->scanner); + + if (len && text[len-1] == '\n') + { /* cut off newline at the end to avoid muti-line log messages */ + len--; + } + PARSER_DBG1(ctx, "%s [%.*s]", s, len, text); +} + +/** + * Create a section and push it to the stack (the name is adopted), returns + * the created section + */ +static section_t *push_section(parser_helper_t *ctx, char *name) +{ + array_t *sections = (array_t*)ctx->context; + section_t *section; + + section = settings_section_create(name); + array_insert(sections, ARRAY_TAIL, section); + return section; +} + +/** + * Removes the top section of the stack and returns it + */ +static section_t *pop_section(parser_helper_t *ctx) +{ + array_t *sections = (array_t*)ctx->context; + section_t *section; + + array_remove(sections, ARRAY_TAIL, §ion); + return section; +} + +/** + * Adds the given section to the section on top of the stack + */ +static void add_section(parser_helper_t *ctx, section_t *section) +{ + array_t *sections = (array_t*)ctx->context; + section_t *parent, *existing; + + array_get(sections, ARRAY_TAIL, &parent); + if (array_bsearch(parent->sections, section->name, settings_section_find, + &existing) == -1) + { + array_insert_create(&parent->sections, ARRAY_TAIL, section); + array_sort(parent->sections, settings_section_sort, NULL); + } + else + { /* extend the existing section */ + settings_section_extend(existing, section); + settings_section_destroy(section); + } +} + +/** + * Adds the given key/value pair to the section on top of the stack + */ +static void add_setting(parser_helper_t *ctx, kv_t *kv) +{ + array_t *sections = (array_t*)ctx->context; + section_t *section; + kv_t *existing; + + array_get(sections, ARRAY_TAIL, §ion); + if (array_bsearch(section->kv, kv->key, settings_kv_find, &existing) == -1) + { + array_insert_create(§ion->kv, ARRAY_TAIL, kv); + array_sort(section->kv, settings_kv_sort, NULL); + } + else + { /* move value to existing object */ + free(existing->value); + existing->value = kv->value; + kv->value = NULL; + settings_kv_destroy(kv); + } +} + +/** + * Parse the given file and add all sections and key/value pairs to the + * given section. + */ +bool settings_parser_parse_file(section_t *root, char *name) +{ + parser_helper_t *helper; + array_t *sections = NULL; + bool success = FALSE; + + array_insert_create(§ions, ARRAY_TAIL, root); + helper = parser_helper_create(sections); + helper->get_lineno = settings_parser_get_lineno; + if (settings_parser_lex_init_extra(helper, &helper->scanner) != 0) + { + helper->destroy(helper); + array_destroy(sections); + return FALSE; + } + helper->file_include(helper, name); + if (!settings_parser_open_next_file(helper)) + { + DBG1(DBG_CFG, "failed to open config file '%s'", name); + } + else + { + if (getenv("DEBUG_SETTINGS_PARSER")) + { + yydebug = 1; + settings_parser_set_debug(1, helper->scanner); + } + success = yyparse(helper) == 0; + if (!success) + { + DBG1(DBG_CFG, "invalid config file '%s'", name); + } + } + array_destroy(sections); + settings_parser_lex_destroy(helper->scanner); + helper->destroy(helper); + return success; +} From 073d72cf49aeac99203cccc6edc7e9c9f0e1bce8 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 7 Mar 2014 17:51:36 +0100 Subject: [PATCH 08/27] settings: Optionally keep track of removed/replaced values --- src/libstrongswan/settings/settings_parser.y | 10 ++--- src/libstrongswan/settings/settings_types.c | 41 ++++++++++++++++---- src/libstrongswan/settings/settings_types.h | 10 +++-- 3 files changed, 45 insertions(+), 16 deletions(-) diff --git a/src/libstrongswan/settings/settings_parser.y b/src/libstrongswan/settings/settings_parser.y index 3f0877ca6..fda349797 100644 --- a/src/libstrongswan/settings/settings_parser.y +++ b/src/libstrongswan/settings/settings_parser.y @@ -85,8 +85,8 @@ static void add_setting(parser_helper_t *ctx, kv_t *kv); /* properly destroy string tokens that are strdup()ed on error */ %destructor { free($$); } NAME STRING value valuepart /* properly destroy parse results on error */ -%destructor { pop_section(ctx); settings_section_destroy($$); } section_start section -%destructor { settings_kv_destroy($$); } setting +%destructor { pop_section(ctx); settings_section_destroy($$, NULL); } section_start section +%destructor { settings_kv_destroy($$, NULL); } setting /* there are two shift/reduce conflicts because of the "NAME = NAME" and * "NAME {" ambiguity, and the "NAME =" rule) */ @@ -226,8 +226,8 @@ static void add_section(parser_helper_t *ctx, section_t *section) } else { /* extend the existing section */ - settings_section_extend(existing, section); - settings_section_destroy(section); + settings_section_extend(existing, section, NULL); + settings_section_destroy(section, NULL); } } @@ -251,7 +251,7 @@ static void add_setting(parser_helper_t *ctx, kv_t *kv) free(existing->value); existing->value = kv->value; kv->value = NULL; - settings_kv_destroy(kv); + settings_kv_destroy(kv, NULL); } } diff --git a/src/libstrongswan/settings/settings_types.c b/src/libstrongswan/settings/settings_types.c index 015fd3f98..4976a3b6c 100644 --- a/src/libstrongswan/settings/settings_types.c +++ b/src/libstrongswan/settings/settings_types.c @@ -32,10 +32,17 @@ kv_t *settings_kv_create(char *key, char *value) /* * Described in header */ -void settings_kv_destroy(kv_t *this) +void settings_kv_destroy(kv_t *this, array_t *contents) { free(this->key); - free(this->value); + if (contents) + { + array_insert(contents, ARRAY_TAIL, this->value); + } + else + { + free(this->value); + } free(this); } @@ -52,13 +59,23 @@ section_t *settings_section_create(char *name) return this; } +static void section_destroy(section_t *section, int idx, array_t *contents) +{ + settings_section_destroy(section, contents); +} + +static void kv_destroy(kv_t *kv, int idx, array_t *contents) +{ + settings_kv_destroy(kv, contents); +} + /* * Described in header */ -void settings_section_destroy(section_t *this) +void settings_section_destroy(section_t *this, array_t *contents) { - array_destroy_function(this->sections, (void*)settings_section_destroy, NULL); - array_destroy_function(this->kv, (void*)settings_kv_destroy, NULL); + array_destroy_function(this->sections, (void*)section_destroy, contents); + array_destroy_function(this->kv, (void*)kv_destroy, contents); array_destroy(this->fallbacks); free(this->name); free(this); @@ -67,7 +84,8 @@ void settings_section_destroy(section_t *this) /* * Described in header */ -void settings_section_extend(section_t *base, section_t *extension) +void settings_section_extend(section_t *base, section_t *extension, + array_t *contents) { enumerator_t *enumerator; section_t *sec; @@ -80,7 +98,7 @@ void settings_section_extend(section_t *base, section_t *extension) if (array_bsearch(base->sections, sec->name, settings_section_find, &found) != -1) { - settings_section_extend(found, sec); + settings_section_extend(found, sec, contents); } else { @@ -97,7 +115,14 @@ void settings_section_extend(section_t *base, section_t *extension) kv_t *found; if (array_bsearch(base->kv, kv->key, settings_kv_find, &found) != -1) { - free(found->value); + if (contents) + { + array_insert(contents, ARRAY_TAIL, found->value); + } + else + { + free(found->value); + } found->value = kv->value; kv->value = NULL; } diff --git a/src/libstrongswan/settings/settings_types.h b/src/libstrongswan/settings/settings_types.h index 79e8dbf5a..28e7fd6f5 100644 --- a/src/libstrongswan/settings/settings_types.h +++ b/src/libstrongswan/settings/settings_types.h @@ -83,8 +83,9 @@ kv_t *settings_kv_create(char *key, char *value); * Destroy a key/value pair. * * @param this key/value pair to destroy + * @param contents optional array to store the value in */ -void settings_kv_destroy(kv_t *this); +void settings_kv_destroy(kv_t *this, array_t *contents); /** * Create a section with the given name. @@ -98,8 +99,9 @@ section_t *settings_section_create(char *name); * Destroy a section. * * @param this section to destroy + * @param contents optional array to store values of removed key/value pairs */ -void settings_section_destroy(section_t *this); +void settings_section_destroy(section_t *this, array_t *contents); /** * Extend the first section with the values and sub-sections of the second @@ -108,8 +110,10 @@ void settings_section_destroy(section_t *this); * * @param base base section to extend * @param extension section whose data is extracted + * @param contents optional array to store replaced values in */ -void settings_section_extend(section_t *base, section_t *extension); +void settings_section_extend(section_t *base, section_t *extension, + array_t *contents); /** * Callback to find a section by name From 47a3ed979beef405bdf2627c5a1f8c5df37ce0ff Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 7 Mar 2014 18:20:28 +0100 Subject: [PATCH 09/27] settings: Use generated parser instead of our own --- src/libstrongswan/settings/settings.c | 619 +++----------------------- 1 file changed, 53 insertions(+), 566 deletions(-) diff --git a/src/libstrongswan/settings/settings.c b/src/libstrongswan/settings/settings.c index acfa7c1ea..25c5d6ea4 100644 --- a/src/libstrongswan/settings/settings.c +++ b/src/libstrongswan/settings/settings.c @@ -25,6 +25,7 @@ #include #include "settings.h" +#include "settings_types.h" #include "collections/array.h" #include "collections/hashtable.h" @@ -32,188 +33,71 @@ #include "threading/rwlock.h" #include "utils/debug.h" -#define MAX_INCLUSION_LEVEL 10 - typedef struct private_settings_t private_settings_t; -typedef struct section_t section_t; -typedef struct kv_t kv_t; /** - * private data of settings + * Parse function provided by the generated parser. + */ +bool settings_parser_parse_file(section_t *root, char *name); + +/** + * Private data of settings */ struct private_settings_t { /** - * public functions + * Public interface */ settings_t public; /** - * top level section + * Top level section */ section_t *top; /** - * contents of loaded files and in-memory settings (char*) + * Contents of replaced settings (char*) + * + * FIXME: This is required because the pointer returned by get_str() + * is not refcounted. Might cause ever increasing usage stats. */ - linked_list_t *contents; + array_t *contents; /** - * lock to safely access the settings + * Lock to safely access the settings */ rwlock_t *lock; }; -/** - * section containing subsections and key value pairs - */ -struct section_t { - - /** - * name of the section - */ - char *name; - - /** - * fallback sections, as section_t - */ - array_t *fallbacks; - - /** - * subsections, as section_t - */ - array_t *sections; - - /** - * key value pairs, as kv_t - */ - array_t *kv; -}; - -/** - * Key value pair - */ -struct kv_t { - - /** - * key string, relative - */ - char *key; - - /** - * value as string - */ - char *value; -}; - -/** - * create a key/value pair - */ -static kv_t *kv_create(char *key, char *value) +static void kv_destroy(kv_t *kv, int idx, array_t *contents) { - kv_t *this; - INIT(this, - .key = strdup(key), - .value = value, - ); - return this; -} - -/** - * destroy a key/value pair - */ -static void kv_destroy(kv_t *this) -{ - free(this->key); - free(this); -} - -/** - * create a section with the given name - */ -static section_t *section_create(char *name) -{ - section_t *this; - INIT(this, - .name = strdupnull(name), - ); - return this; -} - -/** - * destroy a section - */ -static void section_destroy(section_t *this) -{ - array_destroy_function(this->sections, (void*)section_destroy, NULL); - array_destroy_function(this->kv, (void*)kv_destroy, NULL); - array_destroy(this->fallbacks); - free(this->name); - free(this); + settings_kv_destroy(kv, contents); } /** * Purge contents of a section, returns if section can be safely removed. */ -static bool section_purge(section_t *this) +static bool section_purge(section_t *this, array_t *contents) { section_t *current; int i; - array_destroy_function(this->kv, (void*)kv_destroy, NULL); + array_destroy_function(this->kv, (void*)kv_destroy, contents); this->kv = NULL; /* we ensure sections used as fallback, or configured with fallbacks (or * having any such subsections) are not removed */ for (i = array_count(this->sections) - 1; i >= 0; i--) { array_get(this->sections, i, ¤t); - if (section_purge(current)) + if (section_purge(current, contents)) { array_remove(this->sections, i, NULL); - section_destroy(current); + settings_section_destroy(current, contents); } } return !this->fallbacks && !array_count(this->sections); } -/** - * callback to find a section by name - */ -static int section_find(const void *a, const void *b) -{ - const char *key = a; - const section_t *item = b; - return strcmp(key, item->name); -} - -/** - * callback to sort sections by name - */ -static int section_sort(const void *a, const void *b, void *user) -{ - const section_t *sa = a, *sb = b; - return strcmp(sa->name, sb->name); -} - -/** - * callback to find a kv pair by key - */ -static int kv_find(const void *a, const void *b) -{ - const char *key = a; - const kv_t *item = b; - return strcmp(key, item->key); -} - -/** - * callback to sort kv pairs by key - */ -static int kv_sort(const void *a, const void *b, void *user) -{ - const kv_t *kva = a, *kvb = b; - return strcmp(kva->key, kvb->key); -} - /** * Print a format key, but consume already processed arguments */ @@ -286,13 +170,14 @@ static section_t *find_section_buffered(section_t *section, { found = section; } - else if (array_bsearch(section->sections, buf, section_find, &found) == -1) + else if (array_bsearch(section->sections, buf, settings_section_find, + &found) == -1) { if (ensure) { - found = section_create(buf); + found = settings_section_create(strdup(buf)); array_insert_create(§ion->sections, ARRAY_TAIL, found); - array_sort(section->sections, section_sort, NULL); + array_sort(section->sections, settings_section_sort, NULL); } } if (found && pos) @@ -336,7 +221,7 @@ static void find_sections_buffered(section_t *section, char *start, char *key, } else { - array_bsearch(section->sections, buf, section_find, &found); + array_bsearch(section->sections, buf, settings_section_find, &found); } if (found) { @@ -497,14 +382,14 @@ static kv_t *find_value_buffered(section_t *section, char *start, char *key, { found = section; } - else if (array_bsearch(section->sections, buf, section_find, + else if (array_bsearch(section->sections, buf, settings_section_find, &found) == -1) { if (ensure) { - found = section_create(buf); + found = settings_section_create(strdup(buf)); array_insert_create(§ion->sections, ARRAY_TAIL, found); - array_sort(section->sections, section_sort, NULL); + array_sort(section->sections, settings_section_sort, NULL); } } if (found) @@ -528,13 +413,13 @@ static kv_t *find_value_buffered(section_t *section, char *start, char *key, { return NULL; } - if (array_bsearch(section->kv, buf, kv_find, &kv) == -1) + if (array_bsearch(section->kv, buf, settings_kv_find, &kv) == -1) { if (ensure) { - kv = kv_create(buf, NULL); + kv = settings_kv_create(strdup(buf), NULL); array_insert_create(§ion->kv, ARRAY_TAIL, kv); - array_sort(section->kv, kv_sort, NULL); + array_sort(section->kv, settings_kv_sort, NULL); } else if (section->fallbacks) { @@ -594,6 +479,10 @@ static void set_value(private_settings_t *this, section_t *section, { if (!value) { + if (kv->value) + { + array_insert(this->contents, ARRAY_TAIL, kv->value); + } kv->value = NULL; } else if (kv->value && (strlen(value) <= strlen(kv->value))) @@ -601,9 +490,12 @@ static void set_value(private_settings_t *this, section_t *section, strcpy(kv->value, value); } else - { /* otherwise clone the string and store it in the cache */ + { /* otherwise clone the string and cache the replaced one */ + if (kv->value) + { + array_insert(this->contents, ARRAY_TAIL, kv->value); + } kv->value = strdup(value); - this->contents->insert_last(this->contents, kv->value); } } this->lock->unlock(this->lock); @@ -985,412 +877,16 @@ METHOD(settings_t, add_fallback, void, va_end(args); } -/** - * parse text, truncate "skip" chars, delimited by term respecting brackets. - * - * Chars in "skip" are truncated at the beginning and the end of the resulting - * token. "term" contains a list of characters to read up to (first match), - * while "br" contains bracket counterparts found in "term" to skip. - */ -static char parse(char **text, char *skip, char *term, char *br, char **token) -{ - char *best = NULL; - char best_term = '\0'; - - /* skip leading chars */ - while (strchr(skip, **text)) - { - (*text)++; - if (!**text) - { - return 0; - } - } - /* mark begin of subtext */ - *token = *text; - while (*term) - { - char *pos = *text; - int level = 1; - - /* find terminator */ - while (*pos) - { - if (*pos == *term) - { - level--; - } - else if (br && *pos == *br) - { - level++; - } - if (level == 0) - { - if (best == NULL || best > pos) - { - best = pos; - best_term = *term; - } - break; - } - pos++; - } - /* try next terminator */ - term++; - if (br) - { - br++; - } - } - if (best) - { - /* update input */ - *text = best; - /* null trailing bytes */ - do - { - *best = '\0'; - best--; - } - while (best >= *token && strchr(skip, *best)); - /* return found terminator */ - return best_term; - } - return 0; -} - -/** - * Check if "text" starts with "pattern". - * Characters in "skip" are skipped first. If found, TRUE is returned and "text" - * is modified to point to the character right after "pattern". - */ -static bool starts_with(char **text, char *skip, char *pattern) -{ - char *pos = *text; - int len = strlen(pattern); - while (strchr(skip, *pos)) - { - pos++; - if (!*pos) - { - return FALSE; - } - } - if (strlen(pos) < len || !strneq(pos, pattern, len)) - { - return FALSE; - } - *text = pos + len; - return TRUE; -} - -/** - * Check if what follows in "text" is an include statement. - * If this function returns TRUE, "text" will point to the character right after - * the include pattern, which is returned in "pattern". - */ -static bool parse_include(char **text, char **pattern) -{ - char *pos = *text; - if (!starts_with(&pos, "\n\t ", "include")) - { - return FALSE; - } - if (starts_with(&pos, "\t ", "=")) - { /* ignore "include = value" */ - return FALSE; - } - *text = pos; - return parse(text, "\t ", "\n", NULL, pattern) != 0; -} - -/** - * Forward declaration. - */ -static bool parse_files(linked_list_t *contents, char *file, int level, - char *pattern, section_t *section); - -/** - * Parse a section - */ -static bool parse_section(linked_list_t *contents, char *file, int level, - char **text, section_t *section) -{ - bool finished = FALSE; - char *key, *value, *inner; - - while (!finished) - { - if (parse_include(text, &value)) - { - if (!parse_files(contents, file, level, value, section)) - { - DBG1(DBG_LIB, "failed to include '%s'", value); - return FALSE; - } - continue; - } - switch (parse(text, "\t\n ", "{=#", NULL, &key)) - { - case '{': - if (parse(text, "\t ", "}", "{", &inner)) - { - section_t *sub; - if (!strlen(key)) - { - DBG1(DBG_LIB, "skipping section without name in '%s'", - section->name); - continue; - } - if (array_bsearch(section->sections, key, section_find, - &sub) == -1) - { - sub = section_create(key); - if (parse_section(contents, file, level, &inner, sub)) - { - array_insert_create(§ion->sections, ARRAY_TAIL, - sub); - array_sort(section->sections, section_sort, NULL); - continue; - } - section_destroy(sub); - } - else - { /* extend the existing section */ - if (parse_section(contents, file, level, &inner, sub)) - { - continue; - } - } - DBG1(DBG_LIB, "parsing subsection '%s' failed", key); - break; - } - DBG1(DBG_LIB, "matching '}' not found near %s", *text); - break; - case '=': - if (parse(text, "\t ", "\n", NULL, &value)) - { - kv_t *kv; - if (!strlen(key)) - { - DBG1(DBG_LIB, "skipping value without key in '%s'", - section->name); - continue; - } - if (array_bsearch(section->kv, key, kv_find, &kv) == -1) - { - kv = kv_create(key, value); - array_insert_create(§ion->kv, ARRAY_TAIL, kv); - array_sort(section->kv, kv_sort, NULL); - } - else - { /* replace with the most recently read value */ - kv->value = value; - } - continue; - } - DBG1(DBG_LIB, "parsing value failed near %s", *text); - break; - case '#': - parse(text, "", "\n", NULL, &value); - continue; - default: - finished = TRUE; - continue; - } - return FALSE; - } - return TRUE; -} - -/** - * Parse a file and add the settings to the given section. - */ -static bool parse_file(linked_list_t *contents, char *file, int level, - section_t *section) -{ - bool success; - char *text, *pos; - struct stat st; - FILE *fd; - int len; - - DBG2(DBG_LIB, "loading config file '%s'", file); - if (stat(file, &st) == -1) - { - if (errno == ENOENT) - { -#ifdef STRONGSWAN_CONF - if (streq(file, STRONGSWAN_CONF)) - { - DBG2(DBG_LIB, "'%s' does not exist, ignored", file); - } - else -#endif - { - DBG1(DBG_LIB, "'%s' does not exist, ignored", file); - } - return TRUE; - } - DBG1(DBG_LIB, "failed to stat '%s': %s", file, strerror(errno)); - return FALSE; - } - else if (!S_ISREG(st.st_mode)) - { - DBG1(DBG_LIB, "'%s' is not a regular file", file); - return FALSE; - } - fd = fopen(file, "r"); - if (fd == NULL) - { - DBG1(DBG_LIB, "'%s' is not readable", file); - return FALSE; - } - fseek(fd, 0, SEEK_END); - len = ftell(fd); - rewind(fd); - text = malloc(len + 2); - text[len] = text[len + 1] = '\0'; - if (fread(text, 1, len, fd) != len) - { - free(text); - fclose(fd); - return FALSE; - } - fclose(fd); - - pos = text; - success = parse_section(contents, file, level, &pos, section); - if (!success) - { - free(text); - } - else - { - contents->insert_last(contents, text); - } - return success; -} - -/** - * Load the files matching "pattern", which is resolved with glob(3), if - * available. - * If the pattern is relative, the directory of "file" is used as base. - */ -static bool parse_files(linked_list_t *contents, char *file, int level, - char *pattern, section_t *section) -{ - enumerator_t *enumerator; - bool success = TRUE; - char pat[PATH_MAX], *expanded; - - if (level > MAX_INCLUSION_LEVEL) - { - DBG1(DBG_LIB, "maximum level of %d includes reached, ignored", - MAX_INCLUSION_LEVEL); - return TRUE; - } - - if (!strlen(pattern)) - { - DBG1(DBG_LIB, "empty include pattern, ignored"); - return TRUE; - } - - if (!file || pattern[0] == '/') - { /* absolute path */ - if (snprintf(pat, sizeof(pat), "%s", pattern) >= sizeof(pat)) - { - DBG1(DBG_LIB, "include pattern too long, ignored"); - return TRUE; - } - } - else - { /* base relative paths to the directory of the current file */ - char *dir = path_dirname(file); - if (snprintf(pat, sizeof(pat), "%s/%s", dir, pattern) >= sizeof(pat)) - { - DBG1(DBG_LIB, "include pattern too long, ignored"); - free(dir); - return TRUE; - } - free(dir); - } - enumerator = enumerator_create_glob(pat); - if (enumerator) - { - while (enumerator->enumerate(enumerator, &expanded, NULL)) - { - success &= parse_file(contents, expanded, level + 1, section); - if (!success) - { - break; - } - } - enumerator->destroy(enumerator); - } - else - { /* if glob(3) is not available, try to load pattern directly */ - success = parse_file(contents, pat, level + 1, section); - } - return success; -} - -/** - * Recursivly extends "base" with "extension". - */ -static void section_extend(section_t *base, section_t *extension) -{ - enumerator_t *enumerator; - section_t *sec; - kv_t *kv; - - enumerator = array_create_enumerator(extension->sections); - while (enumerator->enumerate(enumerator, (void**)&sec)) - { - section_t *found; - if (array_bsearch(base->sections, sec->name, section_find, - &found) != -1) - { - section_extend(found, sec); - } - else - { - array_remove_at(extension->sections, enumerator); - array_insert_create(&base->sections, ARRAY_TAIL, sec); - array_sort(base->sections, section_sort, NULL); - } - } - enumerator->destroy(enumerator); - - enumerator = array_create_enumerator(extension->kv); - while (enumerator->enumerate(enumerator, (void**)&kv)) - { - kv_t *found; - if (array_bsearch(base->kv, kv->key, kv_find, &found) != -1) - { - found->value = kv->value; - } - else - { - array_remove_at(extension->kv, enumerator); - array_insert_create(&base->kv, ARRAY_TAIL, kv); - array_sort(base->kv, kv_sort, NULL); - } - } - enumerator->destroy(enumerator); -} - /** * Load settings from files matching the given file pattern. * All sections and values are added relative to "parent". * All files (even included ones) have to be loaded successfully. + * If merge is FALSE the contents of parent are replaced with the parsed + * contents, otherwise they are merged together. */ static bool load_files_internal(private_settings_t *this, section_t *parent, char *pattern, bool merge) { - char *text; - linked_list_t *contents; section_t *section; if (pattern == NULL) @@ -1402,32 +898,23 @@ static bool load_files_internal(private_settings_t *this, section_t *parent, #endif } - contents = linked_list_create(); - section = section_create(NULL); - - if (!parse_files(contents, NULL, 0, pattern, section)) + section = settings_section_create(NULL); + if (!settings_parser_parse_file(section, pattern)) { - contents->destroy_function(contents, (void*)free); - section_destroy(section); + settings_section_destroy(section, NULL); return FALSE; } this->lock->write_lock(this->lock); if (!merge) { - section_purge(parent); + section_purge(parent, this->contents); } /* extend parent section */ - section_extend(parent, section); - /* move contents of loaded files to main store */ - while (contents->remove_first(contents, (void**)&text) == SUCCESS) - { - this->contents->insert_last(this->contents, text); - } + settings_section_extend(parent, section, this->contents); this->lock->unlock(this->lock); - section_destroy(section); - contents->destroy(contents); + settings_section_destroy(section, NULL); return TRUE; } @@ -1457,8 +944,8 @@ METHOD(settings_t, load_files_section, bool, METHOD(settings_t, destroy, void, private_settings_t *this) { - section_destroy(this->top); - this->contents->destroy_function(this->contents, (void*)free); + settings_section_destroy(this->top, NULL); + array_destroy_function(this->contents, (void*)free, NULL); this->lock->destroy(this->lock); free(this); } @@ -1490,8 +977,8 @@ settings_t *settings_create(char *file) .load_files_section = _load_files_section, .destroy = _destroy, }, - .top = section_create(NULL), - .contents = linked_list_create(), + .top = settings_section_create(NULL), + .contents = array_create(0, 0), .lock = rwlock_create(RWLOCK_TYPE_DEFAULT), ); From 3855dc01ecfd7c483ba2cd339fa92f4e81e67f77 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Mon, 10 Mar 2014 10:53:52 +0100 Subject: [PATCH 10/27] settings: Don't enumerate key/value pairs with NULL value --- src/libstrongswan/settings/settings.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstrongswan/settings/settings.c b/src/libstrongswan/settings/settings.c index 25c5d6ea4..b36d1dfcc 100644 --- a/src/libstrongswan/settings/settings.c +++ b/src/libstrongswan/settings/settings.c @@ -817,7 +817,7 @@ static bool kv_filter(hashtable_t *seen, kv_t **in, char **key, void *none, char **value) { *key = (*in)->key; - if (seen->get(seen, *key)) + if (seen->get(seen, *key) || !(*in)->value) { return FALSE; } From 2fe04fb312b008d501037b615e3215b0006c499e Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Mon, 10 Mar 2014 14:50:43 +0100 Subject: [PATCH 11/27] unit-tests: Update settings tests to match new parser Empty settings are now ignored, strings are supported, newlines are handled properly (e.g. at the end of files) etc. --- .../tests/suites/test_settings.c | 191 ++++++++++++------ 1 file changed, 128 insertions(+), 63 deletions(-) diff --git a/src/libstrongswan/tests/suites/test_settings.c b/src/libstrongswan/tests/suites/test_settings.c index 8a08e360c..b5bd2c12e 100644 --- a/src/libstrongswan/tests/suites/test_settings.c +++ b/src/libstrongswan/tests/suites/test_settings.c @@ -39,6 +39,7 @@ START_SETUP(setup_base_config) " # this gets overridden below\n" " key2 = val2\n" " none = \n" + " empty = \"\"\n" " sub1 {\n" " key = value\n" " key2 = value2\n" @@ -51,7 +52,8 @@ START_SETUP(setup_base_config) " sub% {\n" " id = %any\n" " }\n" - " key2 = with spaces\n" + " key2 = with space\n" + " key3 = \"string with\\nnewline\"\n" "}\n" "out = side\n" "other {\n" @@ -79,7 +81,9 @@ START_TEST(test_get_str) verify_string("val1", "main.key1"); verify_string("val1", "main..key1"); verify_string("val1", ".main.key1"); - verify_string("with spaces", "main.key2"); + verify_string("", "main.empty"); + verify_string("with space", "main.key2"); + verify_string("string with\nnewline", "main.key3"); verify_string("value", "main.sub1.key"); verify_string("value2", "main.sub1.key2"); verify_string("bar", "main.sub1.subsub.foo"); @@ -88,10 +92,8 @@ START_TEST(test_get_str) verify_string("side", "out"); verify_string("other val", "other.key1"); - /* FIXME: should this rather be undefined i.e. return the default value? */ - verify_string("", "main.none"); - - verify_null("main.key3"); + verify_null("main.none"); + verify_null("main.key4"); verify_null("other.sub"); } END_TEST @@ -125,16 +127,26 @@ START_TEST(test_get_str_printf) * probably document it at least */ verify_null("main.%s%u.key%d", "sub", 1, 2); - verify_null("%s.%s%d", "main", "key", 3); + verify_null("%s.%s%d", "main", "key", 4); } END_TEST START_TEST(test_set_str) { + char *val; + + val = settings->get_str(settings, "main.key1", NULL); + ck_assert_str_eq("val1", val); settings->set_str(settings, "main.key1", "val"); verify_string("val", "main.key1"); + /* since it is shorter the pointer we got above points to the new value */ + ck_assert_str_eq("val", val); + settings->set_str(settings, "main.key1", "longer value"); verify_string("longer value", "main.key1"); + /* here it gets replaced but the pointer is still valid */ + ck_assert_str_eq("val", val); + settings->set_str(settings, "main", "main val"); verify_string("main val", "main"); settings->set_str(settings, "main.sub1.new", "added"); @@ -183,6 +195,7 @@ START_SETUP(setup_bool_config) " key7 = disabled\n" " key8 = 0\n" " key9 = 5\n" + " empty = \"\"\n" " none = \n" " foo = bar\n" "}")); @@ -203,6 +216,8 @@ START_TEST(test_get_bool) verify_bool(FALSE, TRUE, "main.key7"); verify_bool(FALSE, TRUE, "main.key8"); + verify_bool(FALSE, FALSE, "main.empty"); + verify_bool(TRUE, TRUE, "main.empty"); verify_bool(FALSE, FALSE, "main.none"); verify_bool(TRUE, TRUE, "main.none"); verify_bool(FALSE, FALSE, "main.foo"); @@ -240,6 +255,7 @@ START_SETUP(setup_int_config) " # gets cut off\n" " key2 = 5.5\n" " key3 = -42\n" + " empty = \"\"\n" " none = \n" " foo1 = bar\n" " foo2 = bar13\n" @@ -257,8 +273,10 @@ START_TEST(test_get_int) verify_int(5, 0, "main.key2"); verify_int(-42, 0, "main.key3"); + verify_int(0, 11, "main.empty"); + verify_int(11, 11, "main.none"); + /* FIXME: do we want this behavior? */ - verify_int(0, 11, "main.none"); verify_int(0, 11, "main.foo1"); verify_int(0, 11, "main.foo2"); verify_int(13, 11, "main.foo3"); @@ -291,6 +309,7 @@ START_SETUP(setup_double_config) " key2 = 5.5\n" " key3 = -42\n" " key4 = -42.5\n" + " empty = \"\"\n" " none = \n" " foo1 = bar\n" " foo2 = bar13.5\n" @@ -309,8 +328,10 @@ START_TEST(test_get_double) verify_double(-42, 0, "main.key3"); verify_double(-42.5, 0, "main.key4"); + verify_double(0, 11.5, "main.empty"); + verify_double(11.5, 11.5, "main.none"); + /* FIXME: do we want this behavior? */ - verify_double(0, 11.5, "main.none"); verify_double(0, 11.5, "main.foo1"); verify_double(0, 11.5, "main.foo2"); verify_double(13.5, 11.5, "main.foo3"); @@ -345,6 +366,7 @@ START_SETUP(setup_time_config) " key2 = 5m\n" " key3 = 5h\n" " key4 = 5d\n" + " empty = \"\"\n" " none = \n" " foo1 = bar\n" " foo2 = bar13\n" @@ -363,8 +385,10 @@ START_TEST(test_get_time) verify_time(18000, 0, "main.key3"); verify_time(432000, 0, "main.key4"); + verify_time(0, 11, "main.empty"); + verify_time(11, 11, "main.none"); + /* FIXME: do we want this behavior? */ - verify_time(0, 11, "main.none"); verify_time(0, 11, "main.foo1"); verify_time(0, 11, "main.foo2"); verify_time(13, 11, "main.foo3"); @@ -494,8 +518,8 @@ START_TEST(test_key_value_enumerator) { linked_list_t *keys, *values; - keys = linked_list_create_with_items("key1", "key2", "none", NULL); - values = linked_list_create_with_items("val1", "with spaces", "", NULL); + keys = linked_list_create_with_items("key1", "key2", "key3", "empty", NULL); + values = linked_list_create_with_items("val1", "with space", "string with\nnewline", "", NULL); verify_key_values(keys, values, "main"); keys = linked_list_create_with_items("key", "key2", "subsub", NULL); @@ -563,13 +587,14 @@ static void verify_include() { verify_string("n1", "main.key1"); verify_string("v2", "main.key2"); - verify_string("", "main.none"); verify_string("val", "main.sub1.key"); verify_string("v2", "main.sub1.key2"); verify_string("val", "main.sub1.sub1.key"); verify_string("value", "main.sub1.key3"); verify_string("value", "main.sub1.include"); verify_string("val3", "main.sub2.sub3"); + + verify_null("main.none"); } START_TEST(test_include) @@ -580,13 +605,13 @@ START_TEST(test_include) " key2 = val2\n" " none = x\n" " sub1 {\n" + " include this/does/not/exist.conf\n" " include = value\n" " key2 = value2\n" " include " include2 "\n" " }\n" "}\n" - "# currently there must be a newline after include statements\n" - "include " include1 "\n"); + "include " include1); create_settings(contents); verify_include(); @@ -641,11 +666,11 @@ START_TEST(test_load_files_section) ck_assert(settings->load_files_section(settings, include2, TRUE, "main.sub1")); verify_include(); - /* non existing files are no failure */ - ck_assert(settings->load_files_section(settings, include1".conf", TRUE, "")); + /* non existing files are a failure here */ + ck_assert(!settings->load_files_section(settings, include1".conf", TRUE, "")); verify_include(); - /* unreadable files are */ + /* unreadable files are too */ ck_assert(chunk_write(contents, include1".no", 0444, TRUE)); ck_assert(!settings->load_files_section(settings, include1".no", TRUE, "")); unlink(include1".no"); @@ -781,57 +806,86 @@ START_TEST(test_add_fallback_printf) } END_TEST -START_SETUP(setup_invalid_config) + +START_SETUP(setup_string_config) { create_settings(chunk_from_str( - "# section without name\n" - "{\n" - " key1 = val1\n" - "}\n" - "main {\n" - " key2 = val2\n" - " # value without key\n" - " = val3\n" - " key4 = val4\n" - " # key without value does not change it\n" - " key4\n" - " # subsection without name\n" - " {\n" - " key5 = val5\n" - " }\n" - " # empty include pattern\n" - " include\n" - " key6 = val6\n" - "}")); + "string = \" with accurate\twhitespace\"\n" + "special = \"all { special } characters # can be used.\"\n" + "unterminated = \"is fine\n" + "but = produces a warning\n" + "newlines = \"can either be encoded\\nor\\\n" + "escaped\"\n" + "quotes = \"\\\"and\\\" slashes \\\\ can \\\\ be\" # escaped too\n" + "multiple = \"strings\" are \"combined\"\n" + )); } END_SETUP -START_TEST(test_invalid) +START_TEST(test_strings) +{ + verify_string(" with accurate\twhitespace", "string"); + verify_string("all { special } characters # can be used.", "special"); + verify_string("is fine", "unterminated"); + verify_string("produces a warning", "but"); + verify_string("can either be encoded\nor\nescaped", "newlines"); + verify_string("\"and\" slashes \\ can \\ be", "quotes"); + verify_string("strings are combined", "multiple"); +} +END_TEST + +START_TEST(test_valid) { - linked_list_t *keys, *values; chunk_t contents; - verify_null("key1"); - verify_null(".key1"); - verify_null("%s.key1", ""); - verify_string("val2", "main.key2"); - verify_string("val4", "main.key4"); - verify_null("main..key5"); - verify_string("val6", "main.key6"); - - keys = linked_list_create_with_items("main", NULL); - verify_sections(keys, ""); - - keys = linked_list_create_with_items(NULL); - verify_sections(keys, "main"); - - keys = linked_list_create_with_items("key2", "key4", "key6", NULL); - values = linked_list_create_with_items("val2", "val4", "val6", NULL); - verify_key_values(keys, values, "main"); - - /* FIXME: we should probably fix this */ contents = chunk_from_str( - "requires = newline"); + "single = value"); + ck_assert(chunk_write(contents, path, 0022, TRUE)); + ck_assert(settings->load_files(settings, path, FALSE)); + verify_string("value", "single"); + + contents = chunk_from_str( + "singleline { single = value }"); + ck_assert(chunk_write(contents, path, 0022, TRUE)); + ck_assert(settings->load_files(settings, path, FALSE)); + verify_string("value", "singleline.single"); + + contents = chunk_from_str( + "singleline { sub { sub1 = val1 } single = value }"); + ck_assert(chunk_write(contents, path, 0022, TRUE)); + ck_assert(settings->load_files(settings, path, FALSE)); + verify_string("val1", "singleline.sub.sub1"); + + contents = chunk_from_str( + "newline\n { single = value }"); + ck_assert(chunk_write(contents, path, 0022, TRUE)); + ck_assert(settings->load_files(settings, path, FALSE)); + verify_string("value", "newline.single"); + + contents = chunk_from_str( + "section {\n" + " include # without pattern produces a warning, but is fine\n" + "}\n"); + ck_assert(chunk_write(contents, path, 0022, TRUE)); + ck_assert(settings->load_files(settings, path, FALSE)); +} +END_TEST + +START_TEST(test_invalid) +{ + chunk_t contents; + + contents = chunk_from_str( + "{\n" + " no = section name\n" + "}\n"); + ck_assert(chunk_write(contents, path, 0022, TRUE)); + ck_assert(!settings->load_files(settings, path, FALSE)); + + contents = chunk_from_str( + "no {\n" + " = key name\n" + "}\n"); ck_assert(chunk_write(contents, path, 0022, TRUE)); ck_assert(!settings->load_files(settings, path, FALSE)); @@ -842,7 +896,12 @@ START_TEST(test_invalid) ck_assert(!settings->load_files(settings, path, FALSE)); contents = chunk_from_str( - "singleline { not = valid }\n"); + "spaces in name {}"); + ck_assert(chunk_write(contents, path, 0022, TRUE)); + ck_assert(!settings->load_files(settings, path, FALSE)); + + contents = chunk_from_str( + "only = a single setting = per line"); ck_assert(chunk_write(contents, path, 0022, TRUE)); ck_assert(!settings->load_files(settings, path, FALSE)); } @@ -911,8 +970,14 @@ Suite *settings_suite_create() tcase_add_test(tc, test_add_fallback_printf); suite_add_tcase(s, tc); - tc = tcase_create("invalid data"); - tcase_add_checked_fixture(tc, setup_invalid_config, teardown_config); + tc = tcase_create("strings"); + tcase_add_checked_fixture(tc, setup_string_config, teardown_config); + tcase_add_test(tc, test_strings); + suite_add_tcase(s, tc); + + tc = tcase_create("valid/invalid data"); + tcase_add_checked_fixture(tc, setup_base_config, teardown_config); + tcase_add_test(tc, test_valid); tcase_add_test(tc, test_invalid); suite_add_tcase(s, tc); From 725c479f8bbc2d05790c4794e86c37afeda45bff Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 11 Mar 2014 10:58:03 +0100 Subject: [PATCH 12/27] settings: Add functions to add sections and key/value pairs to a section --- src/libstrongswan/settings/settings.c | 9 +- src/libstrongswan/settings/settings_parser.y | 28 +----- src/libstrongswan/settings/settings_types.c | 91 ++++++++++++-------- src/libstrongswan/settings/settings_types.h | 22 ++++- 4 files changed, 82 insertions(+), 68 deletions(-) diff --git a/src/libstrongswan/settings/settings.c b/src/libstrongswan/settings/settings.c index b36d1dfcc..3891578df 100644 --- a/src/libstrongswan/settings/settings.c +++ b/src/libstrongswan/settings/settings.c @@ -176,8 +176,7 @@ static section_t *find_section_buffered(section_t *section, if (ensure) { found = settings_section_create(strdup(buf)); - array_insert_create(§ion->sections, ARRAY_TAIL, found); - array_sort(section->sections, settings_section_sort, NULL); + settings_section_add(section, found, NULL); } } if (found && pos) @@ -388,8 +387,7 @@ static kv_t *find_value_buffered(section_t *section, char *start, char *key, if (ensure) { found = settings_section_create(strdup(buf)); - array_insert_create(§ion->sections, ARRAY_TAIL, found); - array_sort(section->sections, settings_section_sort, NULL); + settings_section_add(section, found, NULL); } } if (found) @@ -418,8 +416,7 @@ static kv_t *find_value_buffered(section_t *section, char *start, char *key, if (ensure) { kv = settings_kv_create(strdup(buf), NULL); - array_insert_create(§ion->kv, ARRAY_TAIL, kv); - array_sort(section->kv, settings_kv_sort, NULL); + settings_kv_add(section, kv, NULL); } else if (section->fallbacks) { diff --git a/src/libstrongswan/settings/settings_parser.y b/src/libstrongswan/settings/settings_parser.y index fda349797..b81003f29 100644 --- a/src/libstrongswan/settings/settings_parser.y +++ b/src/libstrongswan/settings/settings_parser.y @@ -215,20 +215,10 @@ static section_t *pop_section(parser_helper_t *ctx) static void add_section(parser_helper_t *ctx, section_t *section) { array_t *sections = (array_t*)ctx->context; - section_t *parent, *existing; + section_t *parent; array_get(sections, ARRAY_TAIL, &parent); - if (array_bsearch(parent->sections, section->name, settings_section_find, - &existing) == -1) - { - array_insert_create(&parent->sections, ARRAY_TAIL, section); - array_sort(parent->sections, settings_section_sort, NULL); - } - else - { /* extend the existing section */ - settings_section_extend(existing, section, NULL); - settings_section_destroy(section, NULL); - } + settings_section_add(parent, section, NULL); } /** @@ -238,21 +228,9 @@ static void add_setting(parser_helper_t *ctx, kv_t *kv) { array_t *sections = (array_t*)ctx->context; section_t *section; - kv_t *existing; array_get(sections, ARRAY_TAIL, §ion); - if (array_bsearch(section->kv, kv->key, settings_kv_find, &existing) == -1) - { - array_insert_create(§ion->kv, ARRAY_TAIL, kv); - array_sort(section->kv, settings_kv_sort, NULL); - } - else - { /* move value to existing object */ - free(existing->value); - existing->value = kv->value; - kv->value = NULL; - settings_kv_destroy(kv, NULL); - } + settings_kv_add(section, kv, NULL); } /** diff --git a/src/libstrongswan/settings/settings_types.c b/src/libstrongswan/settings/settings_types.c index 4976a3b6c..8171bbc3e 100644 --- a/src/libstrongswan/settings/settings_types.c +++ b/src/libstrongswan/settings/settings_types.c @@ -35,7 +35,7 @@ kv_t *settings_kv_create(char *key, char *value) void settings_kv_destroy(kv_t *this, array_t *contents) { free(this->key); - if (contents) + if (contents && this->value) { array_insert(contents, ARRAY_TAIL, this->value); } @@ -81,6 +81,55 @@ void settings_section_destroy(section_t *this, array_t *contents) free(this); } +/* + * Described in header + */ +void settings_kv_add(section_t *section, kv_t *kv, array_t *contents) +{ + kv_t *found; + + if (array_bsearch(section->kv, kv->key, settings_kv_find, &found) == -1) + { + array_insert_create(§ion->kv, ARRAY_TAIL, kv); + array_sort(section->kv, settings_kv_sort, NULL); + } + else + { + if (contents && found->value) + { + array_insert(contents, ARRAY_TAIL, found->value); + } + else + { + free(found->value); + } + found->value = kv->value; + kv->value = NULL; + settings_kv_destroy(kv, NULL); + } +} + +/* + * Described in header + */ +void settings_section_add(section_t *parent, section_t *section, + array_t *contents) +{ + section_t *found; + + if (array_bsearch(parent->sections, section->name, settings_section_find, + &found) == -1) + { + array_insert_create(&parent->sections, ARRAY_TAIL, section); + array_sort(parent->sections, settings_section_sort, NULL); + } + else + { + settings_section_extend(found, section, contents); + settings_section_destroy(section, contents); + } +} + /* * Described in header */ @@ -88,50 +137,22 @@ void settings_section_extend(section_t *base, section_t *extension, array_t *contents) { enumerator_t *enumerator; - section_t *sec; + section_t *section; kv_t *kv; enumerator = array_create_enumerator(extension->sections); - while (enumerator->enumerate(enumerator, (void**)&sec)) + while (enumerator->enumerate(enumerator, (void**)§ion)) { - section_t *found; - if (array_bsearch(base->sections, sec->name, settings_section_find, - &found) != -1) - { - settings_section_extend(found, sec, contents); - } - else - { - array_remove_at(extension->sections, enumerator); - array_insert_create(&base->sections, ARRAY_TAIL, sec); - array_sort(base->sections, settings_section_sort, NULL); - } + array_remove_at(extension->sections, enumerator); + settings_section_add(base, section, contents); } enumerator->destroy(enumerator); enumerator = array_create_enumerator(extension->kv); while (enumerator->enumerate(enumerator, (void**)&kv)) { - kv_t *found; - if (array_bsearch(base->kv, kv->key, settings_kv_find, &found) != -1) - { - if (contents) - { - array_insert(contents, ARRAY_TAIL, found->value); - } - else - { - free(found->value); - } - found->value = kv->value; - kv->value = NULL; - } - else - { - array_remove_at(extension->kv, enumerator); - array_insert_create(&base->kv, ARRAY_TAIL, kv); - array_sort(base->kv, settings_kv_sort, NULL); - } + array_remove_at(extension->kv, enumerator); + settings_kv_add(base, kv, contents); } enumerator->destroy(enumerator); } diff --git a/src/libstrongswan/settings/settings_types.h b/src/libstrongswan/settings/settings_types.h index 28e7fd6f5..49f75dc16 100644 --- a/src/libstrongswan/settings/settings_types.h +++ b/src/libstrongswan/settings/settings_types.h @@ -87,6 +87,15 @@ kv_t *settings_kv_create(char *key, char *value); */ void settings_kv_destroy(kv_t *this, array_t *contents); +/** + * Add the given key/value pair to the given section. + * + * @param section section to add pair to + * @param kv key/value pair to add (gets adopted) + * @param contents optional array to store replaced values in + */ +void settings_kv_add(section_t *section, kv_t *kv, array_t *contents); + /** * Create a section with the given name. * @@ -103,10 +112,19 @@ section_t *settings_section_create(char *name); */ void settings_section_destroy(section_t *this, array_t *contents); +/** + * Add the given section to the given parent section. + * + * @param parent section to add section to + * @param section section to add (gets adopted) + * @param contents optional array to store replaced values in + */ +void settings_section_add(section_t *parent, section_t *section, + array_t *contents); + /** * Extend the first section with the values and sub-sections of the second - * section, from where they are consequently moved/removed (but there might - * still remain some leftovers). + * section, from where they are consequently removed. * * @param base base section to extend * @param extension section whose data is extracted From 2fbbea55c592c8b3eb9f40295620455434d7f651 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 11 Mar 2014 11:08:15 +0100 Subject: [PATCH 13/27] settings: Don't overwrite values in-place This is not thread safe. If threads are reading from pointers to existing values they could get a partially updated invalid value. Refactored assignment to a separate function. --- src/libstrongswan/settings/settings.c | 21 +---------- src/libstrongswan/settings/settings_types.c | 35 ++++++++++++++----- src/libstrongswan/settings/settings_types.h | 9 +++++ .../tests/suites/test_settings.c | 23 ++++++++---- 4 files changed, 52 insertions(+), 36 deletions(-) diff --git a/src/libstrongswan/settings/settings.c b/src/libstrongswan/settings/settings.c index 3891578df..0acf9bf93 100644 --- a/src/libstrongswan/settings/settings.c +++ b/src/libstrongswan/settings/settings.c @@ -474,26 +474,7 @@ static void set_value(private_settings_t *this, section_t *section, TRUE); if (kv) { - if (!value) - { - if (kv->value) - { - array_insert(this->contents, ARRAY_TAIL, kv->value); - } - kv->value = NULL; - } - else if (kv->value && (strlen(value) <= strlen(kv->value))) - { /* overwrite in-place, if possible */ - strcpy(kv->value, value); - } - else - { /* otherwise clone the string and cache the replaced one */ - if (kv->value) - { - array_insert(this->contents, ARRAY_TAIL, kv->value); - } - kv->value = strdup(value); - } + settings_kv_set(kv, strdupnull(value), this->contents); } this->lock->unlock(this->lock); } diff --git a/src/libstrongswan/settings/settings_types.c b/src/libstrongswan/settings/settings_types.c index 8171bbc3e..50359108f 100644 --- a/src/libstrongswan/settings/settings_types.c +++ b/src/libstrongswan/settings/settings_types.c @@ -81,6 +81,31 @@ void settings_section_destroy(section_t *this, array_t *contents) free(this); } +/* + * Described in header + */ +void settings_kv_set(kv_t *kv, char *value, array_t *contents) +{ + if (value && kv->value && streq(value, kv->value)) + { /* no update required */ + free(value); + return; + } + + /* if the new value was shorter we could overwrite the existing one but that + * could lead to reads of partially updated values from other threads that + * have a pointer to the existing value, so we replace it anyway */ + if (kv->value && contents) + { + array_insert(contents, ARRAY_TAIL, kv->value); + } + else + { + free(kv->value); + } + kv->value = value; +} + /* * Described in header */ @@ -95,15 +120,7 @@ void settings_kv_add(section_t *section, kv_t *kv, array_t *contents) } else { - if (contents && found->value) - { - array_insert(contents, ARRAY_TAIL, found->value); - } - else - { - free(found->value); - } - found->value = kv->value; + settings_kv_set(found, kv->value, contents); kv->value = NULL; settings_kv_destroy(kv, NULL); } diff --git a/src/libstrongswan/settings/settings_types.h b/src/libstrongswan/settings/settings_types.h index 49f75dc16..5bc322f0a 100644 --- a/src/libstrongswan/settings/settings_types.h +++ b/src/libstrongswan/settings/settings_types.h @@ -87,6 +87,15 @@ kv_t *settings_kv_create(char *key, char *value); */ void settings_kv_destroy(kv_t *this, array_t *contents); +/** + * Set the value of the given key/value pair. + * + * @param kv key/value pair + * @param value new value (gets adopted), may be NULL + * @param contents optional array to store replaced values in + */ +void settings_kv_set(kv_t *kv, char *value, array_t *contents); + /** * Add the given key/value pair to the given section. * diff --git a/src/libstrongswan/tests/suites/test_settings.c b/src/libstrongswan/tests/suites/test_settings.c index b5bd2c12e..b66ecd5e9 100644 --- a/src/libstrongswan/tests/suites/test_settings.c +++ b/src/libstrongswan/tests/suites/test_settings.c @@ -133,19 +133,28 @@ END_TEST START_TEST(test_set_str) { - char *val; + char *val1, *val2; - val = settings->get_str(settings, "main.key1", NULL); - ck_assert_str_eq("val1", val); + val1 = settings->get_str(settings, "main.key1", NULL); + ck_assert_str_eq("val1", val1); settings->set_str(settings, "main.key1", "val"); verify_string("val", "main.key1"); - /* since it is shorter the pointer we got above points to the new value */ - ck_assert_str_eq("val", val); + /* the pointer we got before is still valid */ + ck_assert_str_eq("val1", val1); + val2 = settings->get_str(settings, "main.key1", NULL); + ck_assert_str_eq("val", val2); settings->set_str(settings, "main.key1", "longer value"); verify_string("longer value", "main.key1"); - /* here it gets replaced but the pointer is still valid */ - ck_assert_str_eq("val", val); + /* the pointers we got before are still valid */ + ck_assert_str_eq("val1", val1); + ck_assert_str_eq("val", val2); + + val1 = settings->get_str(settings, "main.key1", NULL); + settings->set_str(settings, "main.key1", "longer value"); + val2 = settings->get_str(settings, "main.key1", NULL); + /* setting the same string should should get us the same pointer */ + ck_assert(val1 == val2); settings->set_str(settings, "main", "main val"); verify_string("main val", "main"); From f5dd274ab8aab6255b34f1f580a79a99dbdb2fa1 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 11 Mar 2014 12:33:43 +0100 Subject: [PATCH 14/27] settings: Maintain order of sections and settings while enumerating --- src/libstrongswan/settings/settings.c | 16 ++-- src/libstrongswan/settings/settings_types.c | 18 +++- src/libstrongswan/settings/settings_types.h | 10 +++ .../tests/suites/test_settings.c | 83 ++++++------------- 4 files changed, 60 insertions(+), 67 deletions(-) diff --git a/src/libstrongswan/settings/settings.c b/src/libstrongswan/settings/settings.c index 0acf9bf93..c6ebe894a 100644 --- a/src/libstrongswan/settings/settings.c +++ b/src/libstrongswan/settings/settings.c @@ -80,18 +80,23 @@ static void kv_destroy(kv_t *kv, int idx, array_t *contents) static bool section_purge(section_t *this, array_t *contents) { section_t *current; - int i; + int i, idx; array_destroy_function(this->kv, (void*)kv_destroy, contents); this->kv = NULL; + array_destroy(this->kv_order); + this->kv_order = NULL; /* we ensure sections used as fallback, or configured with fallbacks (or * having any such subsections) are not removed */ - for (i = array_count(this->sections) - 1; i >= 0; i--) + for (i = array_count(this->sections_order) - 1; i >= 0; i--) { array_get(this->sections, i, ¤t); if (section_purge(current, contents)) { - array_remove(this->sections, i, NULL); + array_remove(this->sections_order, i, NULL); + idx = array_bsearch(this->sections, current->name, + settings_section_find, NULL); + array_remove(this->sections, idx, NULL); settings_section_destroy(current, contents); } } @@ -758,7 +763,8 @@ static bool section_filter(hashtable_t *seen, section_t **in, char **out) static enumerator_t *section_enumerator(section_t *section, enumerator_data_t *data) { - return enumerator_create_filter(array_create_enumerator(section->sections), + return enumerator_create_filter( + array_create_enumerator(section->sections_order), (void*)section_filter, data->seen, NULL); } @@ -809,7 +815,7 @@ static bool kv_filter(hashtable_t *seen, kv_t **in, char **key, */ static enumerator_t *kv_enumerator(section_t *section, enumerator_data_t *data) { - return enumerator_create_filter(array_create_enumerator(section->kv), + return enumerator_create_filter(array_create_enumerator(section->kv_order), (void*)kv_filter, data->seen, NULL); } diff --git a/src/libstrongswan/settings/settings_types.c b/src/libstrongswan/settings/settings_types.c index 50359108f..e22d95165 100644 --- a/src/libstrongswan/settings/settings_types.c +++ b/src/libstrongswan/settings/settings_types.c @@ -75,7 +75,9 @@ static void kv_destroy(kv_t *kv, int idx, array_t *contents) void settings_section_destroy(section_t *this, array_t *contents) { array_destroy_function(this->sections, (void*)section_destroy, contents); + array_destroy(this->sections_order); array_destroy_function(this->kv, (void*)kv_destroy, contents); + array_destroy(this->kv_order); array_destroy(this->fallbacks); free(this->name); free(this); @@ -117,6 +119,7 @@ void settings_kv_add(section_t *section, kv_t *kv, array_t *contents) { array_insert_create(§ion->kv, ARRAY_TAIL, kv); array_sort(section->kv, settings_kv_sort, NULL); + array_insert_create(§ion->kv_order, ARRAY_TAIL, kv); } else { @@ -139,6 +142,7 @@ void settings_section_add(section_t *parent, section_t *section, { array_insert_create(&parent->sections, ARRAY_TAIL, section); array_sort(parent->sections, settings_section_sort, NULL); + array_insert_create(&parent->sections_order, ARRAY_TAIL, section); } else { @@ -156,19 +160,25 @@ void settings_section_extend(section_t *base, section_t *extension, enumerator_t *enumerator; section_t *section; kv_t *kv; + int idx; - enumerator = array_create_enumerator(extension->sections); + enumerator = array_create_enumerator(extension->sections_order); while (enumerator->enumerate(enumerator, (void**)§ion)) { - array_remove_at(extension->sections, enumerator); + idx = array_bsearch(extension->sections, section->name, + settings_section_find, NULL); + array_remove(extension->sections, idx, NULL); + array_remove_at(extension->sections_order, enumerator); settings_section_add(base, section, contents); } enumerator->destroy(enumerator); - enumerator = array_create_enumerator(extension->kv); + enumerator = array_create_enumerator(extension->kv_order); while (enumerator->enumerate(enumerator, (void**)&kv)) { - array_remove_at(extension->kv, enumerator); + idx = array_bsearch(extension->kv, kv->key, settings_kv_find, NULL); + array_remove(extension->kv, idx, NULL); + array_remove_at(extension->kv_order, enumerator); settings_kv_add(base, kv, contents); } enumerator->destroy(enumerator); diff --git a/src/libstrongswan/settings/settings_types.h b/src/libstrongswan/settings/settings_types.h index 5bc322f0a..fdca4a580 100644 --- a/src/libstrongswan/settings/settings_types.h +++ b/src/libstrongswan/settings/settings_types.h @@ -64,10 +64,20 @@ struct section_t { */ array_t *sections; + /** + * Subsections in original order, as section_t (pointer to obj in sections). + */ + array_t *sections_order; + /** * Key value pairs, as kv_t. */ array_t *kv; + + /** + * Key value pairs in original order, as kv_t (pointer to obj in kv). + */ + array_t *kv_order; }; /** diff --git a/src/libstrongswan/tests/suites/test_settings.c b/src/libstrongswan/tests/suites/test_settings.c index b66ecd5e9..f58c54a6d 100644 --- a/src/libstrongswan/tests/suites/test_settings.c +++ b/src/libstrongswan/tests/suites/test_settings.c @@ -420,37 +420,21 @@ START_TEST(test_set_time) } END_TEST -static bool verify_section(linked_list_t *verifier, char *section) -{ - enumerator_t *enumerator; - char *current; - bool result = FALSE; - - enumerator = verifier->create_enumerator(verifier); - while (enumerator->enumerate(enumerator, ¤t)) - { - if (streq(current, section)) - { - verifier->remove_at(verifier, enumerator); - result = TRUE; - break; - } - } - enumerator->destroy(enumerator); - return result; -} - static void verify_sections(linked_list_t *verifier, char *parent) { - enumerator_t *enumerator; - char *section; + enumerator_t *enumerator, *ver; + char *section, *current; enumerator = settings->create_section_enumerator(settings, parent); - while (enumerator->enumerate(enumerator, §ion)) + ver = verifier->create_enumerator(verifier); + while (enumerator->enumerate(enumerator, §ion) && + ver->enumerate(ver, ¤t)) { - ck_assert(verify_section(verifier, section)); + ck_assert_str_eq(section, current); + verifier->remove_at(verifier, ver); } enumerator->destroy(enumerator); + ver->destroy(ver); ck_assert_int_eq(0, verifier->get_count(verifier)); verifier->destroy(verifier); } @@ -462,8 +446,8 @@ START_TEST(test_section_enumerator) verifier = linked_list_create_with_items("sub1", "sub%", NULL); verify_sections(verifier, "main"); - settings->set_str(settings, "main.sub2.new", "added"); - verifier = linked_list_create_with_items("sub1", "sub%", "sub2", NULL); + settings->set_str(settings, "main.sub0.new", "added"); + verifier = linked_list_create_with_items("sub1", "sub%", "sub0", NULL); verify_sections(verifier, "main"); verifier = linked_list_create_with_items("subsub", NULL); @@ -480,44 +464,27 @@ START_TEST(test_section_enumerator) } END_TEST -static bool verify_key_value(linked_list_t *keys, linked_list_t *values, - char *key, char *value) -{ - enumerator_t *enum_keys, *enum_values; - char *current_key, *current_value; - bool result = FALSE; - - enum_keys = keys->create_enumerator(keys); - enum_values = values->create_enumerator(values); - while (enum_keys->enumerate(enum_keys, ¤t_key) && - enum_values->enumerate(enum_values, ¤t_value)) - { - if (streq(current_key, key)) - { - ck_assert_str_eq(current_value, value); - keys->remove_at(keys, enum_keys); - values->remove_at(values, enum_values); - result = TRUE; - break; - } - } - enum_keys->destroy(enum_keys); - enum_values->destroy(enum_values); - return result; -} - static void verify_key_values(linked_list_t *keys, linked_list_t *values, char *parent) { - enumerator_t *enumerator; - char *key, *value; + enumerator_t *enumerator, *enum_keys, *enum_values; + char *key, *value, *current_key, *current_value; enumerator = settings->create_key_value_enumerator(settings, parent); - while (enumerator->enumerate(enumerator, &key, &value)) + enum_keys = keys->create_enumerator(keys); + enum_values = values->create_enumerator(values); + while (enumerator->enumerate(enumerator, &key, &value) && + enum_keys->enumerate(enum_keys, ¤t_key) && + enum_values->enumerate(enum_values, ¤t_value)) { - ck_assert(verify_key_value(keys, values, key, value)); + ck_assert_str_eq(current_key, key); + ck_assert_str_eq(current_value, value); + keys->remove_at(keys, enum_keys); + values->remove_at(values, enum_values); } enumerator->destroy(enumerator); + enum_keys->destroy(enum_keys); + enum_values->destroy(enum_values); ck_assert_int_eq(0, keys->get_count(keys)); keys->destroy(keys); values->destroy(values); @@ -527,8 +494,8 @@ START_TEST(test_key_value_enumerator) { linked_list_t *keys, *values; - keys = linked_list_create_with_items("key1", "key2", "key3", "empty", NULL); - values = linked_list_create_with_items("val1", "with space", "string with\nnewline", "", NULL); + keys = linked_list_create_with_items("key1", "key2", "empty", "key3", NULL); + values = linked_list_create_with_items("val1", "with space", "", "string with\nnewline", NULL); verify_key_values(keys, values, "main"); keys = linked_list_create_with_items("key", "key2", "subsub", NULL); From b98551c74a6a8ee8f713b0d2b47a7c654b13598b Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 11 Mar 2014 14:19:59 +0100 Subject: [PATCH 15/27] coverage: Make genhtml not fail if sources are not found For some reason the .y and .l files of the settings parser are searched in the wrong directory. --- Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index 7e3c72b3b..0703abc10 100644 --- a/Makefile.am +++ b/Makefile.am @@ -55,7 +55,7 @@ cov-report: lcov -r $(top_builddir)/coverage/coverage.info '*/tests/*' \ -o $(top_builddir)/coverage/coverage.cleaned.info \ --rc lcov_branch_coverage=1 - genhtml --num-spaces 4 --legend --branch-coverage \ + genhtml --num-spaces 4 --legend --branch-coverage --ignore-errors source \ -t "$(PACKAGE_STRING)" \ -o $(top_builddir)/coverage/html \ -p `readlink -m $(abs_top_srcdir)`/src \ From 5ac20cbb8713cea802aa2764704201ae84a0d2d8 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 13 Mar 2014 16:44:45 +0100 Subject: [PATCH 16/27] settings: Only purge sections if necessary Instead of removing and caching all values of a previous config, we only do this for actually removed sections/settings. --- src/libstrongswan/settings/settings.c | 41 +------- src/libstrongswan/settings/settings_types.c | 96 ++++++++++++++++--- src/libstrongswan/settings/settings_types.h | 4 +- .../tests/suites/test_settings.c | 29 ++++++ 4 files changed, 115 insertions(+), 55 deletions(-) diff --git a/src/libstrongswan/settings/settings.c b/src/libstrongswan/settings/settings.c index c6ebe894a..e467d8821 100644 --- a/src/libstrongswan/settings/settings.c +++ b/src/libstrongswan/settings/settings.c @@ -69,40 +69,6 @@ struct private_settings_t { rwlock_t *lock; }; -static void kv_destroy(kv_t *kv, int idx, array_t *contents) -{ - settings_kv_destroy(kv, contents); -} - -/** - * Purge contents of a section, returns if section can be safely removed. - */ -static bool section_purge(section_t *this, array_t *contents) -{ - section_t *current; - int i, idx; - - array_destroy_function(this->kv, (void*)kv_destroy, contents); - this->kv = NULL; - array_destroy(this->kv_order); - this->kv_order = NULL; - /* we ensure sections used as fallback, or configured with fallbacks (or - * having any such subsections) are not removed */ - for (i = array_count(this->sections_order) - 1; i >= 0; i--) - { - array_get(this->sections, i, ¤t); - if (section_purge(current, contents)) - { - array_remove(this->sections_order, i, NULL); - idx = array_bsearch(this->sections, current->name, - settings_section_find, NULL); - array_remove(this->sections, idx, NULL); - settings_section_destroy(current, contents); - } - } - return !this->fallbacks && !array_count(this->sections); -} - /** * Print a format key, but consume already processed arguments */ @@ -890,12 +856,7 @@ static bool load_files_internal(private_settings_t *this, section_t *parent, } this->lock->write_lock(this->lock); - if (!merge) - { - section_purge(parent, this->contents); - } - /* extend parent section */ - settings_section_extend(parent, section, this->contents); + settings_section_extend(parent, section, this->contents, !merge); this->lock->unlock(this->lock); settings_section_destroy(section, NULL); diff --git a/src/libstrongswan/settings/settings_types.c b/src/libstrongswan/settings/settings_types.c index e22d95165..253d06808 100644 --- a/src/libstrongswan/settings/settings_types.c +++ b/src/libstrongswan/settings/settings_types.c @@ -130,10 +130,11 @@ void settings_kv_add(section_t *section, kv_t *kv, array_t *contents) } /* - * Described in header + * Add a section to the given parent, optionally remove settings/subsections + * not found when extending an existing section */ -void settings_section_add(section_t *parent, section_t *section, - array_t *contents) +static void add_section(section_t *parent, section_t *section, + array_t *contents, bool purge) { section_t *found; @@ -146,42 +147,109 @@ void settings_section_add(section_t *parent, section_t *section, } else { - settings_section_extend(found, section, contents); + settings_section_extend(found, section, contents, purge); settings_section_destroy(section, contents); } } +/* + * Described in header + */ +void settings_section_add(section_t *parent, section_t *section, + array_t *contents) +{ + add_section(parent, section, contents, FALSE); +} + +/** + * Purge contents of a section, returns TRUE if section can be safely removed. + */ +static bool section_purge(section_t *this, array_t *contents) +{ + section_t *current; + int i, idx; + + array_destroy_function(this->kv, (void*)kv_destroy, contents); + this->kv = NULL; + array_destroy(this->kv_order); + this->kv_order = NULL; + /* we ensure sections used as fallback, or configured with fallbacks (or + * having any such subsections) are not removed */ + for (i = array_count(this->sections_order) - 1; i >= 0; i--) + { + array_get(this->sections, i, ¤t); + if (section_purge(current, contents)) + { + array_remove(this->sections_order, i, NULL); + idx = array_bsearch(this->sections, current->name, + settings_section_find, NULL); + array_remove(this->sections, idx, NULL); + settings_section_destroy(current, contents); + } + } + return !this->fallbacks && !array_count(this->sections); +} + /* * Described in header */ void settings_section_extend(section_t *base, section_t *extension, - array_t *contents) + array_t *contents, bool purge) { enumerator_t *enumerator; section_t *section; kv_t *kv; int idx; - enumerator = array_create_enumerator(extension->sections_order); - while (enumerator->enumerate(enumerator, (void**)§ion)) + if (purge) + { /* remove sections and settings in base not found in extension */ + enumerator = array_create_enumerator(base->sections_order); + while (enumerator->enumerate(enumerator, (void**)§ion)) + { + if (array_bsearch(extension->sections, section->name, + settings_section_find, NULL) == -1) + { + idx = array_bsearch(base->sections, section->name, + settings_section_find, NULL); + if (section_purge(section, contents)) + { + array_remove(base->sections, idx, NULL); + array_remove_at(base->sections_order, enumerator); + settings_section_destroy(section, contents); + } + } + } + enumerator->destroy(enumerator); + + enumerator = array_create_enumerator(base->kv_order); + while (enumerator->enumerate(enumerator, (void**)&kv)) + { + if (array_bsearch(extension->kv, kv->key, settings_kv_find, + NULL) == -1) + { + idx = array_bsearch(base->kv, kv->key, settings_kv_find, NULL); + array_remove(base->kv, idx, NULL); + array_remove_at(base->kv_order, enumerator); + settings_kv_destroy(kv, contents); + } + } + enumerator->destroy(enumerator); + } + + while (array_remove(extension->sections_order, 0, §ion)) { idx = array_bsearch(extension->sections, section->name, settings_section_find, NULL); array_remove(extension->sections, idx, NULL); - array_remove_at(extension->sections_order, enumerator); - settings_section_add(base, section, contents); + add_section(base, section, contents, purge); } - enumerator->destroy(enumerator); - enumerator = array_create_enumerator(extension->kv_order); - while (enumerator->enumerate(enumerator, (void**)&kv)) + while (array_remove(extension->kv_order, 0, &kv)) { idx = array_bsearch(extension->kv, kv->key, settings_kv_find, NULL); array_remove(extension->kv, idx, NULL); - array_remove_at(extension->kv_order, enumerator); settings_kv_add(base, kv, contents); } - enumerator->destroy(enumerator); } /* diff --git a/src/libstrongswan/settings/settings_types.h b/src/libstrongswan/settings/settings_types.h index fdca4a580..67299d8e7 100644 --- a/src/libstrongswan/settings/settings_types.h +++ b/src/libstrongswan/settings/settings_types.h @@ -148,9 +148,11 @@ void settings_section_add(section_t *parent, section_t *section, * @param base base section to extend * @param extension section whose data is extracted * @param contents optional array to store replaced values in + * @param purge TRUE to remove settings and sections not found in the + * extension (unless (sub-)sections have/are fallbacks) */ void settings_section_extend(section_t *base, section_t *extension, - array_t *contents); + array_t *contents, bool purge); /** * Callback to find a section by name diff --git a/src/libstrongswan/tests/suites/test_settings.c b/src/libstrongswan/tests/suites/test_settings.c index f58c54a6d..77fef12ad 100644 --- a/src/libstrongswan/tests/suites/test_settings.c +++ b/src/libstrongswan/tests/suites/test_settings.c @@ -531,6 +531,7 @@ START_SETUP(setup_include_config) "main {\n" " key1 = n1\n" " key2 = n2\n" + " key3 = val3\n" " none = \n" " sub1 {\n" " key3 = value\n" @@ -563,6 +564,7 @@ static void verify_include() { verify_string("n1", "main.key1"); verify_string("v2", "main.key2"); + verify_string("val3", "main.key3"); verify_string("val", "main.sub1.key"); verify_string("v2", "main.sub1.key2"); verify_string("val", "main.sub1.sub1.key"); @@ -600,6 +602,7 @@ START_TEST(test_load_files) "main {\n" " key1 = val1\n" " key2 = val2\n" + " key3 = val3\n" " none = x\n" " sub1 {\n" " include = value\n" @@ -609,9 +612,35 @@ START_TEST(test_load_files) " }\n" " }\n" "}"); + char *val1, *val2, *val3; create_settings(contents); + val1 = settings->get_str(settings, "main.key1", NULL); + val2 = settings->get_str(settings, "main.sub1.key2", NULL); + /* loading the same file twice should not change anything, with... */ + ck_assert(settings->load_files(settings, path, TRUE)); + ck_assert(val1 == settings->get_str(settings, "main.key1", NULL)); + ck_assert(val2 == settings->get_str(settings, "main.sub1.key2", NULL)); + /* ...or without merging */ + ck_assert(settings->load_files(settings, path, FALSE)); + ck_assert(val1 == settings->get_str(settings, "main.key1", NULL)); + ck_assert(val2 == settings->get_str(settings, "main.sub1.key2", NULL)); + + val1 = settings->get_str(settings, "main.key2", NULL); + val2 = settings->get_str(settings, "main.key3", NULL); + val3 = settings->get_str(settings, "main.none", NULL); + /* only pointers for modified settings should change, but still be valid */ + ck_assert(settings->load_files(settings, include1, FALSE)); + ck_assert(val1 != settings->get_str(settings, "main.key2", NULL)); + ck_assert_str_eq(val1, "val2"); + ck_assert(val2 == settings->get_str(settings, "main.key3", NULL)); + ck_assert(val3 != settings->get_str(settings, "main.none", NULL)); + ck_assert_str_eq(val3, "x"); + + settings->destroy(settings); + create_settings(contents); + ck_assert(settings->load_files(settings, include1, TRUE)); verify_include(); From 8b43c9ba349d7370eb82e8340f48a7045d9c5c5e Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 29 Apr 2014 16:04:43 +0200 Subject: [PATCH 17/27] settings: Adopt the new order of sections and settings when replacing configs --- src/libstrongswan/settings/settings_types.c | 42 ++++++++-- .../tests/suites/test_settings.c | 84 ++++++++++++++++++- 2 files changed, 119 insertions(+), 7 deletions(-) diff --git a/src/libstrongswan/settings/settings_types.c b/src/libstrongswan/settings/settings_types.c index 253d06808..125676237 100644 --- a/src/libstrongswan/settings/settings_types.c +++ b/src/libstrongswan/settings/settings_types.c @@ -199,10 +199,13 @@ void settings_section_extend(section_t *base, section_t *extension, enumerator_t *enumerator; section_t *section; kv_t *kv; + array_t *sections = NULL, *kvs = NULL; int idx; if (purge) - { /* remove sections and settings in base not found in extension */ + { /* remove sections and settings in base not found in extension, the + * others are removed too (from the _order list) so they can be inserted + * in the order found in extension */ enumerator = array_create_enumerator(base->sections_order); while (enumerator->enumerate(enumerator, (void**)§ion)) { @@ -212,32 +215,49 @@ void settings_section_extend(section_t *base, section_t *extension, idx = array_bsearch(base->sections, section->name, settings_section_find, NULL); if (section_purge(section, contents)) - { + { /* only remove them if we can purge them */ array_remove(base->sections, idx, NULL); array_remove_at(base->sections_order, enumerator); settings_section_destroy(section, contents); } } + else + { + array_remove_at(base->sections_order, enumerator); + array_insert_create(§ions, ARRAY_TAIL, section); + array_sort(sections, settings_section_sort, NULL); + } } enumerator->destroy(enumerator); - enumerator = array_create_enumerator(base->kv_order); - while (enumerator->enumerate(enumerator, (void**)&kv)) + while (array_remove(base->kv_order, 0, &kv)) { if (array_bsearch(extension->kv, kv->key, settings_kv_find, NULL) == -1) { idx = array_bsearch(base->kv, kv->key, settings_kv_find, NULL); array_remove(base->kv, idx, NULL); - array_remove_at(base->kv_order, enumerator); settings_kv_destroy(kv, contents); } + else + { + array_insert_create(&kvs, ARRAY_TAIL, kv); + array_sort(kvs, settings_kv_sort, NULL); + } } - enumerator->destroy(enumerator); } while (array_remove(extension->sections_order, 0, §ion)) { + idx = array_bsearch(sections, section->name, + settings_section_find, NULL); + if (idx != -1) + { + section_t *existing; + + array_remove(sections, idx, &existing); + array_insert(base->sections_order, ARRAY_TAIL, existing); + } idx = array_bsearch(extension->sections, section->name, settings_section_find, NULL); array_remove(extension->sections, idx, NULL); @@ -246,10 +266,20 @@ void settings_section_extend(section_t *base, section_t *extension, while (array_remove(extension->kv_order, 0, &kv)) { + idx = array_bsearch(kvs, kv->key, settings_kv_find, NULL); + if (idx != -1) + { + kv_t *existing; + + array_remove(kvs, idx, &existing); + array_insert(base->kv_order, ARRAY_TAIL, existing); + } idx = array_bsearch(extension->kv, kv->key, settings_kv_find, NULL); array_remove(extension->kv, idx, NULL); settings_kv_add(base, kv, contents); } + array_destroy(sections); + array_destroy(kvs); } /* diff --git a/src/libstrongswan/tests/suites/test_settings.c b/src/libstrongswan/tests/suites/test_settings.c index 77fef12ad..e504b95a1 100644 --- a/src/libstrongswan/tests/suites/test_settings.c +++ b/src/libstrongswan/tests/suites/test_settings.c @@ -694,6 +694,87 @@ START_TEST(test_load_files_section) } END_TEST +START_TEST(test_order_kv) +{ + chunk_t base = chunk_from_str( + "main {\n" + " key1 = val1\n" + " key2 = val2\n" + " key3 = val3\n" + "}"); + chunk_t include = chunk_from_str( + "main {\n" + " key0 = val0\n" + " key3 = val3\n" + " key1 = val1\n" + "}"); + linked_list_t *keys, *values; + + create_settings(base); + ck_assert(chunk_write(include, include1, 0022, TRUE)); + + keys = linked_list_create_with_items("key1", "key2", "key3", NULL); + values = linked_list_create_with_items("val1", "val2", "val3", NULL); + verify_key_values(keys, values, "main"); + + /* the original order is maintained if the settings are merged */ + ck_assert(settings->load_files(settings, include1, TRUE)); + keys = linked_list_create_with_items("key1", "key2", "key3", "key0", NULL); + values = linked_list_create_with_items("val1", "val2", "val3", "val0", NULL); + verify_key_values(keys, values, "main"); + + /* but the new order is adopted if the settings are replaced */ + ck_assert(settings->load_files(settings, include1, FALSE)); + keys = linked_list_create_with_items("key0", "key3", "key1", NULL); + values = linked_list_create_with_items("val0", "val3", "val1", NULL); + verify_key_values(keys, values, "main"); + + unlink(include1); +} +END_TEST + +START_TEST(test_order_section) +{ + chunk_t base = chunk_from_str( + "main {\n" + " sub1 {\n" + " }\n" + " sub2 {\n" + " }\n" + " sub3 {\n" + " }\n" + "}"); + chunk_t include = chunk_from_str( + "main {\n" + " sub0 {\n" + " }\n" + " sub3 {\n" + " }\n" + " sub1 {\n" + " }\n" + "}"); + linked_list_t *sections; + + create_settings(base); + ck_assert(chunk_write(include, include1, 0022, TRUE)); + + sections = linked_list_create_with_items("sub1", "sub2", "sub3", NULL); + verify_sections(sections, "main"); + + /* the original order is maintained if the settings are merged */ + ck_assert(settings->load_files(settings, include1, TRUE)); + sections = linked_list_create_with_items("sub1", "sub2", "sub3", "sub0", NULL); + verify_sections(sections, "main"); + + /* but the new order is adopted if the settings are replaced */ + ck_assert(settings->load_files(settings, include1, FALSE)); + sections = linked_list_create_with_items("sub0", "sub3", "sub1", NULL); + verify_sections(sections, "main"); + + unlink(include1); +} +END_TEST + START_SETUP(setup_fallback_config) { create_settings(chunk_from_str( @@ -811,7 +892,6 @@ START_TEST(test_add_fallback_printf) } END_TEST - START_SETUP(setup_string_config) { create_settings(chunk_from_str( @@ -967,6 +1047,8 @@ Suite *settings_suite_create() tcase_add_test(tc, test_include); tcase_add_test(tc, test_load_files); tcase_add_test(tc, test_load_files_section); + tcase_add_test(tc, test_order_kv); + tcase_add_test(tc, test_order_section); suite_add_tcase(s, tc); tc = tcase_create("fallback"); From f65ac98c645e5b5798ecdd7b81b22c1978cbf1dc Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 29 Apr 2014 11:39:57 +0200 Subject: [PATCH 18/27] settings: Reduce log verbosity if files can't be opened Basically reintroducing 2a38b4556e9fd8102bd6c6c61f2893599a5e8e51. --- src/libstrongswan/settings/settings_parser.y | 11 ++++++++++- src/libstrongswan/utils/parser_helper.c | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/libstrongswan/settings/settings_parser.y b/src/libstrongswan/settings/settings_parser.y index b81003f29..d5566df9d 100644 --- a/src/libstrongswan/settings/settings_parser.y +++ b/src/libstrongswan/settings/settings_parser.y @@ -255,7 +255,16 @@ bool settings_parser_parse_file(section_t *root, char *name) helper->file_include(helper, name); if (!settings_parser_open_next_file(helper)) { - DBG1(DBG_CFG, "failed to open config file '%s'", name); +#ifdef STRONGSWAN_CONF + if (streq(name, STRONGSWAN_CONF)) + { + DBG2(DBG_CFG, "failed to open config file '%s'", name); + } + else +#endif + { + DBG1(DBG_CFG, "failed to open config file '%s'", name); + } } else { diff --git a/src/libstrongswan/utils/parser_helper.c b/src/libstrongswan/utils/parser_helper.c index fb549a572..509316665 100644 --- a/src/libstrongswan/utils/parser_helper.c +++ b/src/libstrongswan/utils/parser_helper.c @@ -111,7 +111,7 @@ METHOD(parser_helper_t, file_next, parser_helper_file_t*, array_insert(this->files, ARRAY_TAIL, next); return &next->public; } - PARSER_DBG1(&this->public, "unable to open '%s'", name); + PARSER_DBG2(&this->public, "unable to open '%s'", name); parser_helper_file_destroy(next); } file->matches->destroy(file->matches); From 813e510d69b27ab25cfab01cc89dd34ad3927e73 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 13 May 2014 09:54:49 +0200 Subject: [PATCH 19/27] settings: Include generated header after others Newer Bison versions declare the parser function in the header, which requires custom types. --- src/libstrongswan/settings/settings_lexer.l | 4 ++-- src/libstrongswan/settings/settings_parser.y | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libstrongswan/settings/settings_lexer.l b/src/libstrongswan/settings/settings_lexer.l index f287761c1..5074e70d8 100644 --- a/src/libstrongswan/settings/settings_lexer.l +++ b/src/libstrongswan/settings/settings_lexer.l @@ -14,10 +14,10 @@ * for more details. */ -#include "settings_parser.h" - #include +#include "settings_parser.h" + bool settings_parser_open_next_file(parser_helper_t *ctx); static void include_files(parser_helper_t *ctx); diff --git a/src/libstrongswan/settings/settings_parser.y b/src/libstrongswan/settings/settings_parser.y index d5566df9d..f4a4431c7 100644 --- a/src/libstrongswan/settings/settings_parser.y +++ b/src/libstrongswan/settings/settings_parser.y @@ -17,13 +17,13 @@ #define _GNU_SOURCE /* for asprintf() */ #include -#include "settings_parser.h" - #include #include #include #include +#include "settings_parser.h" + #define YYDEBUG 1 /** From da45f9e994f61324a058def37fbafd5cce75b5f3 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 13 May 2014 10:50:36 +0200 Subject: [PATCH 20/27] settings: Replace deprecated YYLEX_PARAM with %lex-param With Bison 3.x support for YYLEX_PARAM has been removed and %lex-param should be used. Unfortunately, that option does not take expressions. Instead we use a wrapper function that calls the lexer with the proper scanner object, which should also be backward compatible to older Bison versions. --- src/libstrongswan/settings/settings_parser.y | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libstrongswan/settings/settings_parser.y b/src/libstrongswan/settings/settings_parser.y index f4a4431c7..b79faf6b7 100644 --- a/src/libstrongswan/settings/settings_parser.y +++ b/src/libstrongswan/settings/settings_parser.y @@ -50,9 +50,13 @@ static void add_section(parser_helper_t *ctx, section_t *section); static void add_setting(parser_helper_t *ctx, kv_t *kv); /** - * Make sure the generated parser code passes the correct object to the lexer + * Make sure to call lexer with the proper context */ -#define YYLEX_PARAM ctx->scanner +#undef yylex +static int yylex(YYSTYPE *lvalp, parser_helper_t *ctx) +{ + return settings_parser_lex(lvalp, ctx->scanner); +} %} %debug @@ -65,7 +69,7 @@ static void add_setting(parser_helper_t *ctx, kv_t *kv); %name-prefix "settings_parser_" /* interact properly with the reentrant lexer */ -%lex-param {void *scanner} +%lex-param {parser_helper_t *ctx} %parse-param {parser_helper_t *ctx} /* types for terminal symbols... (can't use the typedef'd types) */ From 4b670a20a9ee0ac5efd806ff804e75fe00509826 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 14 May 2014 16:05:02 +0200 Subject: [PATCH 21/27] settings: strongswan.conf must be loaded explicitly --- src/charon-cmd/charon-cmd.c | 5 ++++- src/charon/charon.c | 5 ++++- src/libstrongswan/library.c | 7 +++++++ src/libstrongswan/settings/settings.c | 8 ++------ src/libstrongswan/settings/settings.h | 4 +++- 5 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/charon-cmd/charon-cmd.c b/src/charon-cmd/charon-cmd.c index a70d314af..b41cf467c 100644 --- a/src/charon-cmd/charon-cmd.c +++ b/src/charon-cmd/charon-cmd.c @@ -126,12 +126,15 @@ static int run() { DBG1(DBG_DMN, "signal of type SIGHUP received. Reloading " "configuration"); - if (lib->settings->load_files(lib->settings, NULL, FALSE)) +#ifdef STRONGSWAN_CONF + if (lib->settings->load_files(lib->settings, STRONGSWAN_CONF, + FALSE)) { charon->load_loggers(charon, levels, TRUE); lib->plugins->reload(lib->plugins, NULL); } else +#endif { DBG1(DBG_DMN, "reloading config failed, keeping old"); } diff --git a/src/charon/charon.c b/src/charon/charon.c index 089ac4570..a82aa4256 100644 --- a/src/charon/charon.c +++ b/src/charon/charon.c @@ -122,12 +122,15 @@ static void run() { DBG1(DBG_DMN, "signal of type SIGHUP received. Reloading " "configuration"); - if (lib->settings->load_files(lib->settings, NULL, FALSE)) +#ifdef STRONGSWAN_CONF + if (lib->settings->load_files(lib->settings, STRONGSWAN_CONF, + FALSE)) { charon->load_loggers(charon, levels, !use_syslog); lib->plugins->reload(lib->plugins, NULL); } else +#endif { DBG1(DBG_DMN, "reloading config failed, keeping old"); } diff --git a/src/libstrongswan/library.c b/src/libstrongswan/library.c index c5bb4cd93..f152a8c1f 100644 --- a/src/libstrongswan/library.c +++ b/src/libstrongswan/library.c @@ -298,6 +298,13 @@ bool library_init(char *settings, const char *namespace) this->objects = hashtable_create((hashtable_hash_t)hash, (hashtable_equals_t)equals, 4); + +#ifdef STRONGSWAN_CONF + if (!settings) + { + settings = STRONGSWAN_CONF; + } +#endif this->public.settings = settings_create(settings); /* all namespace settings may fall back to libstrongswan */ lib->settings->add_fallback(lib->settings, lib->ns, "libstrongswan"); diff --git a/src/libstrongswan/settings/settings.c b/src/libstrongswan/settings/settings.c index e467d8821..6c69782ec 100644 --- a/src/libstrongswan/settings/settings.c +++ b/src/libstrongswan/settings/settings.c @@ -839,13 +839,9 @@ static bool load_files_internal(private_settings_t *this, section_t *parent, { section_t *section; - if (pattern == NULL) - { -#ifdef STRONGSWAN_CONF - pattern = STRONGSWAN_CONF; -#else + if (pattern == NULL || !pattern[0]) + { /* TODO: Clear parent if merge is FALSE? */ return FALSE; -#endif } section = settings_section_create(NULL); diff --git a/src/libstrongswan/settings/settings.h b/src/libstrongswan/settings/settings.h index 23f3a8ead..3b87c8feb 100644 --- a/src/libstrongswan/settings/settings.h +++ b/src/libstrongswan/settings/settings.h @@ -343,7 +343,9 @@ struct settings_t { /** * Load settings from a file. * - * @param file file to read settings from, NULL for default + * @note If parsing the file fails the object is still created. + * + * @param file optional file to read settings from * @return settings object */ settings_t *settings_create(char *file); From c976cc7d33d6c3aeeb6e2a8a82c10401f629f93c Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 14 May 2014 17:19:41 +0200 Subject: [PATCH 22/27] parser-helper: Make parser_helper_log a function --- src/libstrongswan/utils/parser_helper.c | 27 +++++++++++++++++++++++++ src/libstrongswan/utils/parser_helper.h | 10 +-------- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/src/libstrongswan/utils/parser_helper.c b/src/libstrongswan/utils/parser_helper.c index 509316665..403cca92b 100644 --- a/src/libstrongswan/utils/parser_helper.c +++ b/src/libstrongswan/utils/parser_helper.c @@ -15,6 +15,7 @@ #include #include +#include #include "parser_helper.h" @@ -196,6 +197,32 @@ METHOD(parser_helper_t, destroy, void, free(this); } +/** + * Described in header + */ +void parser_helper_log(int level, parser_helper_t *ctx, char *fmt, ...) +{ + parser_helper_file_t *file; + char msg[8192]; + va_list args; + int line; + + va_start(args, fmt); + vsnprintf(msg, sizeof(msg), fmt, args); + va_end(args); + + file = ctx->file_current(ctx); + line = ctx->get_lineno ? ctx->get_lineno(ctx->scanner) : 0; + if (file) + { + dbg(DBG_CFG, level, "%s:%d: %s", file->name, line, msg); + } + else + { + dbg(DBG_CFG, level, "%s", msg); + } +} + /** * Described in header */ diff --git a/src/libstrongswan/utils/parser_helper.h b/src/libstrongswan/utils/parser_helper.h index 741582d2c..6a2c26c8e 100644 --- a/src/libstrongswan/utils/parser_helper.h +++ b/src/libstrongswan/utils/parser_helper.h @@ -148,15 +148,7 @@ struct parser_helper_file_t { * @param fmt error message format * @param ... additional arguments */ -#define parser_helper_log(level, ctx, fmt, ...) ({ \ - parser_helper_file_t *_file = (ctx)->file_current(ctx); \ - int _line = (ctx)->get_lineno ? (ctx)->get_lineno((ctx)->scanner) : 0; \ - if (_file) {\ - DBG##level(DBG_CFG, "%s:%d: " fmt, _file->name, _line, ##__VA_ARGS__); \ - } else { \ - DBG##level(DBG_CFG, fmt, ##__VA_ARGS__); \ - } \ -}) +void parser_helper_log(int level, parser_helper_t *ctx, char *fmt, ...); #define PARSER_DBG1(ctx, fmt, ...) parser_helper_log(1, ctx, fmt, ##__VA_ARGS__) #define PARSER_DBG2(ctx, fmt, ...) parser_helper_log(2, ctx, fmt, ##__VA_ARGS__) From 66248396c69d16988f45ef6879389f3e0914ec10 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 14 May 2014 17:21:04 +0200 Subject: [PATCH 23/27] parser-helper: Make parser_helper_file_t private --- src/libstrongswan/settings/settings_lexer.l | 6 +- src/libstrongswan/utils/parser_helper.c | 65 +++++++++++---------- src/libstrongswan/utils/parser_helper.h | 27 +-------- 3 files changed, 40 insertions(+), 58 deletions(-) diff --git a/src/libstrongswan/settings/settings_lexer.l b/src/libstrongswan/settings/settings_lexer.l index 5074e70d8..a680e71f4 100644 --- a/src/libstrongswan/settings/settings_lexer.l +++ b/src/libstrongswan/settings/settings_lexer.l @@ -163,7 +163,7 @@ static void include_files(parser_helper_t *ctx); */ bool settings_parser_open_next_file(parser_helper_t *ctx) { - parser_helper_file_t *file; + FILE *file; file = ctx->file_next(ctx); if (!file) @@ -171,9 +171,9 @@ bool settings_parser_open_next_file(parser_helper_t *ctx) return FALSE; } - settings_parser_set_in(file->file, ctx->scanner); + settings_parser_set_in(file, ctx->scanner); settings_parser_push_buffer_state( - settings_parser__create_buffer(file->file, YY_BUF_SIZE, + settings_parser__create_buffer(file, YY_BUF_SIZE, ctx->scanner), ctx->scanner); return TRUE; } diff --git a/src/libstrongswan/utils/parser_helper.c b/src/libstrongswan/utils/parser_helper.c index 403cca92b..40c6cfedd 100644 --- a/src/libstrongswan/utils/parser_helper.c +++ b/src/libstrongswan/utils/parser_helper.c @@ -22,7 +22,7 @@ #include typedef struct private_parser_helper_t private_parser_helper_t; -typedef struct private_parser_helper_file_t private_parser_helper_file_t; +typedef struct parser_helper_file_t parser_helper_file_t; struct private_parser_helper_t { @@ -32,7 +32,7 @@ struct private_parser_helper_t { parser_helper_t public; /** - * Stack of included files, as private_parser_helper_file_t. + * Stack of included files, as parser_helper_file_t. */ array_t *files; @@ -42,12 +42,17 @@ struct private_parser_helper_t { bio_writer_t *writer; }; -struct private_parser_helper_file_t { +struct parser_helper_file_t { /** - * File data. + * File name */ - parser_helper_file_t public; + char *name; + + /** + * File stream + */ + FILE *file; /** * Enumerator of paths matching the most recent inclusion pattern. @@ -58,34 +63,36 @@ struct private_parser_helper_file_t { /** * Destroy the given file data. */ -static void parser_helper_file_destroy(private_parser_helper_file_t *this) +static void parser_helper_file_destroy(parser_helper_file_t *this) { - if (this->public.file) + if (this->file) { - fclose(this->public.file); + fclose(this->file); } - free(this->public.name); + free(this->name); DESTROY_IF(this->matches); free(this); } -METHOD(parser_helper_t, file_current, parser_helper_file_t*, - private_parser_helper_t *this) +/** + * Returns the current file, if any. + */ +static parser_helper_file_t *current_file(private_parser_helper_t *this) { - private_parser_helper_file_t *file; + parser_helper_file_t *file; array_get(this->files, ARRAY_TAIL, &file); - if (file->public.name) + if (file->name) { - return &file->public; + return file; } return NULL; } -METHOD(parser_helper_t, file_next, parser_helper_file_t*, +METHOD(parser_helper_t, file_next, FILE*, private_parser_helper_t *this) { - private_parser_helper_file_t *file, *next; + parser_helper_file_t *file, *next; char *name; array_get(this->files, ARRAY_TAIL, &file); @@ -101,16 +108,14 @@ METHOD(parser_helper_t, file_next, parser_helper_file_t*, while (file->matches->enumerate(file->matches, &name, NULL)) { INIT(next, - .public = { - .name = strdup(name), - .file = fopen(name, "r"), - }, + .name = strdup(name), + .file = fopen(name, "r"), ); - if (next->public.file) + if (next->file) { array_insert(this->files, ARRAY_TAIL, next); - return &next->public; + return next->file; } PARSER_DBG2(&this->public, "unable to open '%s'", name); parser_helper_file_destroy(next); @@ -124,7 +129,7 @@ METHOD(parser_helper_t, file_next, parser_helper_file_t*, METHOD(parser_helper_t, file_include, void, private_parser_helper_t *this, char *pattern) { - private_parser_helper_file_t *file; + parser_helper_file_t *file; char pat[PATH_MAX]; array_get(this->files, ARRAY_TAIL, &file); @@ -135,7 +140,7 @@ METHOD(parser_helper_t, file_include, void, return; } - if (!file->public.name || pattern[0] == '/') + if (!file->name || pattern[0] == '/') { /* absolute path */ if (snprintf(pat, sizeof(pat), "%s", pattern) >= sizeof(pat)) { @@ -146,7 +151,7 @@ METHOD(parser_helper_t, file_include, void, } else { /* base relative paths to the directory of the current file */ - char *dir = path_dirname(file->public.name); + char *dir = path_dirname(file->name); if (snprintf(pat, sizeof(pat), "%s/%s", dir, pattern) >= sizeof(pat)) { PARSER_DBG1(&this->public, "include pattern too long, ignored"); @@ -202,6 +207,7 @@ METHOD(parser_helper_t, destroy, void, */ void parser_helper_log(int level, parser_helper_t *ctx, char *fmt, ...) { + private_parser_helper_t *this = (private_parser_helper_t*)ctx; parser_helper_file_t *file; char msg[8192]; va_list args; @@ -211,7 +217,7 @@ void parser_helper_log(int level, parser_helper_t *ctx, char *fmt, ...) vsnprintf(msg, sizeof(msg), fmt, args); va_end(args); - file = ctx->file_current(ctx); + file = current_file(this); line = ctx->get_lineno ? ctx->get_lineno(ctx->scanner) : 0; if (file) { @@ -229,12 +235,11 @@ void parser_helper_log(int level, parser_helper_t *ctx, char *fmt, ...) parser_helper_t *parser_helper_create(void *context) { private_parser_helper_t *this; - private_parser_helper_file_t *sentinel; + parser_helper_file_t *sentinel; INIT(this, .public = { .context = context, - .file_current = _file_current, .file_include = _file_include, .file_next = _file_next, .string_init = _string_init, @@ -247,9 +252,7 @@ parser_helper_t *parser_helper_create(void *context) ); INIT(sentinel, - .public = { - .name = NULL, - }, + .name = NULL, ); array_insert(this->files, ARRAY_TAIL, sentinel); diff --git a/src/libstrongswan/utils/parser_helper.h b/src/libstrongswan/utils/parser_helper.h index 6a2c26c8e..d8602522b 100644 --- a/src/libstrongswan/utils/parser_helper.h +++ b/src/libstrongswan/utils/parser_helper.h @@ -25,7 +25,6 @@ #include typedef struct parser_helper_t parser_helper_t; -typedef struct parser_helper_file_t parser_helper_file_t; /** * Helper class for flex/bison based parsers. @@ -58,13 +57,6 @@ struct parser_helper_t { */ int (*get_lineno)(void *scanner); - /** - * Get the current file. - * - * @return current file, or NULL - */ - parser_helper_file_t *(*file_current)(parser_helper_t *this); - /** * Resolves the given include pattern, relative to the location of the * current file. @@ -89,8 +81,8 @@ struct parser_helper_t { * then call this method to check if there are more files to include for * the most recent call to file_include(), if so, call * @code - * PREFIXset_in(file->file, helper->scanner); - * PREFIXpush_buffer_state(PREFIX_create_buffer(file->file, YY_BUF_SIZE, + * PREFIXset_in(file, helper->scanner); + * PREFIXpush_buffer_state(PREFIX_create_buffer(file, YY_BUF_SIZE, * helper->scanner), helper->scanner); * @endcode * @@ -99,7 +91,7 @@ struct parser_helper_t { * * @return next file to process, or NULL (see comment) */ - parser_helper_file_t *(*file_next)(parser_helper_t *this); + FILE *(*file_next)(parser_helper_t *this); /** * Start parsing a string, discards any currently stored data. @@ -127,19 +119,6 @@ struct parser_helper_t { void (*destroy)(parser_helper_t *this); }; -struct parser_helper_file_t { - - /** - * File name - */ - char *name; - - /** - * File stream - */ - FILE *file; -}; - /** * Log the given message either as error or warning * From 4102fc9c093e32cd20132665e927ef79bfab3b6d Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 14 May 2014 18:38:35 +0200 Subject: [PATCH 24/27] parser-helper: Define debug macros depending on DEBUG_LEVEL --- src/libstrongswan/utils/parser_helper.h | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/libstrongswan/utils/parser_helper.h b/src/libstrongswan/utils/parser_helper.h index d8602522b..09ed1991c 100644 --- a/src/libstrongswan/utils/parser_helper.h +++ b/src/libstrongswan/utils/parser_helper.h @@ -21,6 +21,7 @@ #ifndef PARSER_HELPER_H_ #define PARSER_HELPER_H_ +#include #include #include @@ -129,9 +130,25 @@ struct parser_helper_t { */ void parser_helper_log(int level, parser_helper_t *ctx, char *fmt, ...); -#define PARSER_DBG1(ctx, fmt, ...) parser_helper_log(1, ctx, fmt, ##__VA_ARGS__) -#define PARSER_DBG2(ctx, fmt, ...) parser_helper_log(2, ctx, fmt, ##__VA_ARGS__) -#define PARSER_DBG3(ctx, fmt, ...) parser_helper_log(3, ctx, fmt, ##__VA_ARGS__) +#if DEBUG_LEVEL >= 1 +# define PARSER_DBG1(ctx, fmt, ...) parser_helper_log(1, ctx, fmt, ##__VA_ARGS__) +#endif +#if DEBUG_LEVEL >= 2 +# define PARSER_DBG2(ctx, fmt, ...) parser_helper_log(2, ctx, fmt, ##__VA_ARGS__) +#endif +#if DEBUG_LEVEL >= 3 +# define PARSER_DBG3(ctx, fmt, ...) parser_helper_log(3, ctx, fmt, ##__VA_ARGS__) +#endif + +#ifndef PARSER_DBG1 +# define PARSER_DBG1(...) {} +#endif +#ifndef PARSER_DBG2 +# define PARSER_DBG2(...) {} +#endif +#ifndef PARSER_DBG3 +# define PARSER_DBG3(...) {} +#endif /** * Create a parser helper object From 5b64c04046c1f3eada62e510e5ec4cd8b128f021 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Mon, 10 Mar 2014 14:51:48 +0100 Subject: [PATCH 25/27] scripts: Add test script for settings_t --- scripts/Makefile.am | 3 +- scripts/settings-test.c | 126 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 scripts/settings-test.c diff --git a/scripts/Makefile.am b/scripts/Makefile.am index ed5147a3d..abc6d75dd 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -5,7 +5,7 @@ AM_CPPFLAGS = \ noinst_PROGRAMS = bin2array bin2sql id2sql key2keyid keyid2sql oid2der \ thread_analysis dh_speed pubkey_speed crypt_burn hash_burn fetch \ - dnssec malloc_speed aes-test + dnssec malloc_speed aes-test settings-test if USE_TLS noinst_PROGRAMS += tls_test @@ -40,6 +40,7 @@ malloc_speed_LDADD = $(top_builddir)/src/libstrongswan/libstrongswan.la $(RTLIB) fetch_LDADD = $(top_builddir)/src/libstrongswan/libstrongswan.la dnssec_LDADD = $(top_builddir)/src/libstrongswan/libstrongswan.la aes_test_LDADD = $(top_builddir)/src/libstrongswan/libstrongswan.la +settings_test_LDADD = $(top_builddir)/src/libstrongswan/libstrongswan.la key2keyid.o : $(top_builddir)/config.status diff --git a/scripts/settings-test.c b/scripts/settings-test.c new file mode 100644 index 000000000..452798aee --- /dev/null +++ b/scripts/settings-test.c @@ -0,0 +1,126 @@ +/* + * Copyright (C) 2014 Tobias Brunner + * Hochschule fuer Technik Rapperswil + * + * 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. See . + * + * 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. + */ + +#include +#include +#include +#include +#include +#include + +#include +#include + +/** + * Defined in libstrongswan but not part of the public API + */ +bool settings_parser_parse_file(void *this, char *name); + +/** + * Recursively print the section and all subsections/settings + */ +static void print_section(section_t *section, int level) +{ + section_t *sub; + kv_t *kv; + int i; + char indent[256]; + + for (i = 0; i < level * 2 && i < sizeof(indent) - 2; i += 2) + { + indent[i ] = ' '; + indent[i+1] = ' '; + } + indent[i] = '\0'; + + for (i = 0; i < array_count(section->kv_order); i++) + { + array_get(section->kv_order, i, &kv); + printf("%s%s = %s\n", indent, kv->key, kv->value); + } + for (i = 0; i < array_count(section->sections_order); i++) + { + array_get(section->sections_order, i, &sub); + printf("%s%s {\n", indent, sub->name); + print_section(sub, level + 1); + printf("%s}\n", indent); + } +} + +static void usage(FILE *out, char *name) +{ + fprintf(out, "Test strongswan.conf parser\n\n"); + fprintf(out, "%s [OPTIONS]\n\n", name); + fprintf(out, "Options:\n"); + fprintf(out, " -h, --help print this help.\n"); + fprintf(out, " -d, --debug enables debugging of the parser.\n"); + fprintf(out, " -f, --file=FILE config file to load (default STDIN).\n"); + fprintf(out, "\n"); +} + +int main(int argc, char *argv[]) +{ + char *file = NULL; + + /* don't load strongswan.conf */ + library_init("", "settings-test"); + atexit(library_deinit); + + dbg_default_set_level(3); + + while (true) + { + struct option long_opts[] = { + {"help", no_argument, NULL, 'h' }, + {"debug", no_argument, NULL, 'd' }, + {"file", required_argument, NULL, 'f' }, + {0,0,0,0 }, + }; + switch (getopt_long(argc, argv, "hdf:", long_opts, NULL)) + { + case EOF: + break; + case 'h': + usage(stdout, argv[0]); + return 0; + case 'd': + setenv("DEBUG_SETTINGS_PARSER", "1", TRUE); + continue; + case 'f': + file = optarg; + continue; + default: + usage(stderr, argv[0]); + return 1; + } + break; + } + + if (file) + { + section_t *root = settings_section_create(strdup("root")); + + settings_parser_parse_file(root, file); + + print_section(root, 0); + + settings_section_destroy(root, NULL); + } + else + { + usage(stderr, argv[0]); + } + return 0; +} From c92d44f2cf60fee5dcdf8aed5b0778b4b915acda Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 15 May 2014 11:53:03 +0200 Subject: [PATCH 26/27] settings: Eliminate performance warning This was useful during development, but we accept that matching \n together with %option yylineno impacts performance. --- src/libstrongswan/settings/settings_lexer.l | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libstrongswan/settings/settings_lexer.l b/src/libstrongswan/settings/settings_lexer.l index a680e71f4..d9af1531d 100644 --- a/src/libstrongswan/settings/settings_lexer.l +++ b/src/libstrongswan/settings/settings_lexer.l @@ -24,8 +24,6 @@ static void include_files(parser_helper_t *ctx); %} %option debug -%option perf-report - %option warn /* use start conditions stack */ From b3dd0168f123cd9161578dd359f34dab9355857c Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 15 May 2014 11:55:23 +0200 Subject: [PATCH 27/27] settings: Properly match } and # in include statements Found due to %option nodefault. A match for } was actually missing and # was not properly matched if it was part of an include statement on the last line of a file that did not end with a newline. --- src/libstrongswan/settings/settings_lexer.l | 22 +++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/libstrongswan/settings/settings_lexer.l b/src/libstrongswan/settings/settings_lexer.l index d9af1531d..c6546f464 100644 --- a/src/libstrongswan/settings/settings_lexer.l +++ b/src/libstrongswan/settings/settings_lexer.l @@ -82,12 +82,22 @@ static void include_files(parser_helper_t *ctx); { /* we allow all characters except #, } and spaces, they can be escaped */ <> | - \n|#.*\n | - [\t ] { - if (*yytext && yytext[strlen(yytext) - 1] == '\n') - { /* put the newline back to fix the line numbers */ - unput('\n'); - yy_set_bol(0); + [#}\n\t ] { + if (*yytext) + { + switch (yytext[0]) + { + case '\n': + /* put the newline back to fix the line numbers */ + unput('\n'); + yy_set_bol(0); + break; + case '#': + case '}': + /* these are parsed outside of this start condition */ + unput(yytext[0]); + break; + } } include_files(yyextra); yy_pop_state(yyscanner);