core: drop nm_settings_get_best_connections() for new nm_settings_get_connections_clone()

nm_settings_get_best_connections() has only one caller: to create
the hidden-SSID list.

Instead of having a highly specialised function (that accepts 3 ways for
filtering -- one of them broken, has one hard-coded way of sorting, and
a @max_requested argument), add a more generic nm_settings_get_connections_clone()
function.

Also invert nm_settings_sort_connections(). The two callers want
to sort descending, not ascending.

5 files changed, 95 insertions, 112 deletions

diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c
index ce0c690e3..e70432ff6 100644
--- a/src/devices/wifi/nm-device-wifi.c
+++ b/src/devices/wifi/nm-device-wifi.c
@@ -1338,12 +1338,14 @@ check_scanning_allowed (NMDeviceWifi *self)
 
 static gboolean
 hidden_filter_func (NMSettings *settings,
-                    NMConnection *connection,
+                    NMSettingsConnection *connection,
                     gpointer user_data)
 {
 	NMSettingWireless *s_wifi;
 
-	s_wifi = (NMSettingWireless *) nm_connection_get_setting_wireless (connection);
+	if (!nm_connection_is_type (NM_CONNECTION (connection), NM_SETTING_WIRELESS_SETTING_NAME))
+		return FALSE;
+	s_wifi = (NMSettingWireless *) nm_connection_get_setting_wireless (NM_CONNECTION (connection));
 	return s_wifi ? nm_setting_wireless_get_hidden (s_wifi) : FALSE;
 }
 
@@ -1352,7 +1354,8 @@ build_hidden_probe_list (NMDeviceWifi *self)
 {
 	NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self);
 	guint max_scan_ssids = nm_supplicant_interface_get_max_scan_ssids (priv->sup_iface);
-	GSList *connections, *iter;
+	gs_free NMSettingsConnection **connections = NULL;
+	guint i, len;
 	GPtrArray *ssids = NULL;
 	static GByteArray *nullssid = NULL;
 
@@ -1360,28 +1363,31 @@ build_hidden_probe_list (NMDeviceWifi *self)
 	if (max_scan_ssids < 2)
 		return NULL;
 
-	/* Static wildcard SSID used for every scan */
+	connections = nm_settings_get_connections_clone (nm_device_get_settings ((NMDevice *) self),
+	                                                 &len,
+	                                                 hidden_filter_func,
+	                                                 NULL);
+	if (!connections[0])
+		return NULL;
+
+	g_qsort_with_data (connections, len, sizeof (NMSettingsConnection *), nm_settings_connection_cmp_timestamp_p_with_data, NULL);
+
+	ssids = g_ptr_array_new_full (max_scan_ssids, (GDestroyNotify) g_byte_array_unref);
+
+	/* Add wildcard SSID using a static wildcard SSID used for every scan */
 	if (G_UNLIKELY (nullssid == NULL))
 		nullssid = g_byte_array_new ();
+	g_ptr_array_add (ssids, g_byte_array_ref (nullssid));
 
-	connections = nm_settings_get_best_connections (nm_device_get_settings ((NMDevice *) self),
-	                                                max_scan_ssids - 1,
-	                                                NM_SETTING_WIRELESS_SETTING_NAME,
-	                                                NULL,
-	                                                hidden_filter_func,
-	                                                NULL);
-	if (connections && connections->data) {
-		ssids = g_ptr_array_new_full (max_scan_ssids - 1, (GDestroyNotify) g_byte_array_unref);
-		g_ptr_array_add (ssids, g_byte_array_ref (nullssid));  /* Add wildcard SSID */
-	}
-
-	for (iter = connections; iter; iter = g_slist_next (iter)) {
-		NMConnection *connection = iter->data;
+	for (i = 0; connections[i]; i++) {
 		NMSettingWireless *s_wifi;
 		GBytes *ssid;
 		GByteArray *ssid_array;
 
-		s_wifi = (NMSettingWireless *) nm_connection_get_setting_wireless (connection);
+		if (i >= max_scan_ssids - 1)
+			break;
+
+		s_wifi = (NMSettingWireless *) nm_connection_get_setting_wireless (NM_CONNECTION (connections[i]));
 		g_assert (s_wifi);
 		ssid = nm_setting_wireless_get_ssid (s_wifi);
 		g_assert (ssid);
@@ -1391,7 +1397,6 @@ build_hidden_probe_list (NMDeviceWifi *self)
 		                     g_bytes_get_size (ssid));
 		g_ptr_array_add (ssids, ssid_array);
 	}
-	g_slist_free (connections);
 
 	return ssids;
 }
diff --git a/src/nm-manager.c b/src/nm-manager.c
index 9893f7c47..3cb158e08 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -1738,7 +1738,7 @@ get_existing_connection (NMManager *self, NMDevice *device, gboolean *out_genera
 	 * When no configured connection matches the generated connection, we keep
 	 * the generated connection instead.
 	 */
-	connections = g_slist_reverse (g_slist_sort (connections, (GCompareFunc) nm_settings_connection_cmp_timestamp));
+	connections = g_slist_sort (connections, (GCompareFunc) nm_settings_connection_cmp_timestamp);
 	matched = NM_SETTINGS_CONNECTION (nm_utils_match_connection (connections,
 	                                                             connection,
 	                                                             nm_device_has_carrier (device),
diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c
index 453b3c5b3..345701a33 100644
--- a/src/settings/nm-settings-connection.c
+++ b/src/settings/nm-settings-connection.c
@@ -2163,8 +2163,8 @@ nm_settings_connection_set_flags_all (NMSettingsConnection *self, NMSettingsConn
 /*****************************************************************************/
 
 /* sorting for "best" connections.
- * The function sorts connections in ascending timestamp order.
- * That means an older connection (lower timestamp) goes before
+ * The function sorts connections in descending timestamp order.
+ * That means an older connection (lower timestamp) goes after
  * a newer one.
  */
 int
@@ -2175,18 +2175,18 @@ nm_settings_connection_cmp_timestamp (NMSettingsConnection *ac, NMSettingsConnec
 	if (ac == bc)
 		return 0;
 	if (!ac)
-		return -1;
-	if (!bc)
 		return 1;
+	if (!bc)
+		return -1;
 
 	/* In the future we may use connection priorities in addition to timestamps */
 	nm_settings_connection_get_timestamp (ac, &ats);
 	nm_settings_connection_get_timestamp (bc, &bts);
 
 	if (ats < bts)
-		return -1;
-	else if (ats > bts)
 		return 1;
+	else if (ats > bts)
+		return -1;
 	return 0;
 }
 
diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c
index 0189d842b..057d341d4 100644
--- a/src/settings/nm-settings.c
+++ b/src/settings/nm-settings.c
@@ -418,28 +418,79 @@ nm_settings_get_connections (NMSettings *self, guint *out_len)
 
 	priv = NM_SETTINGS_GET_PRIVATE (self);
 
-	if (priv->connections_cached_list) {
+	if (G_LIKELY (priv->connections_cached_list)) {
 		NM_SET_OUT (out_len, g_hash_table_size (priv->connections));
 		return priv->connections_cached_list;
 	}
 
 	l = g_hash_table_size (priv->connections);
 
-	v = g_new (NMSettingsConnection *, l + 1);
+	v = g_new (NMSettingsConnection *, (gsize) l + 1);
 
 	i = 0;
 	g_hash_table_iter_init (&iter, priv->connections);
-	while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &con))
+	while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &con)) {
+		nm_assert (i < l);
 		v[i++] = con;
-	v[i] = NULL;
-
+	}
 	nm_assert (i == l);
+	v[i] = NULL;
 
 	NM_SET_OUT (out_len, l);
 	priv->connections_cached_list = v;
 	return v;
 }
 
+/**
+ * nm_settings_get_connections_clone:
+ * @self: the #NMSetting
+ * @out_len: (allow-none): optional output argument
+ * @func: caller-supplied function for filtering connections
+ * @func_data: caller-supplied data passed to @func
+ *
+ * Returns: (transfer container) (element-type NMSettingsConnection):
+ *   an NULL terminated array of #NMSettingsConnection objects that were
+ *   filtered by @func (or all connections if no filter was specified).
+ *   The order is arbitrary.
+ *   Caller is responsible for freeing the returned array with free(),
+ *   the contained values do not need to be unrefed.
+ */
+NMSettingsConnection **
+nm_settings_get_connections_clone (NMSettings *self,
+                                   guint *out_len,
+                                   NMSettingsConnectionFilterFunc func,
+                                   gpointer func_data)
+{
+	NMSettingsConnection *const*list_cached;
+	NMSettingsConnection **list;
+	guint len, i, j;
+
+	g_return_val_if_fail (NM_IS_SETTINGS (self), NULL);
+
+	list_cached = nm_settings_get_connections (self, &len);
+
+#if NM_MORE_ASSERTS
+	nm_assert (list_cached);
+	for (i = 0; i < len; i++)
+		nm_assert (NM_IS_SETTINGS_CONNECTION (list_cached[i]));
+	nm_assert (!list_cached[i]);
+#endif
+
+	list = g_new (NMSettingsConnection *, ((gsize) len + 1));
+	if (func) {
+		for (i = 0, j = 0; i < len; i++) {
+			if (func (self, list_cached[i], func_data))
+				list[j++] = list_cached[i];
+		}
+		list[j] = NULL;
+		len = j;
+	} else
+		memcpy (list, list_cached, sizeof (list[0]) * ((gsize) len + 1));
+
+	NM_SET_OUT (out_len, len);
+	return list;
+}
+
 /* Returns a list of NMSettingsConnections.
  * The list is sorted in the order suitable for auto-connecting, i.e.
  * first go connections with autoconnect=yes and most recent timestamp.
@@ -2056,77 +2107,6 @@ nm_settings_device_removed (NMSettings *self, NMDevice *device, gboolean quittin
 
 /*****************************************************************************/
 
-/**
- * nm_settings_get_best_connections:
- * @self: the #NMSetting
- * @max_requested: if non-zero, the maximum number of connections to return
- * @ctype1: an #NMSetting base type (eg NM_SETTING_WIRELESS_SETTING_NAME) to
- *   filter connections against
- * @ctype2: a second #NMSetting base type (eg NM_SETTING_WIRELESS_SETTING_NAME)
- *   to filter connections against
- * @func: caller-supplied function for filtering connections
- * @func_data: caller-supplied data passed to @func
- *
- * Returns: a #GSList of #NMConnection objects in sorted order representing the
- *   "best" or highest-priority connections filtered by @ctype1 and/or @ctype2,
- *   and/or @func.  Caller is responsible for freeing the returned #GSList, but
- *   the contained values do not need to be unreffed.
- */
-GSList *
-nm_settings_get_best_connections (NMSettings *self,
-                                  guint max_requested,
-                                  const char *ctype1,
-                                  const char *ctype2,
-                                  NMConnectionFilterFunc func,
-                                  gpointer func_data)
-{
-	NMSettingsPrivate *priv;
-	GSList *sorted = NULL;
-	GHashTableIter iter;
-	NMSettingsConnection *connection;
-	guint added = 0;
-	guint64 oldest = 0;
-
-	g_return_val_if_fail (NM_IS_SETTINGS (self), NULL);
-
-	priv = NM_SETTINGS_GET_PRIVATE (self);
-
-	g_hash_table_iter_init (&iter, priv->connections);
-	while (g_hash_table_iter_next (&iter, NULL, (gpointer) &connection)) {
-		guint64 cur_ts = 0;
-
-		if (ctype1 && !nm_connection_is_type (NM_CONNECTION (connection), ctype1))
-			continue;
-		if (ctype2 && !nm_connection_is_type (NM_CONNECTION (connection), ctype2))
-			continue;
-		if (func && !func (self, NM_CONNECTION (connection), func_data))
-			continue;
-
-		/* Don't bother with a connection that's older than the oldest one in the list */
-		if (max_requested && added >= max_requested) {
-		    nm_settings_connection_get_timestamp (connection, &cur_ts);
-		    if (cur_ts <= oldest)
-				continue;
-		}
-
-		/* List is sorted with oldest first */
-		sorted = g_slist_insert_sorted (sorted, connection, (GCompareFunc) nm_settings_connection_cmp_timestamp);
-		added++;
-
-		if (max_requested && added > max_requested) {
-			/* Over the limit, remove the oldest one */
-			sorted = g_slist_delete_link (sorted, sorted);
-			added--;
-		}
-
-		nm_settings_connection_get_timestamp (NM_SETTINGS_CONNECTION (sorted->data), &oldest);
-	}
-
-	return g_slist_reverse (sorted);
-}
-
-/*****************************************************************************/
-
 gboolean
 nm_settings_get_startup_complete (NMSettings *self)
 {
diff --git a/src/settings/nm-settings.h b/src/settings/nm-settings.h
index 3e56bf388..09fe2d137 100644
--- a/src/settings/nm-settings.h
+++ b/src/settings/nm-settings.h
@@ -57,9 +57,9 @@
  *
  * Returns: %TRUE to allow the connection, %FALSE to ignore it
  */
-typedef gboolean (*NMConnectionFilterFunc) (NMSettings *settings,
-                                            NMConnection *connection,
-                                            gpointer func_data);
+typedef gboolean (*NMSettingsConnectionFilterFunc) (NMSettings *settings,
+                                                    NMSettingsConnection *connection,
+                                                    gpointer func_data);
 
 typedef struct _NMSettingsClass NMSettingsClass;
 
@@ -97,14 +97,12 @@ void nm_settings_add_connection_dbus (NMSettings *self,
 
 NMSettingsConnection *const* nm_settings_get_connections (NMSettings *settings, guint *out_len);
 
-GSList *nm_settings_get_connections_sorted (NMSettings *settings);
+NMSettingsConnection **nm_settings_get_connections_clone (NMSettings *self,
+                                                          guint *out_len,
+                                                          NMSettingsConnectionFilterFunc func,
+                                                          gpointer func_data);
 
-GSList *nm_settings_get_best_connections (NMSettings *self,
-                                          guint max_requested,
-                                          const char *ctype1,
-                                          const char *ctype2,
-                                          NMConnectionFilterFunc func,
-                                          gpointer func_data);
+GSList *nm_settings_get_connections_sorted (NMSettings *settings);
 
 NMSettingsConnection *nm_settings_add_connection (NMSettings *settings,
                                                   NMConnection *connection,