config: simplify no-auto-default list handling and sort entriesth/no-auto-default-by-ifname

- don't let no_auto_default_from_file() do any preprocessing of
  the lines that it reads. It merely splits the lines at '\n'
  and utf8safe-unescapes them.
  This was previously duplicated also by NMConfigData's property
  setter. We don't need to do it twice.

- sort the lines. This makes the entire handling O(n*ln(n)) instead
  of O(n^2). Also, sorting effectively normalizes the content, and
  it's desirable to have one true representation of what we write.

3 files changed, 54 insertions, 68 deletions

diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c
index 4c94ddb8af..c8a253a668 100644
--- a/shared/nm-glib-aux/nm-shared-utils.c
+++ b/shared/nm-glib-aux/nm-shared-utils.c
@@ -1364,6 +1364,8 @@ _nm_utils_strv_cleanup (char **strv,
 		return strv;
 
 	if (strip_whitespace) {
+		/* we only modify the strings pointed to by @strv if @strip_whitespace is
+		 * requested. Otherwise, the strings themselves are untouched. */
 		for (i = 0; strv[i]; i++)
 			g_strstrip (strv[i]);
 	}
diff --git a/src/nm-config-data.c b/src/nm-config-data.c
index 77368b4dad..5655c8fd0e 100644
--- a/src/nm-config-data.c
+++ b/src/nm-config-data.c
@@ -1623,23 +1623,28 @@ set_property (GObject *object,
 	case PROP_NO_AUTO_DEFAULT:
 		/* construct-only */
 		{
-			const char *const*value_arr = g_value_get_boxed (value);
-			gsize i, j = 0;
+			const char *const*value_arr_orig = g_value_get_boxed (value);
+			gs_free const char **value_arr = NULL;
+			GSList *specs = NULL;
+			gsize i, j;
 			gsize len;
 
-			len = NM_PTRARRAY_LEN (value_arr);
+			len = NM_PTRARRAY_LEN (value_arr_orig);
 
-			priv->no_auto_default.arr = g_new (char *, len + 1);
-			priv->no_auto_default.specs = NULL;
+			/* sort entries, remove duplicates and empty words. */
+			value_arr =   len == 0
+			            ? NULL
+			            : nm_memdup (value_arr_orig, sizeof (const char *) * (len + 1));
+			nm_utils_strv_sort (value_arr, len);
+			_nm_utils_strv_cleanup ((char **) value_arr, FALSE, TRUE, TRUE);
 
+			len = NM_PTRARRAY_LEN (value_arr);
+			j = 0;
 			for (i = 0; i < len; i++) {
 				const char *s = value_arr[i];
 				gboolean is_mac;
 				char *spec;
 
-				if (!s[0])
-					continue;
-
 				if (NM_STR_HAS_PREFIX (s, NM_MATCH_SPEC_INTERFACE_NAME_TAG"="))
 					is_mac = FALSE;
 				else if (nm_utils_hwaddr_valid (s, -1))
@@ -1649,19 +1654,16 @@ set_property (GObject *object,
 					continue;
 				}
 
-				if (nm_utils_strv_find_first (priv->no_auto_default.arr, j, s) >= 0)
-					continue;
+				value_arr[j++] = s;
 
 				spec = is_mac
 				       ? g_strdup_printf (NM_MATCH_SPEC_MAC_TAG"%s", s)
 				       : g_strdup (s);
-
-				priv->no_auto_default.arr[j++] = g_strdup (s);
-				priv->no_auto_default.specs = g_slist_prepend (priv->no_auto_default.specs, spec);
+				specs = g_slist_prepend (specs, spec);
 			}
-			nm_assert (j <= len);
-			priv->no_auto_default.arr[j++] = NULL;
-			priv->no_auto_default.specs = g_slist_reverse (priv->no_auto_default.specs);
+
+			priv->no_auto_default.arr = nm_utils_strv_dup (value_arr, j);
+			priv->no_auto_default.specs = g_slist_reverse (specs);
 		}
 		break;
 	default:
diff --git a/src/nm-config.c b/src/nm-config.c
index f92e5a5846..d12798143f 100644
--- a/src/nm-config.c
+++ b/src/nm-config.c
@@ -349,9 +349,8 @@ nm_config_get_first_start (NMConfig *config)
 static char **
 no_auto_default_from_file (const char *no_auto_default_file)
 {
-	gs_free const char **list = NULL;
 	gs_free char *data = NULL;
-	gsize l = 0;
+	const char **list = NULL;
 	gsize i;
 
 	if (   no_auto_default_file
@@ -359,47 +358,21 @@ no_auto_default_from_file (const char *no_auto_default_file)
 		list = nm_utils_strsplit_set (data, "\n");
 
 	if (list) {
-		for (i = 0; list[i]; i++) {
-			gs_free char *s_to_free = NULL;
-			const char *s = list[i];
-
-			if (!s[0])
-				continue;
-
-			s = nm_utils_str_utf8safe_unescape (s, &s_to_free);
-
-			if (   !NM_STR_HAS_PREFIX (s, NM_MATCH_SPEC_INTERFACE_NAME_TAG"=")
-			    && !nm_utils_hwaddr_valid (s, -1)) {
-				/* Maybe we shouldn't pre-validate the device specs that we read
-				 * from the file. After all, nm_match_spec_*() API silently ignores
-				 * all unknown value. However, lets just be strict here for now
-				 * and only accept what we also write. */
-				continue;
-			}
-
-			if (nm_utils_strv_find_first ((char **) list, l, s) >= 0)
-				continue;
-
-			list[l++] =    g_steal_pointer (&s_to_free)
-			            ?: g_strdup (s);
-		}
+		for (i = 0; list[i]; i++)
+			list[i] = nm_utils_str_utf8safe_unescape_cp (list[i]);
 	}
 
-	if (l == 0)
-		return NULL;
-
-	/* the strings from [0..l-1] are already cloned. We just need to
-	 * clone the outer list (and NULL terminate).  */
-	list[l] = NULL;
-	return nm_memdup (list, sizeof (list[0]) * (l + 1));
+	/* The returned buffer here is not at all compact. That means, it has additional
+	 * memory allocations and is larger than needed. That means, you should not keep
+	 * this result around, only process it further and free it. */
+	return (char **) list;
 }
 
 static gboolean
 no_auto_default_to_file (const char *no_auto_default_file, const char *const*no_auto_default, GError **error)
 {
-	GString *data;
-	gboolean success;
-	guint i;
+	nm_auto_free_gstring GString *data = NULL;
+	gsize i;
 
 	data = g_string_new ("");
 	for (i = 0; no_auto_default && no_auto_default[i]; i++) {
@@ -413,9 +386,7 @@ no_auto_default_to_file (const char *no_auto_default_file, const char *const*no_
 		g_string_append (data, s);
 		g_string_append_c (data, '\n');
 	}
-	success = g_file_set_contents (no_auto_default_file, data->str, data->len, error);
-	g_string_free (data, TRUE);
-	return success;
+	return  g_file_set_contents (no_auto_default_file, data->str, data->len, error);
 }
 
 gboolean
@@ -444,9 +415,10 @@ nm_config_set_no_auto_default_for_device (NMConfig *self, NMDevice *device)
 	const char *hw_address;
 	const char *spec;
 	const char *const*no_auto_default_current;
-	GPtrArray *no_auto_default_new = NULL;
+	gs_free const char **no_auto_default_new = NULL;
 	gboolean is_fake;
-	guint i;
+	gsize len;
+	gssize idx;
 
 	g_return_if_fail (NM_IS_CONFIG (self));
 	g_return_if_fail (NM_IS_DEVICE (device));
@@ -476,28 +448,38 @@ nm_config_set_no_auto_default_for_device (NMConfig *self, NMDevice *device)
 
 	no_auto_default_current = nm_config_data_get_no_auto_default (priv->config_data);
 
-	if (nm_utils_strv_find_first ((char **) no_auto_default_current, -1, spec) >= 0) {
+	len = NM_PTRARRAY_LEN (no_auto_default_current);
+
+	idx = nm_utils_ptrarray_find_binary_search ((gconstpointer *) no_auto_default_current,
+	                                            len,
+	                                            spec,
+	                                            nm_strcmp_with_data,
+	                                            NULL,
+	                                            NULL,
+	                                            NULL);
+	if (idx >= 0) {
 		/* @spec is already blocked. We don't have to update our in-memory representation.
 		 * Maybe we should write to no_auto_default_file anew, but let's save that too. */
 		return;
 	}
 
-	no_auto_default_new = g_ptr_array_new ();
-	for (i = 0; no_auto_default_current && no_auto_default_current[i]; i++)
-		g_ptr_array_add (no_auto_default_new, (char *) no_auto_default_current[i]);
-	g_ptr_array_add (no_auto_default_new, (char *) spec);
-	g_ptr_array_add (no_auto_default_new, NULL);
+	idx = ~idx;
 
-	if (!no_auto_default_to_file (priv->no_auto_default_file, (const char *const*) no_auto_default_new->pdata, &error)) {
+	no_auto_default_new = g_new (const char *, len + 2);
+	if (idx > 0)
+		memcpy (no_auto_default_new, no_auto_default_current, sizeof (const char *) * idx);
+	no_auto_default_new[idx] = spec;
+	if (idx < len)
+		memcpy (&no_auto_default_new[idx + 1], &no_auto_default_current[idx], sizeof (const char *) * (len - idx));
+	no_auto_default_new[len + 1] = NULL;
+
+	if (!no_auto_default_to_file (priv->no_auto_default_file, no_auto_default_new, &error)) {
 		_LOGW ("Could not update no-auto-default.state file: %s",
 		       error->message);
 		g_error_free (error);
 	}
 
-	new_data = nm_config_data_new_update_no_auto_default (priv->config_data, (const char *const*) no_auto_default_new->pdata);
-
-	/* unref no_auto_default_set here. Note that _set_config_data() probably invalidates the content of the array. */
-	g_ptr_array_unref (no_auto_default_new);
+	new_data = nm_config_data_new_update_no_auto_default (priv->config_data, no_auto_default_new);
 
 	_set_config_data (self, new_data, NM_CONFIG_CHANGE_CAUSE_NO_AUTO_DEFAULT);
 }