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.

6 files changed, 87 insertions, 23 deletions

diff --git a/man/NetworkManager.conf.xml.in b/man/NetworkManager.conf.xml.in
index 83143eecaf..e2501c8464 100644
--- a/man/NetworkManager.conf.xml.in
+++ b/man/NetworkManager.conf.xml.in
@@ -62,7 +62,8 @@ Copyright 2010 - 2014 Red Hat, Inc.
     <para>
       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 <xref linkend="device-spec"/>
+      below.
     </para>
     <para>
       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:
       <programlisting>
 	plugins+=another-plugin
+	plugins-=remove-me
       </programlisting>
     </para>
   </refsect1>
diff --git a/src/nm-config.c b/src/nm-config.c
index 509ea992d7..f2e112c7bd 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;
-
-			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);
-
-				if (old_val && *old_val) {
-					char *combined = g_strconcat (old_val, ",", new_val, NULL);
-
-					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);
+			const char *key;
+			char *new_value;
+			char last_char;
+			gsize key_len;
+
+			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;
+
+				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_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 9057f26850..8c4043cf51 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);
 }