platform: no longer expose udi field in NMPlatformLink

The @udi field is not a static string, so any user of a NMPlatformLink
instance must make sure not to use the field beyond the lifetime of the
NMPlatformLink instance.
As we pass on the platform link instance during platform changed events,
this is hard to ensure for the subscriber of the signal -- because a
call back into platform could invalidate/modify the object.

Just not expose this field as part of the link instance. The few callers
who actually needed it should instead call nm_platform_get_uid(). With
that, the lifetime of the returned 'const char *' pointer is clearly
defined.

src/devices/nm-device-ethernet.c

@@ -1559,7 +1559,7 @@ link_changed (NMDevice *device, NMPlatformLink *info)
 	NMDeviceEthernetPrivate *priv = NM_DEVICE_ETHERNET_GET_PRIVATE (self);
 
 	NM_DEVICE_CLASS (nm_device_ethernet_parent_class)->link_changed (device, info);
-	if (!priv->subchan1 && info->udi)
+	if (!priv->subchan1 && info->initialized)
 		_update_s390_subchannels (self);
 }
 

src/devices/nm-device.c

@@ -1341,11 +1341,13 @@ device_link_changed (NMDevice *self, NMPlatformLink *info)
 	NMUtilsIPv6IfaceId token_iid;
 	gboolean ip_ifname_changed = FALSE;
 	gboolean platform_unmanaged = FALSE;
+	const char *udi;
 
-	if (info->udi && g_strcmp0 (info->udi, priv->udi)) {
+	udi = nm_platform_link_get_udi (NM_PLATFORM_GET, info->ifindex);
+	if (udi && g_strcmp0 (udi, priv->udi)) {
 		/* Update UDI to what udev gives us */
 		g_free (priv->udi);
-		priv->udi = g_strdup (info->udi);
+		priv->udi = g_strdup (udi);
 		g_object_notify (G_OBJECT (self), NM_DEVICE_UDI);
 	}
 
@@ -8938,7 +8940,7 @@ set_property (GObject *object, guint prop_id,
 		platform_device = g_value_get_pointer (value);
 		if (platform_device) {
 			g_free (priv->udi);
-			priv->udi = g_strdup (platform_device->udi);
+			priv->udi = g_strdup (nm_platform_link_get_udi (NM_PLATFORM_GET, platform_device->ifindex));
 			g_free (priv->iface);
 			priv->iface = g_strdup (platform_device->name);
 			priv->ifindex = platform_device->ifindex;

src/platform/nm-fake-platform.c

@@ -129,7 +129,7 @@ link_init (NMFakePlatformLink *device, int ifindex, int type, const char *name)
 	device->link.type = type;
 	device->link.kind = type_to_type_name (type);
 	device->link.driver = type_to_type_name (type);
-	device->link.udi = device->udi = g_strdup_printf ("fake:%d", ifindex);
+	device->udi = g_strdup_printf ("fake:%d", ifindex);
 	device->link.initialized = TRUE;
 	device->ip6_lladdr = *nmtst_inet6_from_string (ip6_lladdr);
 	if (name)
@@ -543,6 +543,16 @@ link_get_dev_id (NMPlatform *platform, int ifindex)
 	return 0;
 }
 
+static const char *
+link_get_udi (NMPlatform *platform, int ifindex)
+{
+	NMFakePlatformLink *device = link_get (platform, ifindex);
+
+	if (!device)
+		return NULL;
+	return device->udi;
+}
+
 static gboolean
 link_get_wake_on_lan (NMPlatform *platform, int ifindex)
 {
@@ -1511,6 +1521,8 @@ nm_fake_platform_class_init (NMFakePlatformClass *klass)
 	platform_class->link_get_type_name = link_get_type_name;
 	platform_class->link_get_unmanaged = link_get_unmanaged;
 
+	platform_class->link_get_udi = link_get_udi;
+
 	platform_class->link_set_up = link_set_up;
 	platform_class->link_set_down = link_set_down;
 	platform_class->link_set_arp = link_set_arp;

src/platform/nm-linux-platform.c

@@ -2950,6 +2950,18 @@ link_get_ipv6_token (NMPlatform *platform, int ifindex, NMUtilsIPv6IfaceId *iid)
 	return TRUE;
 }
 
+static const char *
+link_get_udi (NMPlatform *platform, int ifindex)
+{
+	const NMPObject *obj = cache_lookup_link (platform, ifindex);
+
+	if (   !obj
+	    || !obj->_link.netlink.is_in_netlink
+	    || !obj->_link.udev.device)
+		return NULL;
+	return g_udev_device_get_sysfs_path (obj->_link.udev.device);
+}
+
 static gboolean
 link_get_user_ipv6ll_enabled (NMPlatform *platform, int ifindex)
 {
@@ -4979,6 +4991,7 @@ nm_linux_platform_class_init (NMLinuxPlatformClass *klass)
 	platform_class->link_is_connected = link_is_connected;
 	platform_class->link_uses_arp = link_uses_arp;
 
+	platform_class->link_get_udi = link_get_udi;
 	platform_class->link_get_ipv6_token = link_get_ipv6_token;
 
 	platform_class->link_get_user_ipv6ll_enabled = link_get_user_ipv6ll_enabled;

src/platform/nm-platform.c

@@ -915,6 +915,18 @@ nm_platform_link_get_ipv6_token (NMPlatform *self, int ifindex, NMUtilsIPv6Iface
 	return FALSE;
 }
 
+const char *
+nm_platform_link_get_udi (NMPlatform *self, int ifindex)
+{
+	_CHECK_SELF (self, klass, FALSE);
+	reset_error (self);
+
+	g_return_val_if_fail (ifindex >= 0, NULL);
+
+	if (klass->link_get_udi)
+		return klass->link_get_udi (self, ifindex);
+	return NULL;
+}
 
 /**
  * nm_platform_link_get_user_ip6vll_enabled:
@@ -2533,7 +2545,7 @@ nm_platform_link_to_string (const NMPlatformLink *link)
 	            "%s%s" /* addr */
 	            "%s%s" /* inet6_token */
 	            "%s%s" /* driver */
-	            "%s%s", /* udi */
+	            ,
 	            link->ifindex,
 	            link->name,
 	            parent,
@@ -2552,9 +2564,7 @@ nm_platform_link_to_string (const NMPlatformLink *link)
 	            str_inet6_token ? " inet6token " : "",
 	            str_inet6_token ? str_inet6_token : "",
 	            link->driver ? " driver " : "",
-	            link->driver ? link->driver : "",
-	            link->udi ? " udi " : "",
-	            link->udi ? link->udi : "");
+	            link->driver ? link->driver : "");
 	g_string_free (str_flags, TRUE);
 	return _nm_platform_to_string_buffer;
 }
@@ -2854,11 +2864,6 @@ nm_platform_link_cmp (const NMPlatformLink *a, const NMPlatformLink *b)
 	_CMP_FIELD (a, b, inet6_addr_gen_mode_inv);
 	_CMP_FIELD (a, b, inet6_token.is_valid);
 	_CMP_FIELD_STR_INTERNED (a, b, kind);
-
-	/* udi is not an interned string, but NMRefString. Hence,
-	 * do a pointer comparison first. */
-	_CMP_FIELD_STR_INTERNED (a, b, udi);
-
 	_CMP_FIELD_STR_INTERNED (a, b, driver);
 	if (a->addr.len)
 		_CMP_FIELD_MEMCMP_LEN (a, b, addr.data, a->addr.len);

src/platform/nm-platform.h

@@ -95,12 +95,6 @@ struct _NMPlatformLink {
 	/* NMPlatform initializes this field with a static string. */
 	const char *kind;
 
-	/* Beware: NMPlatform initializes this string with an allocated string
-	 * (NMRefString). Handle it properly (i.e. don't keep a reference to it
-	 * without incrementing the ref-counter).
-	 * This property depends on @initialized. */
-	const char *udi;
-
 	/* NMPlatform initializes this field with a static string. */
 	const char *driver;
 
@@ -438,6 +432,7 @@ typedef struct {
 	gboolean (*link_is_connected) (NMPlatform *, int ifindex);
 	gboolean (*link_uses_arp) (NMPlatform *, int ifindex);
 
+	const char *(*link_get_udi) (NMPlatform *self, int ifindex);
 	gboolean (*link_get_ipv6_token) (NMPlatform *, int ifindex, NMUtilsIPv6IfaceId *iid);
 
 	gboolean (*link_get_user_ipv6ll_enabled) (NMPlatform *, int ifindex);
@@ -627,6 +622,7 @@ gboolean nm_platform_link_is_connected (NMPlatform *self, int ifindex);
 gboolean nm_platform_link_uses_arp (NMPlatform *self, int ifindex);
 
 gboolean nm_platform_link_get_ipv6_token (NMPlatform *self, int ifindex, NMUtilsIPv6IfaceId *iid);
+const char *nm_platform_link_get_udi (NMPlatform *self, int ifindex);
 
 gboolean nm_platform_link_get_user_ipv6ll_enabled (NMPlatform *self, int ifindex);
 gboolean nm_platform_link_set_user_ipv6ll_enabled (NMPlatform *self, int ifindex, gboolean enabled);

src/platform/nmp-object.c

@@ -132,7 +132,6 @@ void
 _nmp_object_fixup_link_udev_fields (NMPObject *obj, gboolean use_udev)
 {
 	const char *driver = NULL;
-	const char *udi = NULL;
 	gboolean initialized = FALSE;
 
 	nm_assert (NMP_OBJECT_GET_TYPE (obj) == OBJECT_TYPE_LINK);
@@ -145,10 +144,9 @@ _nmp_object_fixup_link_udev_fields (NMPObject *obj, gboolean use_udev)
 		driver = _link_get_driver (obj->_link.udev.device,
 		                           obj->link.kind,
 		                           obj->link.name);
-		if (obj->_link.udev.device) {
-			udi = g_udev_device_get_sysfs_path (obj->_link.udev.device);
+		if (obj->_link.udev.device)
 			initialized = TRUE;
-		} else if (!use_udev) {
+		else if (!use_udev) {
 			/* If we don't use udev, we immediately mark the link as initialized.
 			 *
 			 * For that, we consult @use_udev argument, that is cached via
@@ -165,7 +163,6 @@ _nmp_object_fixup_link_udev_fields (NMPObject *obj, gboolean use_udev)
 	}
 
 	obj->link.driver = driver;
-	obj->link.udi = nm_ref_string_replace (obj->link.udi, udi);
 	obj->link.initialized = initialized;
 }
 
@@ -225,7 +222,6 @@ nmp_object_unref (NMPObject *obj)
 static void
 _vt_cmd_obj_dispose_link (NMPObject *obj)
 {
-	nm_ref_string_unref (obj->link.udi);
 	g_clear_object (&obj->_link.udev.device);
 }
 
@@ -587,10 +583,6 @@ _vt_cmd_obj_copy_link (NMPObject *dst, const NMPObject *src)
 		if (src->_link.udev.device)
 			g_object_ref (src->_link.udev.device);
 	}
-	if (dst->link.udi != src->link.udi) {
-		nm_ref_string_unref (dst->link.udi);
-		nm_ref_string_ref (src->link.udi);
-	}
 	dst->_link = src->_link;
 }
 
@@ -1512,8 +1504,7 @@ nmp_cache_update_netlink (NMPCache *cache, NMPObject *obj, NMPObject **out_obj,
 	 * a use-case */
 	nm_assert (NMP_OBJECT_GET_TYPE (obj) != OBJECT_TYPE_LINK ||
 	           (   !obj->_link.udev.device
-	            && !obj->link.driver
-	            && !obj->link.udi));
+	            && !obj->link.driver));
 
 	old = g_hash_table_lookup (cache->idx_main, obj);
 

src/platform/tests/dump.c

@@ -44,7 +44,7 @@ dump_interface (NMPlatformLink *link)
 	printf ("\n");
 	if (link->driver)
 		printf ("    driver: %s\n", link->driver);
-	printf ("    UDI: %s\n", link->udi);
+	printf ("    UDI: %s\n", nm_platform_link_get_udi (NM_PLATFORM_GET, link->ifindex));
 	if (!nm_platform_vlan_get_info (NM_PLATFORM_GET, link->ifindex, &vlan_parent, &vlan_id))
 		g_assert_not_reached ();
 	if (vlan_parent)