platform: refactor nm_platform_link_get_all() to return GPtrArray

Instead of doing a full clone, return a pointer array (with references
owned). The NMPlatformLink instances are now immutable.

5 files changed, 74 insertions, 61 deletions

diff --git a/src/nm-manager.c b/src/nm-manager.c
index 90ad66403e..dd168fd0be 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -35,6 +35,7 @@
 #include "devices/nm-device.h"
 #include "devices/nm-device-generic.h"
 #include "platform/nm-platform.h"
+#include "platform/nmp-object.h"
 #include "nm-hostname-manager.h"
 #include "nm-rfkill-manager.h"
 #include "dhcp/nm-dhcp-manager.h"
@@ -2478,8 +2479,7 @@ platform_link_cb (NMPlatform *platform,
 static void
 platform_query_devices (NMManager *self)
 {
-	GArray *links_array;
-	NMPlatformLink *links;
+	gs_unref_ptrarray GPtrArray *links = NULL;
 	int i;
 	gboolean guess_assume;
 	const char *order;
@@ -2489,21 +2489,21 @@ platform_query_devices (NMManager *self)
 	                                         NM_CONFIG_KEYFILE_GROUP_MAIN,
 	                                         NM_CONFIG_KEYFILE_KEY_MAIN_SLAVES_ORDER,
 	                                         NM_CONFIG_GET_VALUE_STRIP);
-	links_array = nm_platform_link_get_all (NM_PLATFORM_GET, !nm_streq0 (order, "index"));
-	links = (NMPlatformLink *) links_array->data;
-	for (i = 0; i < links_array->len; i++) {
+	links = nm_platform_link_get_all (NM_PLATFORM_GET, !nm_streq0 (order, "index"));
+	if (!links)
+		return;
+	for (i = 0; i < links->len; i++) {
+		const NMPlatformLink *link = NMP_OBJECT_CAST_LINK ((const NMPObject *) links->pdata[i]);
 		gs_free NMConfigDeviceStateData *dev_state = NULL;
 
-		dev_state = nm_config_device_state_load (links[i].ifindex);
+		dev_state = nm_config_device_state_load (link->ifindex);
 
 		platform_link_added (self,
-		                     links[i].ifindex,
-		                     &links[i],
+		                     link->ifindex,
+		                     link,
 		                     guess_assume && (!dev_state || !dev_state->connection_uuid),
 		                     dev_state);
 	}
-
-	g_array_unref (links_array);
 }
 
 static void
diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c
index 85a7b7c175..73e508ff00 100644
--- a/src/platform/nm-platform.c
+++ b/src/platform/nm-platform.c
@@ -464,8 +464,8 @@ _link_get_all_presort (gconstpointer  p_a,
                        gconstpointer  p_b,
                        gpointer       sort_by_name)
 {
-	const NMPlatformLink *a = p_a;
-	const NMPlatformLink *b = p_b;
+	const NMPlatformLink *a = NMP_OBJECT_CAST_LINK (*((const NMPObject **) p_a));
+	const NMPlatformLink *b = NMP_OBJECT_CAST_LINK (*((const NMPObject **) p_b));
 
 	/* Loopback always first */
 	if (a->ifindex == 1)
@@ -487,38 +487,47 @@ _link_get_all_presort (gconstpointer  p_a,
 
 /**
  * nm_platform_link_get_all:
- * self: platform instance
+ * @self: platform instance
+ * @sort_by_name: whether to sort by name or ifindex.
  *
  * Retrieve a snapshot of configuration for all links at once. The result is
- * owned by the caller and should be freed with g_array_unref().
+ * owned by the caller and should be freed with g_ptr_array_unref().
  */
-GArray *
+GPtrArray *
 nm_platform_link_get_all (NMPlatform *self, gboolean sort_by_name)
 {
-	GArray *links, *result;
-	guint i, j, nresult;
-	GHashTable *unseen;
-	NMPlatformLink *item;
+	gs_unref_ptrarray GPtrArray *links = NULL;
+	GPtrArray *result;
+	guint i, nresult;
+	gs_unref_hashtable GHashTable *unseen = NULL;
+	const NMPlatformLink *item;
 	NMPLookup lookup;
 
 	_CHECK_SELF (self, klass, NULL);
 
 	nmp_lookup_init_obj_type (&lookup, NMP_OBJECT_TYPE_LINK);
-	links = nmp_cache_lookup_to_array (nmp_cache_lookup (nm_platform_get_cache (self), &lookup),
-	                                   NMP_OBJECT_TYPE_LINK,
-	                                   TRUE);
+	links = nm_dedup_multi_objs_to_ptr_array_head (nm_platform_lookup (self, &lookup),
+	                                               NULL, NULL);
+	if (!links)
+		return NULL;
 
-	if (!links || links->len == 0)
-		return links;
+	for (i = 0; i < links->len; ) {
+		if (!nmp_object_is_visible (links->pdata[i]))
+			g_ptr_array_remove_index_fast (links, i);
+		else
+			i++;
+	}
+
+	if (links->len == 0)
+		return NULL;
 
 	/* first sort the links by their ifindex or name. Below we will sort
 	 * further by moving children/slaves to the end. */
-	g_array_sort_with_data (links, _link_get_all_presort, GINT_TO_POINTER (sort_by_name));
+	g_ptr_array_sort_with_data (links, _link_get_all_presort, GINT_TO_POINTER (sort_by_name));
 
 	unseen = g_hash_table_new (g_direct_hash, g_direct_equal);
 	for (i = 0; i < links->len; i++) {
-		item = &g_array_index (links, NMPlatformLink, i);
-
+		item = NMP_OBJECT_CAST_LINK ((const NMPObject *) links->pdata[i]);
 		nm_assert (item->ifindex > 0);
 		if (!nm_g_hash_table_insert (unseen, GINT_TO_POINTER (item->ifindex), NULL))
 			nm_assert_not_reached ();
@@ -527,7 +536,7 @@ nm_platform_link_get_all (NMPlatform *self, gboolean sort_by_name)
 #if NM_MORE_ASSERTS
 	/* Ensure that link_get_all returns a consistent and valid result. */
 	for (i = 0; i < links->len; i++) {
-		item = &g_array_index (links, NMPlatformLink, i);
+		item = NMP_OBJECT_CAST_LINK ((const NMPObject *) links->pdata[i]);
 
 		if (!item->ifindex)
 			continue;
@@ -547,50 +556,52 @@ nm_platform_link_get_all (NMPlatform *self, gboolean sort_by_name)
 #endif
 
 	/* Re-order the links list such that children/slaves come after all ancestors */
-	nresult = g_hash_table_size (unseen);
-	result = g_array_sized_new (TRUE, TRUE, sizeof (NMPlatformLink), nresult);
-	g_array_set_size (result, nresult);
+	nm_assert (g_hash_table_size (unseen) == links->len);
+	nresult = links->len;
+	result = g_ptr_array_new_full (nresult, (GDestroyNotify) nmp_object_unref);
 
-	j = 0;
-	do {
+	while (TRUE) {
 		gboolean found_something = FALSE;
 		guint first_idx = G_MAXUINT;
 
 		for (i = 0; i < links->len; i++) {
-			item = &g_array_index (links, NMPlatformLink, i);
+			item = NMP_OBJECT_CAST_LINK ((const NMPObject *) links->pdata[i]);
 
-			if (!item->ifindex)
+			if (!item)
 				continue;
 
-			if (first_idx == G_MAXUINT)
-				first_idx = i;
-
 			g_assert (g_hash_table_contains (unseen, GINT_TO_POINTER (item->ifindex)));
 
 			if (item->master > 0 && g_hash_table_contains (unseen, GINT_TO_POINTER (item->master)))
-				continue;
+				goto skip;
 			if (item->parent > 0 && g_hash_table_contains (unseen, GINT_TO_POINTER (item->parent)))
-				continue;
+				goto skip;
 
 			g_hash_table_remove (unseen, GINT_TO_POINTER (item->ifindex));
-			g_array_index (result, NMPlatformLink, j++) = *item;
-			item->ifindex = 0;
+			g_ptr_array_add (result, links->pdata[i]);
+			links->pdata[i] = NULL;
 			found_something = TRUE;
+			continue;
+skip:
+			if (first_idx == G_MAXUINT)
+				first_idx = i;
 		}
 
-		if (!found_something) {
+		if (found_something) {
+			if (first_idx == G_MAXUINT)
+				break;
+		} else {
+			nm_assert (first_idx != G_MAXUINT);
 			/* There is a loop, pop the first (remaining) element from the list.
 			 * This can happen for veth pairs where each peer is parent of the other end. */
-			item = &g_array_index (links, NMPlatformLink, first_idx);
-
+			item = NMP_OBJECT_CAST_LINK ((const NMPObject *) links->pdata[first_idx]);
 			g_hash_table_remove (unseen, GINT_TO_POINTER (item->ifindex));
-			g_array_index (result, NMPlatformLink, j++) = *item;
-			item->ifindex = 0;
+			g_ptr_array_add (result, links->pdata[first_idx]);
+			links->pdata[first_idx] = NULL;
 		}
-	} while (j < nresult);
-
-	g_hash_table_destroy (unseen);
-	g_array_free (links, TRUE);
+		nm_assert (result->len < nresult);
+	}
+	nm_assert (result->len == nresult);
 
 	return result;
 }
diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h
index 20c8e0d9f8..6e7a8b0658 100644
--- a/src/platform/nm-platform.h
+++ b/src/platform/nm-platform.h
@@ -740,7 +740,7 @@ const NMPlatformLink *nm_platform_link_get (NMPlatform *self, int ifindex);
 const NMPlatformLink *nm_platform_link_get_by_ifname (NMPlatform *self, const char *ifname);
 const NMPlatformLink *nm_platform_link_get_by_address (NMPlatform *self, gconstpointer address, size_t length);
 
-GArray *nm_platform_link_get_all (NMPlatform *self, gboolean sort_by_name);
+GPtrArray *nm_platform_link_get_all (NMPlatform *self, gboolean sort_by_name);
 NMPlatformError nm_platform_link_dummy_add (NMPlatform *self, const char *name, const NMPlatformLink **out_link);
 NMPlatformError nm_platform_link_bridge_add (NMPlatform *self, const char *name, const void *address, size_t address_len, const NMPlatformLink **out_link);
 NMPlatformError nm_platform_link_bond_add (NMPlatform *self, const char *name, const NMPlatformLink **out_link);
diff --git a/src/platform/tests/test-common.c b/src/platform/tests/test-common.c
index eec92af1fd..62c2c83fd1 100644
--- a/src/platform/tests/test-common.c
+++ b/src/platform/tests/test-common.c
@@ -156,9 +156,9 @@ link_callback (NMPlatform *platform, int obj_type_i, int ifindex, NMPlatformLink
 {
 	const NMPObjectType obj_type = obj_type_i;
 	const NMPlatformSignalChangeType change_type = change_type_i;
-	GArray *links;
-	NMPlatformLink *cached;
-	int i;
+	NMPLookup lookup;
+	NMDedupMultiIter iter;
+	const NMPlatformLink *cached;
 
 	g_assert_cmpint (obj_type, ==, NMP_OBJECT_TYPE_LINK);
 	g_assert (received);
@@ -188,19 +188,21 @@ link_callback (NMPlatform *platform, int obj_type_i, int ifindex, NMPlatformLink
 
 	/* Check the data */
 	g_assert (received->ifindex > 0);
-	links = nm_platform_link_get_all (NM_PLATFORM_GET, TRUE);
-	for (i = 0; i < links->len; i++) {
-		cached = &g_array_index (links, NMPlatformLink, i);
+
+	nmp_lookup_init_obj_type (&lookup, NMP_OBJECT_TYPE_LINK);
+	nmp_cache_iter_for_each_link (&iter,
+	                              nm_platform_lookup (platform, &lookup),
+	                              &cached) {
+		if (!nmp_object_is_visible (NMP_OBJECT_UP_CAST (cached)))
+			continue;
 		if (cached->ifindex == received->ifindex) {
 			g_assert_cmpint (nm_platform_link_cmp (cached, received), ==, 0);
 			g_assert (!memcmp (cached, received, sizeof (*cached)));
 			if (data->change_type == NM_PLATFORM_SIGNAL_REMOVED)
 				g_error ("Deleted link still found in the local cache.");
-			g_array_unref (links);
 			return;
 		}
 	}
-	g_array_unref (links);
 
 	if (data->change_type != NM_PLATFORM_SIGNAL_REMOVED)
 		g_error ("Added/changed link not found in the local cache.");
diff --git a/src/platform/tests/test-general.c b/src/platform/tests/test-general.c
index e772662c37..342aa0d610 100644
--- a/src/platform/tests/test-general.c
+++ b/src/platform/tests/test-general.c
@@ -44,7 +44,7 @@ static void
 test_link_get_all (void)
 {
 	gs_unref_object NMPlatform *platform = NULL;
-	gs_unref_array GArray *links = NULL;
+	gs_unref_ptrarray GPtrArray *links = NULL;
 
 	platform = nm_linux_platform_new (TRUE, NM_PLATFORM_NETNS_SUPPORT_DEFAULT);