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 fab5c6a372)

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>

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);
 	}

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

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

src/tests/config/conf.d/90-last.conf

@@ -3,3 +3,6 @@ plugins+=one,two
 
 [order]
 a=90
+
+[append]
+val1+=c,a

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);
 }