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.

7 files changed, 97 insertions, 22 deletions

diff --git a/man/NetworkManager.conf.xml.in b/man/NetworkManager.conf.xml.in
index e2ab2d7886..aec3bcb247 100644
--- a/man/NetworkManager.conf.xml.in
+++ b/man/NetworkManager.conf.xml.in
@@ -69,7 +69,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:
@@ -83,6 +84,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 5e3c5d8a24..666fc246ef 100644
--- a/src/nm-config.c
+++ b/src/nm-config.c
@@ -410,7 +410,7 @@ nm_config_create_keyfile ()
 	GKeyFile *keyfile;
 
 	keyfile = g_key_file_new ();
-	g_key_file_set_list_separator (keyfile, ',');
+	g_key_file_set_list_separator (keyfile, NM_CONFIG_KEYFILE_LIST_SEPARATOR);
 	return keyfile;
 }
 
@@ -452,31 +452,61 @@ read_config (GKeyFile *keyfile, const char *dirname, const char *path, GError **
 		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);
+			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, groups[g], base_key, NULL, NULL);
+				gs_free char **new_val = g_key_file_get_string_list (kf, groups[g], 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_key_file_set_value (keyfile, groups[g], base_key, combined);
-					g_free (combined);
+				if (new->len > 0) {
+					g_ptr_array_add (new, NULL);
+					g_key_file_set_string_list (keyfile, groups[g], base_key, (const char *const*) new->pdata, new->len - 1);
+
+					/* for some reason g_key_file_set_string_list() appends a trailing separator. Get rid of it again. */
+					new_value = g_key_file_get_value (keyfile, groups[g], base_key, NULL);
+					if (new_value) {
+						gsize l;
+
+						l = strlen (new_value);
+						if (l > 0 && new_value[l - 1] == NM_CONFIG_KEYFILE_LIST_SEPARATOR) {
+							new_value[l - 1] = '\0';
+							g_key_file_set_value (keyfile, groups[g], base_key, new_value);
+						}
+						g_free (new_value);
+					}
 				} 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);
+					g_key_file_remove_key (keyfile, groups[g], 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, groups[g], key, NULL);
+			g_key_file_set_value (keyfile, groups[g], key, new_value);
+			g_free (new_value);
 		}
 		g_strfreev (keys);
 	}
diff --git a/src/nm-config.h b/src/nm-config.h
index 83c4d119fa..13b5804db8 100644
--- a/src/nm-config.h
+++ b/src/nm-config.h
@@ -46,6 +46,8 @@ G_BEGIN_DECLS
 #define NM_CONFIG_DEFAULT_CONNECTIVITY_INTERVAL 300
 #define NM_CONFIG_DEFAULT_CONNECTIVITY_RESPONSE "NetworkManager is online" /* NOT LOCALIZED */
 
+#define NM_CONFIG_KEYFILE_LIST_SEPARATOR ','
+
 typedef struct NMConfigCmdLineOptions NMConfigCmdLineOptions;
 
 struct _NMConfig {
diff --git a/src/tests/config/conf.d/00-overrides.conf b/src/tests/config/conf.d/00-overrides.conf
index 0aa19d484c..170cec6907 100644
--- a/src/tests/config/conf.d/00-overrides.conf
+++ b/src/tests/config/conf.d/00-overrides.conf
@@ -9,3 +9,19 @@ a=0
 b=0
 c=0
 
+[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 b1424a4bc8..94d5642448 100644
--- a/src/tests/config/conf.d/10-more.conf
+++ b/src/tests/config/conf.d/10-more.conf
@@ -9,3 +9,5 @@ uri=http://example.net
 a=10
 b=10
 
+[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 3bbe7a96d8..c69b813fba 100644
--- a/src/tests/config/test-config.c
+++ b/src/tests/config/test-config.c
@@ -328,6 +328,26 @@ test_config_confdir (void)
 	g_assert_cmpstr (value, ==, "0");
 	g_free (value);
 
+	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);
 }