From cf942832c3af4b6640c9f752816b38ceed6258c5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 28 Sep 2022 20:47:33 +0200 Subject: [PATCH 1/3] device: simplify resource management in nm_device_master_release_slave() --- src/core/devices/nm-device.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 7985362ac0..1d21bc5501 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -6198,7 +6198,8 @@ nm_device_master_release_slave(NMDevice *self, NMDevicePrivate *priv; NMDevicePrivate *slave_priv; SlaveInfo *info; - gs_unref_object NMDevice *self_free = NULL; + gs_unref_object NMDevice *self_free = NULL; + gs_unref_object NMDevice *slave_free = NULL; g_return_if_fail(NM_DEVICE(self)); g_return_if_fail(NM_DEVICE(slave)); @@ -6241,14 +6242,15 @@ nm_device_master_release_slave(NMDevice *self, /* keep both alive until the end of the function. * Transfers ownership from slave_priv->master. */ - self_free = self; + nm_assert(self == slave_priv->master); + self_free = g_steal_pointer(&slave_priv->master); + + nm_assert(slave == info->slave); + slave_free = g_steal_pointer(&info->slave); c_list_unlink(&info->lst_slave); - slave_priv->master = NULL; - g_signal_handler_disconnect(slave, info->watch_id); - g_object_unref(slave); - g_slice_free(SlaveInfo, info); + nm_g_slice_free(info); if (c_list_is_empty(&priv->slaves)) { _active_connection_set_state_flags_full(self, From 0d764715dd684755750c8c747e090ec6483d5816 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 28 Sep 2022 20:52:26 +0200 Subject: [PATCH 2/3] device: downgrade warning level for logging in nm_device_queue_state() This is something that does happen. Is that a bug? If so, this should not be a warning message but an assertion failure. If it's not a bug, then this does not warrant warning level, because the user wouldn't know what to do about this and it's something that occasionally happens. Granted, the state handling in NMDevice is complex, that it's unclear whether this indicates a problem or not. In any case, having a warning does only confuse the user. --- src/core/devices/nm-device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 1d21bc5501..5e503cda58 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -16114,7 +16114,7 @@ nm_device_queue_state(NMDevice *self, NMDeviceState state, NMDeviceStateReason r /* We should only ever have one delayed state transition at a time */ if (priv->queued_state.id) { - _LOGW(LOGD_DEVICE, + _LOGD(LOGD_DEVICE, "queue-state[%s, reason:%s, id:%u]: %s", nm_device_state_to_string(priv->queued_state.state), nm_device_state_reason_to_string_a(priv->queued_state.reason), From 45eab7b2d8ab5960c297696081be00214d7b4f95 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 28 Sep 2022 19:02:27 +0200 Subject: [PATCH 3/3] core: obfuscate pointer value in logging in "nm-active-connection.c" --- src/core/nm-active-connection.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/core/nm-active-connection.c b/src/core/nm-active-connection.c index 49568f7c7a..4488d909e3 100644 --- a/src/core/nm-active-connection.c +++ b/src/core/nm-active-connection.c @@ -116,22 +116,22 @@ static void auth_complete(NMActiveConnection *self, gboolean result, const char #define _NMLOG_DOMAIN LOGD_DEVICE #define _NMLOG_PREFIX_NAME "active-connection" -#define _NMLOG(level, ...) \ - G_STMT_START \ - { \ - char _sbuf[64]; \ - NMActiveConnectionPrivate *_priv = self ? NM_ACTIVE_CONNECTION_GET_PRIVATE(self) : NULL; \ - \ - nm_log((level), \ - _NMLOG_DOMAIN, \ - (_priv && _priv->device) ? nm_device_get_iface(_priv->device) : NULL, \ - (_priv && _priv->applied_connection) \ - ? nm_connection_get_uuid(_priv->applied_connection) \ - : NULL, \ - "%s%s: " _NM_UTILS_MACRO_FIRST(__VA_ARGS__), \ - _NMLOG_PREFIX_NAME, \ - self ? nm_sprintf_buf(_sbuf, "[%p]", self) : "" _NM_UTILS_MACRO_REST(__VA_ARGS__)); \ - } \ +#define _NMLOG(level, ...) \ + G_STMT_START \ + { \ + char _sbuf[64]; \ + NMActiveConnectionPrivate *_priv = self ? NM_ACTIVE_CONNECTION_GET_PRIVATE(self) : NULL; \ + \ + nm_log((level), \ + _NMLOG_DOMAIN, \ + (_priv && _priv->device) ? nm_device_get_iface(_priv->device) : NULL, \ + (_priv && _priv->applied_connection) \ + ? nm_connection_get_uuid(_priv->applied_connection) \ + : NULL, \ + "%s%s: " _NM_UTILS_MACRO_FIRST(__VA_ARGS__), \ + _NMLOG_PREFIX_NAME, \ + NM_HASH_OBFUSCATE_PTR_STR(self, _sbuf) _NM_UTILS_MACRO_REST(__VA_ARGS__)); \ + } \ G_STMT_END /*****************************************************************************/