From f10f0199826ea8b7b818bfe5e8268b4f4839a493 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 17 Nov 2018 13:35:02 +0100 Subject: [PATCH] policy: don't check for valid error in active_connection_keep_alive_changed() Most (not all) functions that can fail and report the reason with an GError are required to set the error if they fail. It's a bug to claim to fail without returning the GError reason. Hence, our callers usually don't check whether a GError is present but just access it. Likewise, for better or worse, our GError codes are often not meaningful (unless explicitly documented). Meaning, logging the error code number is not helpful. Instead, error messages should be written in a manner that one can find the source code location where it happened. Also, return-early to reduce the indentation level of the code. Also, drop the code comment. It seems to just describe what is obviously visible by reading the source. It doesn't explain much beside that the "doesn't have a reason", but not really why. --- src/nm-policy.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index 323df112de..5199afbf1c 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -2190,22 +2190,20 @@ active_connection_keep_alive_changed (NMActiveConnection *ac, NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); GError *error = NULL; - /* The connection does not have a reason to stay alive anymore. */ - if (!nm_active_connection_get_keep_alive (ac)) { - if (nm_active_connection_get_state (ac) <= NM_ACTIVE_CONNECTION_STATE_ACTIVATED) { - if (!nm_manager_deactivate_connection (priv->manager, - ac, - NM_DEVICE_STATE_REASON_CONNECTION_REMOVED, - &error)) { - _LOGW (LOGD_DEVICE, "connection '%s' is no longer kept alive, but error deactivating it: (%d) %s", - nm_active_connection_get_settings_connection_id (ac), - error ? error->code : -1, - error ? error->message : "(unknown)"); - g_clear_error (&error); - } + if (nm_active_connection_get_keep_alive (ac)) + return; + + if (nm_active_connection_get_state (ac) <= NM_ACTIVE_CONNECTION_STATE_ACTIVATED) { + if (!nm_manager_deactivate_connection (priv->manager, + ac, + NM_DEVICE_STATE_REASON_CONNECTION_REMOVED, + &error)) { + _LOGW (LOGD_DEVICE, "connection '%s' is no longer kept alive, but error deactivating it: %s", + nm_active_connection_get_settings_connection_id (ac), + error->message); + g_clear_error (&error); } } - } static void