summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorThomas Haller <thaller@redhat.com>2018-11-17 13:35:02 +0100
committerThomas Haller <thaller@redhat.com>2018-11-17 13:38:15 +0100
commitf10f0199826ea8b7b818bfe5e8268b4f4839a493 (patch)
tree70f1fc2a363339a84735e0f12a48f4d394c5df87
parent6b79af28d66a709a8feda9dc3824ac544db0e7bb (diff)
downloadNetworkManager-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.c26
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