From a325abc425e34d42aa3ef790a99a1ee9936a9a41 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 7 Jun 2015 12:03:19 +0200 Subject: [PATCH] config: refactor processing of 'option+' and 'option-' config settings We have a hack to extend GKeyFile to support specifying an 'option+' key. Also add support for 'option-'. Options that make use of these modifiers can only be string lists. So do the concatenation not based on plain strings, but by treating the values as string lists. Also, don't add duplicates. (cherry picked from commit fab5c6a372092bd350460a27891136c861ea2852) --- man/NetworkManager.conf.xml.in | 4 +- src/nm-config.c | 58 +++++++++++++++-------- src/tests/config/conf.d/00-overrides.conf | 17 +++++++ src/tests/config/conf.d/10-more.conf | 4 ++ src/tests/config/conf.d/90-last.conf | 3 ++ src/tests/config/test-config.c | 20 ++++++++ 6 files changed, 85 insertions(+), 21 deletions(-) diff --git a/man/NetworkManager.conf.xml.in b/man/NetworkManager.conf.xml.in index f3011c4f73..40fa49dd7a 100644 --- a/man/NetworkManager.conf.xml.in +++ b/man/NetworkManager.conf.xml.in @@ -62,7 +62,8 @@ Copyright 2010 - 2014 Red Hat, Inc. For keys that take a list of devices as their value, you can specify devices by their MAC addresses or interface names, or - "*" to specify all devices. + "*" to specify all devices. See + below. Minimal system settings configuration file looks like this: @@ -76,6 +77,7 @@ Copyright 2010 - 2014 Red Hat, Inc. append a value to a previously-set list-valued key by doing: plugins+=another-plugin + plugins-=remove-me diff --git a/src/nm-config.c b/src/nm-config.c index 51aff75db2..f018a91819 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -527,35 +527,53 @@ read_config (GKeyFile *keyfile, const char *path, GError **error) } for (g = 0; groups[g]; g++) { - keys = g_key_file_get_keys (kf, groups[g], &nkeys, NULL); + const char *group = groups[g]; + + keys = g_key_file_get_keys (kf, group, &nkeys, NULL); if (!keys) continue; for (k = 0; keys[k]; k++) { - int len = strlen (keys[k]); - char *v; + const char *key; + char *new_value; + char last_char; + gsize key_len; - if (keys[k][len - 1] == '+') { - char *base_key = g_strndup (keys[k], len - 1); - char *old_val = g_key_file_get_value (keyfile, groups[g], base_key, NULL); - char *new_val = g_key_file_get_value (kf, groups[g], keys[k], NULL); + key = keys[k]; + g_assert (key && *key); + key_len = strlen (key); + last_char = key[key_len - 1]; + if ( key_len > 1 + && (last_char == '+' || last_char == '-')) { + gs_free char *base_key = g_strndup (key, key_len - 1); + gs_strfreev char **old_val = g_key_file_get_string_list (keyfile, group, base_key, NULL, NULL); + gs_free char **new_val = g_key_file_get_string_list (kf, group, key, NULL, NULL); + gs_unref_ptrarray GPtrArray *new = g_ptr_array_new_with_free_func (g_free); + char **iter_val; - if (old_val && *old_val) { - char *combined = g_strconcat (old_val, ",", new_val, NULL); + for (iter_val = old_val; iter_val && *iter_val; iter_val++) { + if ( last_char != '-' + || _nm_utils_strv_find_first (new_val, -1, *iter_val) < 0) + g_ptr_array_add (new, g_strdup (*iter_val)); + } + for (iter_val = new_val; iter_val && *iter_val; iter_val++) { + /* don't add duplicates. That means an "option=a,b"; "option+=a,c" results in "option=a,b,c" */ + if ( last_char == '+' + && _nm_utils_strv_find_first (old_val, -1, *iter_val) < 0) + g_ptr_array_add (new, *iter_val); + else + g_free (*iter_val); + } - g_key_file_set_value (keyfile, groups[g], base_key, combined); - g_free (combined); - } else - g_key_file_set_value (keyfile, groups[g], base_key, new_val); - - g_free (base_key); - g_free (old_val); - g_free (new_val); + if (new->len > 0) + nm_config_keyfile_set_string_list (keyfile, group, base_key, (const char *const*) new->pdata, new->len); + else + g_key_file_remove_key (keyfile, group, base_key, NULL); continue; } - g_key_file_set_value (keyfile, groups[g], keys[k], - v = g_key_file_get_value (kf, groups[g], keys[k], NULL)); - g_free (v); + new_value = g_key_file_get_value (kf, group, key, NULL); + g_key_file_set_value (keyfile, group, key, new_value); + g_free (new_value); } g_strfreev (keys); } diff --git a/src/tests/config/conf.d/00-overrides.conf b/src/tests/config/conf.d/00-overrides.conf index 0c246a02a4..cb0116edd8 100644 --- a/src/tests/config/conf.d/00-overrides.conf +++ b/src/tests/config/conf.d/00-overrides.conf @@ -31,3 +31,20 @@ ord.key07=B-1.3.07 ord.key08=B-1.3.08 ord.key09=B-1.3.09 + +[append] +val1=a,b + +val2-=VAL2 +val2=VAL2 + +val3=VAL3 +val3-=VAL3 + +val4=VAL4 +val4+=VAL4,va,vb,va,vb +val4-=VAL4,va + +val5=VAL5 +val5-=VAL5 +val5+=VAL5 diff --git a/src/tests/config/conf.d/10-more.conf b/src/tests/config/conf.d/10-more.conf index 08b73ddfdc..a1959c1948 100644 --- a/src/tests/config/conf.d/10-more.conf +++ b/src/tests/config/conf.d/10-more.conf @@ -27,3 +27,7 @@ ord.key09=C-2.3.09 # low priority and is shadowed by [connection.ord.2.1]. [connection.ord.0.1] ord.ovw01=C-0.1.ovw01 + + +[append] +val1-=b diff --git a/src/tests/config/conf.d/90-last.conf b/src/tests/config/conf.d/90-last.conf index dc1de394f1..c75dcc4710 100644 --- a/src/tests/config/conf.d/90-last.conf +++ b/src/tests/config/conf.d/90-last.conf @@ -3,3 +3,6 @@ plugins+=one,two [order] a=90 + +[append] +val1+=c,a diff --git a/src/tests/config/test-config.c b/src/tests/config/test-config.c index f12752847a..13a2aca9e1 100644 --- a/src/tests/config/test-config.c +++ b/src/tests/config/test-config.c @@ -341,6 +341,26 @@ test_config_confdir (void) ASSERT_GET_CONN_DEFAULT (config, "ord.key09", "C-2.1.09"); ASSERT_GET_CONN_DEFAULT (config, "ord.ovw01", "C-0.1.ovw01"); + value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val1", NULL); + g_assert_cmpstr (value, ==, "a,c"); + g_free (value); + + value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val2", NULL); + g_assert_cmpstr (value, ==, "VAL2"); + g_free (value); + + value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val3", NULL); + g_assert_cmpstr (value, ==, NULL); + g_free (value); + + value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val4", NULL); + g_assert_cmpstr (value, ==, "vb,vb"); + g_free (value); + + value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val5", NULL); + g_assert_cmpstr (value, ==, "VAL5"); + g_free (value); + g_object_unref (config); }