device: initialize NMDevice's hw_addr at end of object construction

hw-addr is a constuct-only property. We should not do complex stuff in the property
setter before the object is sufficiently initialized. For example, the logging
macros access nm_device_get_iface(), which might be unset at that early
point.

Instead, initialize hw_addr and hw_addr_len later, at the end of the constructor()
function.

Also, ensure that @hw_addr_len is zero iff @hw_addr is unset.

Also, ensure that we always log a message when changing/setting the
hardware address -- except when clearing it during unrealize. It's
implicit that unrealize clears the hardware address.

Also, give all related logging messages a "hw-addr:" prefix.

2 files changed, 28 insertions, 45 deletions

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 25bf01d4a6..2a9f875fef 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -2325,6 +2325,7 @@ nm_device_unrealize (NMDevice *self, gboolean remove_resources, GError **error)
 		_notify (self, PROP_UDI);
 	}
 	if (priv->hw_addr) {
+		priv->hw_addr_len = 0;
 		g_clear_pointer (&priv->hw_addr, g_free);
 		_notify (self, PROP_HW_ADDRESS);
 	}
@@ -11337,23 +11338,29 @@ nm_device_get_hw_address (NMDevice *self)
 	g_return_val_if_fail (NM_IS_DEVICE (self), NULL);
 	priv = NM_DEVICE_GET_PRIVATE (self);
 
-	return priv->hw_addr_len ? priv->hw_addr : NULL;
+	nm_assert ((!priv->hw_addr) ^ (priv->hw_addr_len > 0));
+
+	return priv->hw_addr;
 }
 
 void
 nm_device_update_hw_address (NMDevice *self)
 {
-	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
-	int ifindex = nm_device_get_ifindex (self);
+	NMDevicePrivate *priv;
+	int ifindex;
 	const guint8 *hwaddr;
 	gsize hwaddrlen = 0;
 
+	ifindex = nm_device_get_ifindex (self);
 	if (ifindex <= 0)
 		return;
 
+	priv = NM_DEVICE_GET_PRIVATE (self);
+
 	hwaddr = nm_platform_link_get_address (NM_PLATFORM_GET, ifindex, &hwaddrlen);
 
 	if (   priv->type == NM_DEVICE_TYPE_ETHERNET
+	    && hwaddr
 	    && nm_utils_hwaddr_matches (hwaddr, hwaddrlen, nm_ip_addr_zero.addr_eth, sizeof (nm_ip_addr_zero.addr_eth)))
 		hwaddrlen = 0;
 
@@ -11363,7 +11370,7 @@ nm_device_update_hw_address (NMDevice *self)
 			g_free (priv->hw_addr);
 			priv->hw_addr = nm_utils_hwaddr_ntoa (hwaddr, hwaddrlen);
 
-			_LOGD (LOGD_HW | LOGD_DEVICE, "hardware address now %s", priv->hw_addr);
+			_LOGD (LOGD_HW | LOGD_DEVICE, "hw-addr: hardware address now %s", priv->hw_addr);
 			_notify (self, PROP_HW_ADDRESS);
 		}
 	} else {
@@ -11372,7 +11379,7 @@ nm_device_update_hw_address (NMDevice *self)
 			g_clear_pointer (&priv->hw_addr, g_free);
 			priv->hw_addr_len = 0;
 			_LOGD (LOGD_HW | LOGD_DEVICE,
-			       "previous hardware address is no longer valid");
+			       "hw-addr: previous hardware address is no longer valid");
 			_notify (self, PROP_HW_ADDRESS);
 		}
 	}
@@ -11639,6 +11646,7 @@ constructor (GType type,
 	NMDevice *self;
 	NMDevicePrivate *priv;
 	const NMPlatformLink *pllink;
+	guint count;
 
 	klass = G_OBJECT_CLASS (nm_device_parent_class);
 	object = klass->constructor (type, n_construct_params, construct_params);
@@ -11648,7 +11656,8 @@ constructor (GType type,
 	self = NM_DEVICE (object);
 	priv = NM_DEVICE_GET_PRIVATE (self);
 
-	if (priv->iface) {
+	if (   priv->iface
+	    && G_LIKELY (!nm_utils_get_testing ())) {
 		pllink = nm_platform_link_get_by_ifname (NM_PLATFORM_GET, priv->iface);
 
 		if (pllink && link_type_compatible (self, pllink->type, NULL, NULL)) {
@@ -11657,6 +11666,17 @@ constructor (GType type,
 		}
 	}
 
+	if (priv->hw_addr) {
+		count = _nm_utils_hwaddr_length (priv->hw_addr);
+		if (count <= 0) {
+			_LOGW (LOGD_DEVICE, "hw-addr: could not parse hw-address '%s'", priv->hw_addr);
+			g_clear_pointer (&priv->hw_addr, g_free);
+		} else {
+			priv->hw_addr_len = count;
+			_LOGT (LOGD_DEVICE, "hw-addr: set current hw-address '%s'", priv->hw_addr);
+		}
+	}
+
 	return object;
 }
 
@@ -11812,10 +11832,8 @@ static void
 set_property (GObject *object, guint prop_id,
               const GValue *value, GParamSpec *pspec)
 {
-	NMDevice *self = NM_DEVICE (object);
+	NMDevice *self = (NMDevice *) object;
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
-	const char *hw_addr, *p;
-	guint count;
 
 	switch (prop_id) {
 	case PROP_UDI:
@@ -11895,30 +11913,7 @@ set_property (GObject *object, guint prop_id,
 		break;
 	case PROP_HW_ADDRESS:
 		/* construct only */
-		p = hw_addr = g_value_get_string (value);
-
-		/* Hardware address length is the number of ':' plus 1 */
-		count = 1;
-		while (p && *p) {
-			if (*p++ == ':')
-				count++;
-		}
-		if (count < ETH_ALEN || count > NM_UTILS_HWADDR_LEN_MAX) {
-			if (hw_addr && *hw_addr) {
-				_LOGW (LOGD_DEVICE, "ignoring hardware address '%s' with unexpected length %d",
-				       hw_addr, count);
-			}
-			break;
-		}
-
-		priv->hw_addr_len = count;
-		g_free (priv->hw_addr);
-		if (nm_utils_hwaddr_valid (hw_addr, priv->hw_addr_len))
-			priv->hw_addr = g_strdup (hw_addr);
-		else {
-			_LOGW (LOGD_DEVICE, "could not parse hw-address '%s'", hw_addr);
-			priv->hw_addr = NULL;
-		}
+		priv->hw_addr = g_value_dup_string (value);
 		break;
 	default:
 		G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
diff --git a/src/tests/config/nm-test-device.c b/src/tests/config/nm-test-device.c
index 04ccaec214..be67646fb4 100644
--- a/src/tests/config/nm-test-device.c
+++ b/src/tests/config/nm-test-device.c
@@ -38,17 +38,6 @@ nm_test_device_init (NMTestDevice *self)
 /* We jump over NMDevice's construct/destruct methods, which require NMPlatform
  * and NMConnectionProvider to be initialized.
  */
-
-static GObject*
-constructor (GType type,
-             guint n_construct_params,
-             GObjectConstructParam *construct_params)
-{
-	return PARENT_CLASS->constructor (type,
-	                                  n_construct_params,
-	                                  construct_params);
-}
-
 static void
 constructed (GObject *object)
 {
@@ -73,7 +62,6 @@ nm_test_device_class_init (NMTestDeviceClass *klass)
 	GObjectClass *object_class = G_OBJECT_CLASS (klass);
 	NMDeviceClass *device_class = NM_DEVICE_CLASS (klass);
 
-	object_class->constructor = constructor;
 	object_class->constructed = constructed;
 	object_class->dispose = dispose;