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.
This commit is contained in:
Thomas Haller 2016-05-20 12:34:13 +02:00
parent e92b743ce9
commit 6947aedb6e
2 changed files with 28 additions and 45 deletions

View file

@ -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);

View file

@ -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;