diff options
author | Thomas Haller <thaller@redhat.com> | 2018-11-17 13:35:02 +0100 |
---|---|---|
committer | Thomas Haller <thaller@redhat.com> | 2018-11-17 13:38:15 +0100 |
commit | f10f0199826ea8b7b818bfe5e8268b4f4839a493 (patch) | |
tree | 70f1fc2a363339a84735e0f12a48f4d394c5df87 | |
parent | 6b79af28d66a709a8feda9dc3824ac544db0e7bb (diff) | |
download | NetworkManager-f10f0199826ea8b7b818bfe5e8268b4f4839a493.tar.gz |
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.
-rw-r--r-- | src/nm-policy.c | 26 |
1 files 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 |