core: consolidate sorting of connections by autoconnect/timestamp

NMPolicy's auto_activate_device() wants to sort by autoconnect-priority,
nm_utils_cmp_connection_by_autoconnect_priority() but fallback to the default
nm_settings_connection_cmp_default(), which includes the timestamp.

Extend nm_settings_connection_cmp_default() to consider the
autoconnect-priority as well. Thus change behavior so that
nm_settings_connection_cmp_default() is the sort order that
auto_activate_device() wants. That makes sense, as
nm_settings_connection_cmp_default() already considered the
ability to autoconnect as first. Hence, it should also honor
the autoconnect priority.

When doing that, rename nm_settings_connection_cmp_default()
to nm_settings_connection_cmp_autoconnect_priority().

8 files changed, 41 insertions, 57 deletions

diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c
index 143fdfe8b9..75c44b7865 100644
--- a/src/nm-core-utils.c
+++ b/src/nm-core-utils.c
@@ -2030,15 +2030,6 @@ nm_utils_cmp_connection_by_autoconnect_priority (NMConnection *a, NMConnection *
 	return 0;
 }
 
-int
-nm_utils_cmp_connection_by_autoconnect_priority_p_with_data (gconstpointer pa, gconstpointer pb, gpointer user_data)
-{
-	nm_assert (pa);
-	nm_assert (pb);
-	return nm_utils_cmp_connection_by_autoconnect_priority (*((NMConnection **) pa),
-	                                                        *((NMConnection **) pb));
-}
-
 /*****************************************************************************/
 
 static gint64 monotonic_timestamp_offset_sec;
diff --git a/src/nm-core-utils.h b/src/nm-core-utils.h
index 82e10132b3..a1d9c2d462 100644
--- a/src/nm-core-utils.h
+++ b/src/nm-core-utils.h
@@ -273,7 +273,6 @@ GPtrArray *nm_utils_read_resolv_conf_nameservers (const char *rc_contents);
 GPtrArray *nm_utils_read_resolv_conf_dns_options (const char *rc_contents);
 
 int nm_utils_cmp_connection_by_autoconnect_priority (NMConnection *a, NMConnection *b);
-int nm_utils_cmp_connection_by_autoconnect_priority_p_with_data (gconstpointer pa, gconstpointer pb, gpointer user_data);
 
 void nm_utils_log_connection_diff (NMConnection *connection, NMConnection *diff_base, guint32 level, guint64 domain, const char *name, const char *prefix);
 
diff --git a/src/nm-manager.c b/src/nm-manager.c
index c06b5c4862..152b1701fd 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -448,7 +448,7 @@ nm_manager_get_activatable_connections (NMManager *manager, guint *out_len, gboo
 	                                                 _get_activatable_connections_filter,
 	                                                 manager);
 	if (sort && len > 1)
-		g_qsort_with_data (connections, len, sizeof (connections[0]), nm_settings_connection_cmp_default_p_with_data, NULL);
+		g_qsort_with_data (connections, len, sizeof (connections[0]), nm_settings_connection_cmp_autoconnect_priority_p_with_data, NULL);
 	NM_SET_OUT (out_len, len);
 	return connections;
 }
diff --git a/src/nm-policy.c b/src/nm-policy.c
index 0df576ffe2..7050f93281 100644
--- a/src/nm-policy.c
+++ b/src/nm-policy.c
@@ -953,19 +953,6 @@ activate_data_free (ActivateData *data)
 	g_slice_free (ActivateData, data);
 }
 
-static int
-_auto_activate_device_cmp (gconstpointer pa, gconstpointer pb, gpointer user_data)
-{
-	NMSettingsConnection *a = NM_SETTINGS_CONNECTION (*((NMSettingsConnection **) pa));
-	NMSettingsConnection *b = NM_SETTINGS_CONNECTION (*((NMSettingsConnection **) pb));
-	int i;
-
-	i = nm_utils_cmp_connection_by_autoconnect_priority (NM_CONNECTION (a), NM_CONNECTION (b));
-	if (i != 0)
-		return i;
-	return nm_settings_connection_cmp_default (a, b);
-}
-
 static void
 auto_activate_device (NMPolicy *self,
                       NMDevice *device)
@@ -988,12 +975,10 @@ auto_activate_device (NMPolicy *self,
 	if (nm_device_get_act_request (device))
 		return;
 
-	connections = nm_manager_get_activatable_connections (priv->manager, &len, FALSE);
+	connections = nm_manager_get_activatable_connections (priv->manager, &len, TRUE);
 	if (!connections[0])
 		return;
 
-	g_qsort_with_data (connections, len, sizeof (connections[0]), _auto_activate_device_cmp, NULL);
-
 	/* Find the first connection that should be auto-activated */
 	best_connection = NULL;
 	for (i = 0; i < len; i++) {
diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c
index 345701a339..4d0c3a75ac 100644
--- a/src/settings/nm-settings-connection.c
+++ b/src/settings/nm-settings-connection.c
@@ -2179,14 +2179,11 @@ nm_settings_connection_cmp_timestamp (NMSettingsConnection *ac, NMSettingsConnec
 	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 (ats > bts) ? -1 : 1;
 
-	if (ats < bts)
-		return 1;
-	else if (ats > bts)
-		return -1;
 	return 0;
 }
 
@@ -2198,35 +2195,41 @@ nm_settings_connection_cmp_timestamp_p_with_data (gconstpointer pa, gconstpointe
 }
 
 int
-nm_settings_connection_cmp_default (NMSettingsConnection *a, NMSettingsConnection *b)
+nm_settings_connection_cmp_autoconnect_priority (NMSettingsConnection *a, NMSettingsConnection *b)
 {
-	NMSettingConnection *con_a;
-	NMSettingConnection *con_b;
-	guint64 ts_a = 0, ts_b = 0;
-	gboolean can_ac_a, can_ac_b;
+	guint64 ats = 0, bts = 0;
+	int c;
+
+	if (a == b)
+		return 0;
 
-	con_a = nm_connection_get_setting_connection (NM_CONNECTION (a));
-	con_b = nm_connection_get_setting_connection (NM_CONNECTION (b));
+	/* first we compare them by their autoconnect priority. */
+	c = nm_utils_cmp_connection_by_autoconnect_priority (NM_CONNECTION (a), NM_CONNECTION (b));
+	if (c)
+		return c;
 
-	can_ac_a = !!nm_setting_connection_get_autoconnect (con_a);
-	can_ac_b = !!nm_setting_connection_get_autoconnect (con_b);
-	if (can_ac_a != can_ac_b)
-		return can_ac_a ? -1 : 1;
+	/* then by their last activation timestamp (with the more recently connected one first) */
+	nm_settings_connection_get_timestamp (a, &ats);
+	nm_settings_connection_get_timestamp (b, &bts);
+	if (ats != bts)
+		return (ats > bts) ? -1 : 1;
 
-	nm_settings_connection_get_timestamp (a, &ts_a);
-	nm_settings_connection_get_timestamp (b, &ts_b);
-	if (ts_a > ts_b)
-		return -1;
-	else if (ts_a == ts_b)
-		return 0;
-	return 1;
+	/* if they are still equal, sort them by their UUID to give them an arbitrary, but stable
+	 * order. */
+	c = g_strcmp0 (nm_connection_get_uuid (NM_CONNECTION (a)),
+	               nm_connection_get_uuid (NM_CONNECTION (b)));
+	if (c)
+		return c;
+
+	/* hm, still the same. Use their pointer value. */
+	return (a > b) ? -1 : 1;
 }
 
 int
-nm_settings_connection_cmp_default_p_with_data (gconstpointer pa, gconstpointer pb, gpointer user_data)
+nm_settings_connection_cmp_autoconnect_priority_p_with_data (gconstpointer pa, gconstpointer pb, gpointer user_data)
 {
-	return nm_settings_connection_cmp_default (*((NMSettingsConnection **) pa),
-	                                           *((NMSettingsConnection **) pb));
+	return nm_settings_connection_cmp_autoconnect_priority (*((NMSettingsConnection **) pa),
+	                                                        *((NMSettingsConnection **) pb));
 }
 
 /*****************************************************************************/
diff --git a/src/settings/nm-settings-connection.h b/src/settings/nm-settings-connection.h
index 8fe9533a86..a2d6125734 100644
--- a/src/settings/nm-settings-connection.h
+++ b/src/settings/nm-settings-connection.h
@@ -186,8 +186,8 @@ NMSettingsConnectionFlags nm_settings_connection_set_flags_all (NMSettingsConnec
 
 int nm_settings_connection_cmp_timestamp (NMSettingsConnection *ac, NMSettingsConnection *ab);
 int nm_settings_connection_cmp_timestamp_p_with_data (gconstpointer pa, gconstpointer pb, gpointer user_data);
-int nm_settings_connection_cmp_default (NMSettingsConnection *a, NMSettingsConnection *b);
-int nm_settings_connection_cmp_default_p_with_data (gconstpointer pa, gconstpointer pb, gpointer user_data);
+int nm_settings_connection_cmp_autoconnect_priority (NMSettingsConnection *a, NMSettingsConnection *b);
+int nm_settings_connection_cmp_autoconnect_priority_p_with_data (gconstpointer pa, gconstpointer pb, gpointer user_data);
 
 gboolean nm_settings_connection_get_timestamp (NMSettingsConnection *self,
                                                guint64 *out_timestamp);
diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c
index 1739a32fef..cc03a2c4d9 100644
--- a/src/settings/nm-settings.c
+++ b/src/settings/nm-settings.c
@@ -506,7 +506,7 @@ nm_settings_get_connections_sorted (NMSettings *self, guint *out_len)
 
 	connections = nm_settings_get_connections_clone (self, &len, NULL, NULL);
 	if (len > 1)
-		g_qsort_with_data (connections, len, sizeof (NMSettingsConnection *), nm_settings_connection_cmp_default_p_with_data, NULL);
+		g_qsort_with_data (connections, len, sizeof (NMSettingsConnection *), nm_settings_connection_cmp_autoconnect_priority_p_with_data, NULL);
 
 	NM_SET_OUT (out_len, len);
 	return connections;
diff --git a/src/tests/test-general.c b/src/tests/test-general.c
index f8f8fa2606..ef1a4f2b6a 100644
--- a/src/tests/test-general.c
+++ b/src/tests/test-general.c
@@ -870,6 +870,12 @@ _create_connection_autoconnect (const char *id, gboolean autoconnect, int autoco
 	return c;
 }
 
+static int
+_cmp_autoconnect_priority_p_with_data (gconstpointer pa, gconstpointer pb, gpointer user_data)
+{
+	return nm_utils_cmp_connection_by_autoconnect_priority (*((NMConnection **) pa), *((NMConnection **) pb));
+}
+
 static void
 _test_connection_sort_autoconnect_priority_one (NMConnection **list, gboolean shuffle)
 {
@@ -892,7 +898,7 @@ _test_connection_sort_autoconnect_priority_one (NMConnection **list, gboolean sh
 	}
 
 	/* sort it... */
-	g_ptr_array_sort_with_data (connections, nm_utils_cmp_connection_by_autoconnect_priority_p_with_data, NULL);
+	g_ptr_array_sort_with_data (connections, _cmp_autoconnect_priority_p_with_data, NULL);
 
 	for (i = 0; i < count; i++) {
 		if (list[i] == connections->pdata[i])