core: simplify hardware address reading

We no longer need a class method for reading the hardware address
length, for a couple reasons:

1) Using the IP interface hardware address for IP operations now makes
NMDevice's priv->hw_addr_len constant.  So there's no reason to re-read
it all the time.

2) get_hw_address_length() is now only used for determining whether the
hardware address is permanent, and that only mattered for Bluetooth.
Since Bluetooth interfaces have a bogus interface name, they will never
have a valid ifindex, and thus nm_platform_link_get_address() would be
useless.  So instead of using the 'permanent' stuff, just don't bother
updating the hardware address if the NMDevice's ifindex isn't valid,
and let subclasses pass the initial hardware address at device creation.

This also works correctly for NMDevice classes that previously
implemented get_hw_address_length() like ADSL and WWAN, since those
too will never have a valid ifindex or a valid hardware address.

3) Reading the device's hardware address length just ended up calling
nm_platform_link_get_address() for most devices anyway, so
nm_device_update_hw_address() would effectively read the link address
twice (once to read the length, the second time to read the actual
address).  Let's just read the address once.

6 files changed, 44 insertions, 121 deletions

diff --git a/src/config/tests/nm-test-device.c b/src/config/tests/nm-test-device.c
index fcfc63a7db..aeac6137c4 100644
--- a/src/config/tests/nm-test-device.c
+++ b/src/config/tests/nm-test-device.c
@@ -52,10 +52,6 @@ constructor (GType type,
 static void
 constructed (GObject *object)
 {
-	NMDevice *device = NM_DEVICE (object);
-
-	nm_device_update_hw_address (device);
-
 	g_object_class->constructed (object);
 }
 
@@ -71,19 +67,10 @@ finalize (GObject *object)
 	g_object_class->finalize (object);
 }
 
-static guint
-get_hw_address_length (NMDevice *dev, gboolean *out_permanent)
-{
-	if (out_permanent)
-		*out_permanent = TRUE;
-	return ETH_ALEN;
-}
-
 static void
 nm_test_device_class_init (NMTestDeviceClass *klass)
 {
 	GObjectClass *object_class = G_OBJECT_CLASS (klass);
-	NMDeviceClass *device_class = NM_DEVICE_CLASS (klass);
 
 	g_object_class = g_type_class_peek (G_TYPE_OBJECT);
 
@@ -91,8 +78,6 @@ nm_test_device_class_init (NMTestDeviceClass *klass)
 	object_class->constructed = constructed;
 	object_class->dispose = dispose;
 	object_class->finalize = finalize;
-
-	device_class->get_hw_address_length = get_hw_address_length;
 }
 
 NMDevice *
diff --git a/src/devices/adsl/nm-device-adsl.c b/src/devices/adsl/nm-device-adsl.c
index 627594c378..c4a98fa3ab 100644
--- a/src/devices/adsl/nm-device-adsl.c
+++ b/src/devices/adsl/nm-device-adsl.c
@@ -483,12 +483,6 @@ deactivate (NMDevice *device)
 
 /**************************************************************/
 
-static guint
-get_hw_address_length (NMDevice *device, gboolean *out_permanent)
-{
-	return 0;
-}
-
 static gboolean
 carrier_update_cb (gpointer user_data)
 {
@@ -621,7 +615,6 @@ nm_device_adsl_class_init (NMDeviceAdslClass *klass)
 	parent_class->check_connection_compatible = check_connection_compatible;
 	parent_class->complete_connection = complete_connection;
 
-	parent_class->get_hw_address_length = get_hw_address_length;
 	parent_class->act_stage2_config = act_stage2_config;
 	parent_class->act_stage3_ip4_config_start = act_stage3_ip4_config_start;
 	parent_class->deactivate = deactivate;
diff --git a/src/devices/bluetooth/nm-device-bt.c b/src/devices/bluetooth/nm-device-bt.c
index e9c14e837d..9d59344933 100644
--- a/src/devices/bluetooth/nm-device-bt.c
+++ b/src/devices/bluetooth/nm-device-bt.c
@@ -111,15 +111,6 @@ guint32 nm_device_bt_get_capabilities (NMDeviceBt *self)
 	return NM_DEVICE_BT_GET_PRIVATE (self)->capabilities;
 }
 
-static guint
-get_hw_address_length (NMDevice *device, gboolean *out_permanent)
-{
-	/* HW address is the Bluetooth HW address of the remote device */
-	if (out_permanent)
-		*out_permanent = TRUE;   /* the bdaddr of the remote device will never change */
-	return ETH_ALEN;
-}
-
 static guint32
 get_connection_bt_type (NMConnection *connection)
 {
@@ -1207,7 +1198,6 @@ nm_device_bt_class_init (NMDeviceBtClass *klass)
 	object_class->dispose = dispose;
 	object_class->finalize = finalize;
 
-	device_class->get_hw_address_length = get_hw_address_length;
 	device_class->can_auto_connect = can_auto_connect;
 	device_class->deactivate = deactivate;
 	device_class->act_stage2_config = act_stage2_config;
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index b048571e5a..f638f09b95 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -324,7 +324,7 @@ static void _set_state_full (NMDevice *device,
                              NMDeviceStateReason reason,
                              gboolean quitting);
 
-static gboolean nm_device_update_hw_address (NMDevice *dev);
+static void nm_device_update_hw_address (NMDevice *dev);
 
 /***********************************************************/
 
@@ -6934,12 +6934,6 @@ nm_device_get_state (NMDevice *device)
 /***********************************************************/
 /* NMConfigDevice interface related stuff */
 
-static guint
-nm_device_get_hw_address_length (NMDevice *dev, gboolean *out_permanent)
-{
-	return NM_DEVICE_GET_CLASS (dev)->get_hw_address_length (dev, out_permanent);
-}
-
 const guint8 *
 nm_device_get_hw_address (NMDevice *dev, guint *out_len)
 {
@@ -6951,68 +6945,46 @@ nm_device_get_hw_address (NMDevice *dev, guint *out_len)
 	if (out_len)
 		*out_len = priv->hw_addr_len;
 
-	if (priv->hw_addr_len == 0)
-		return NULL;
-	else
-		return priv->hw_addr;
+	return priv->hw_addr_len ? priv->hw_addr : NULL;
 }
 
-static gboolean
+static void
 nm_device_update_hw_address (NMDevice *dev)
 {
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (dev);
-	gboolean changed = FALSE, permanent = FALSE;
+	int ifindex = nm_device_get_ifindex (dev);
+	const char *iface = nm_device_get_iface (dev);
+	const guint8 *hwaddr;
+	gsize hwaddrlen = 0;
 
-	priv->hw_addr_len = nm_device_get_hw_address_length (dev, &permanent);
-
-	/* If the address can't be changed, don't bother trying */
-	if (permanent)
-		return FALSE;
-
-	if (priv->hw_addr_len) {
-		int ifindex = nm_device_get_ifindex (dev);
-		gsize addrlen;
-		const guint8 *binaddr;
+	if (ifindex <= 0)
+		return;
 
-		g_return_val_if_fail (ifindex > 0, FALSE);
+	hwaddr = nm_platform_link_get_address (ifindex, &hwaddrlen);
+	g_assert (hwaddrlen <= sizeof (priv->hw_addr));
+	if (hwaddrlen) {
+		if (hwaddrlen != priv->hw_addr_len || memcmp (priv->hw_addr, hwaddr, hwaddrlen)) {
+			memcpy (priv->hw_addr, hwaddr, hwaddrlen);
 
-		binaddr = nm_platform_link_get_address (ifindex, &addrlen);
+			if (nm_logging_enabled (LOGL_DEBUG, LOGD_HW | LOGD_DEVICE)) {
+				char *addrstr = nm_utils_hwaddr_ntoa_len (hwaddr, hwaddrlen);
 
-		if (addrlen != priv->hw_addr_len) {
-			nm_log_err (LOGD_HW | LOGD_DEVICE,
-			            "(%s): hardware address is wrong length (got %zd, expected %d)",
-			            nm_device_get_iface (dev), addrlen, priv->hw_addr_len);
-		} else {
-			changed = !!memcmp (priv->hw_addr, binaddr, addrlen);
-			if (changed) {
-				char *addrstr = nm_utils_hwaddr_ntoa_len (binaddr, priv->hw_addr_len);
-
-				memcpy (priv->hw_addr, binaddr, addrlen);
-				nm_log_dbg (LOGD_HW | LOGD_DEVICE,
-				            "(%s): hardware address is %s",
-				            nm_device_get_iface (dev), addrstr);
+				nm_log_dbg (LOGD_HW | LOGD_DEVICE, "(%s): hardware address now %s", iface, addrstr);
 				g_free (addrstr);
-				g_object_notify (G_OBJECT (dev), NM_DEVICE_HW_ADDRESS);
 			}
+			g_object_notify (G_OBJECT (dev), NM_DEVICE_HW_ADDRESS);
 		}
 	} else {
-		int i;
-
-		/* hw_addr_len is now 0; see if hw_addr was already empty */
-		for (i = 0; i < sizeof (priv->hw_addr) && !changed; i++) {
-			if (priv->hw_addr[i])
-				changed = TRUE;
-		}
-		if (changed) {
+		/* Invalid or no hardware address */
+		if (priv->hw_addr_len != 0) {
 			memset (priv->hw_addr, 0, sizeof (priv->hw_addr));
 			nm_log_dbg (LOGD_HW | LOGD_DEVICE,
 			            "(%s): previous hardware address is no longer valid",
-			            nm_device_get_iface (dev));
+			            iface);
 			g_object_notify (G_OBJECT (dev), NM_DEVICE_HW_ADDRESS);
 		}
 	}
-
-	return changed;
+	priv->hw_addr_len = hwaddrlen;
 }
 
 gboolean
@@ -7118,17 +7090,6 @@ spec_match_list (NMDevice *device, const GSList *specs)
 	return matched;
 }
 
-static guint
-get_hw_address_length (NMDevice *dev, gboolean *out_permanent)
-{
-	size_t len;
-
-	if (nm_platform_link_get_address (nm_device_get_ifindex (dev), &len))
-		return len;
-	else
-		return 0;
-}
-
 /***********************************************************/
 
 #define DEFAULT_AUTOCONNECT TRUE
@@ -7375,9 +7336,9 @@ set_property (GObject *object, guint prop_id,
 {
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (object);
 	NMPlatformLink *platform_device;
-	const char *hw_addr;
-	guint hw_addr_len;
-
+	const char *hw_addr, *p;
+	guint count;
+ 
 	switch (prop_id) {
 	case PROP_PLATFORM_DEVICE:
 		platform_device = g_value_get_pointer (value);
@@ -7455,18 +7416,21 @@ set_property (GObject *object, guint prop_id,
 		priv->is_master = g_value_get_boolean (value);
 		break;
 	case PROP_HW_ADDRESS:
-		hw_addr_len = nm_device_get_hw_address_length (NM_DEVICE (object), NULL);
-		g_return_if_fail (hw_addr_len <= NM_UTILS_HWADDR_LEN_MAX);
-		priv->hw_addr_len = hw_addr_len;
-
-		hw_addr = g_value_get_string (value);
-		if (!hw_addr)
-			break;
-		if (priv->hw_addr_len == 0) {
-			g_warn_if_fail (*hw_addr == '\0');
+		/* 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) {
+			g_warn_if_fail (!hw_addr || *hw_addr == '\0');
 			break;
 		}
 
+		priv->hw_addr_len = count;
 		if (!nm_utils_hwaddr_aton_len (hw_addr, priv->hw_addr, priv->hw_addr_len)) {
 			g_warning ("Could not parse hw-address '%s'", hw_addr);
 			memset (priv->hw_addr, 0, sizeof (priv->hw_addr));
@@ -7647,7 +7611,6 @@ nm_device_class_init (NMDeviceClass *klass)
 	klass->bring_up = bring_up;
 	klass->take_down = take_down;
 	klass->carrier_changed = carrier_changed;
-	klass->get_hw_address_length = get_hw_address_length;
 
 	/* Properties */
 	g_object_class_install_property
diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h
index 3d45ccf3a9..871764531f 100644
--- a/src/devices/nm-device.h
+++ b/src/devices/nm-device.h
@@ -55,12 +55,13 @@
 #define NM_DEVICE_AVAILABLE_CONNECTIONS "available-connections"
 #define NM_DEVICE_PHYSICAL_PORT_ID "physical-port-id"
 #define NM_DEVICE_MTU              "mtu"
-#define NM_DEVICE_TYPE_DESC        "type-desc"    /* Internal only */
-#define NM_DEVICE_RFKILL_TYPE      "rfkill-type"  /* Internal only */
-#define NM_DEVICE_IFINDEX          "ifindex"      /* Internal only */
-#define NM_DEVICE_IS_MASTER        "is-master"    /* Internal only */
-#define NM_DEVICE_MASTER           "master"       /* Internal only */
-#define NM_DEVICE_HW_ADDRESS       "hw-address"   /* Internal only */
+#define NM_DEVICE_HW_ADDRESS       "hw-address"
+
+#define NM_DEVICE_TYPE_DESC        "type-desc"      /* Internal only */
+#define NM_DEVICE_RFKILL_TYPE      "rfkill-type"    /* Internal only */
+#define NM_DEVICE_IFINDEX          "ifindex"        /* Internal only */
+#define NM_DEVICE_IS_MASTER        "is-master"      /* Internal only */
+#define NM_DEVICE_MASTER           "master"         /* Internal only */
 #define NM_DEVICE_HAS_PENDING_ACTION "has-pending-action" /* Internal only */
 
 /* Internal signals */
@@ -115,10 +116,8 @@ typedef struct {
 	/* Carrier state (IFF_LOWER_UP) */
 	void            (*carrier_changed) (NMDevice *, gboolean carrier);
 
-	void        (* update_hw_address) (NMDevice *self);
 	void        (* update_permanent_hw_address) (NMDevice *self);
 	void        (* update_initial_hw_address) (NMDevice *self);
-	guint       (* get_hw_address_length) (NMDevice *self, gboolean *out_permanent);
 
 	guint32		(* get_generic_capabilities)	(NMDevice *self);
 
diff --git a/src/devices/wwan/nm-device-modem.c b/src/devices/wwan/nm-device-modem.c
index 156c3abc8d..98ff610229 100644
--- a/src/devices/wwan/nm-device-modem.c
+++ b/src/devices/wwan/nm-device-modem.c
@@ -290,12 +290,6 @@ device_state_changed (NMDevice *device,
 	}
 }
 
-static guint
-get_hw_address_length (NMDevice *device, gboolean *out_permanent)
-{
-	return 0;
-}
-
 static gboolean
 check_connection_compatible (NMDevice *device, NMConnection *connection)
 {
@@ -596,7 +590,6 @@ nm_device_modem_class_init (NMDeviceModemClass *mclass)
 	object_class->get_property = get_property;
 	object_class->set_property = set_property;
 
-	device_class->get_hw_address_length = get_hw_address_length;
 	device_class->check_connection_compatible = check_connection_compatible;
 	device_class->check_connection_available = check_connection_available;
 	device_class->complete_connection = complete_connection;