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.

diff --git a/src/libstrongswan/settings/settings.c b/src/libstrongswan/settings/settings.c
index c6ebe894a273bef5aa9768f47ec55ae9a7962b0c..e467d88214ea20cd7a23fbfc476c9f873727fcc4 100644 (file)
--- 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, &current);
-               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 e22d95165b50e9461797af563ddf4ed3dafb6eee..253d06808a37b9aa45d2ced6d8f9fa7d5e8a8de6 100644 (file)
--- 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, &current);
+               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**)&section))
+       if (purge)
+       {       /* remove sections and settings in base not found in extension */
+               enumerator = array_create_enumerator(base->sections_order);
+               while (enumerator->enumerate(enumerator, (void**)&section))
+               {
+                       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, &section))
        {
                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 fdca4a58028b62bcca744d185e071fefbc735298..67299d8e76a2bfdd190dd0d8c54fe960a309da02 100644 (file)
--- 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 f58c54a6decc582a6f2a852f46210a690971bb99..77fef12ad44018f62ed3a65b844313171720c160 100644 (file)
--- 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();