From 123bf9eea44b2a570cf862f98564ca3a3e2ce8de Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 11 Dec 2013 14:33:49 +0100 Subject: [PATCH] libnm-util: raise CHANGED signal in nm_connection_update_secrets only on change This changes behaviour of nm_connection_update_secrets() in that it will now return %TRUE, if there are no secrets to be cleared. Seems more correct, to return success if there is nothing to do. Signed-off-by: Thomas Haller --- libnm-util/nm-connection.c | 43 +++++++++++++++------- libnm-util/nm-setting-private.h | 9 +++++ libnm-util/nm-setting-vpn.c | 38 +++++++++++-------- libnm-util/nm-setting.c | 65 +++++++++++++++++++++++---------- libnm-util/nm-setting.h | 2 +- 5 files changed, 108 insertions(+), 49 deletions(-) diff --git a/libnm-util/nm-connection.c b/libnm-util/nm-connection.c index f62a2e9c55..807a893a47 100644 --- a/libnm-util/nm-connection.c +++ b/libnm-util/nm-connection.c @@ -676,11 +676,12 @@ nm_connection_update_secrets (NMConnection *connection, GError **error) { NMSetting *setting; - gboolean success = FALSE, updated = FALSE; + gboolean success = TRUE, updated = FALSE; GHashTable *setting_hash = NULL; GHashTableIter iter; const char *key; gboolean hashed_connection = FALSE; + int success_detail; g_return_val_if_fail (NM_IS_CONNECTION (connection), FALSE); g_return_val_if_fail (secrets != NULL, FALSE); @@ -721,15 +722,20 @@ nm_connection_update_secrets (NMConnection *connection, /* The hashed connection that didn't contain any secrets for * @setting_name; just return success. */ - success = TRUE; + return TRUE; } } - if (!success) { - updated = success = nm_setting_update_secrets (setting, - setting_hash ? setting_hash : secrets, - error); - } + g_signal_handlers_block_by_func (setting, (GCallback) setting_changed_cb, connection); + success_detail = _nm_setting_update_secrets (setting, + setting_hash ? setting_hash : secrets, + error); + g_signal_handlers_unblock_by_func (setting, (GCallback) setting_changed_cb, connection); + + if (success_detail == NM_SETTING_UPDATE_SECRET_ERROR) + return FALSE; + if (success_detail == NM_SETTING_UPDATE_SECRET_SUCCESS_MODIFIED) + updated = TRUE; } else { if (!hashed_connection) { g_set_error_literal (error, @@ -739,9 +745,9 @@ nm_connection_update_secrets (NMConnection *connection, return FALSE; } - /* Update each setting with any secrets from the hashed connection */ + /* check first, whether all the settings exist... */ g_hash_table_iter_init (&iter, secrets); - while (g_hash_table_iter_next (&iter, (gpointer) &key, (gpointer) &setting_hash)) { + while (g_hash_table_iter_next (&iter, (gpointer) &key, NULL)) { setting = nm_connection_get_setting_by_name (connection, key); if (!setting) { g_set_error_literal (error, @@ -750,13 +756,24 @@ nm_connection_update_secrets (NMConnection *connection, key); return FALSE; } + } + /* Update each setting with any secrets from the hashed connection */ + g_hash_table_iter_init (&iter, secrets); + while (g_hash_table_iter_next (&iter, (gpointer) &key, (gpointer) &setting_hash)) { /* Update the secrets for this setting */ - success = nm_setting_update_secrets (setting, setting_hash, error); - if (success) - updated = TRUE; - else + setting = nm_connection_get_setting_by_name (connection, key); + + g_signal_handlers_block_by_func (setting, (GCallback) setting_changed_cb, connection); + success_detail = _nm_setting_update_secrets (setting, setting_hash, error); + g_signal_handlers_unblock_by_func (setting, (GCallback) setting_changed_cb, connection); + + if (success_detail == NM_SETTING_UPDATE_SECRET_ERROR) { + success = FALSE; break; + } + if (success_detail == NM_SETTING_UPDATE_SECRET_SUCCESS_MODIFIED) + updated = TRUE; } } diff --git a/libnm-util/nm-setting-private.h b/libnm-util/nm-setting-private.h index 058e7bd8bd..fd5f009bcd 100644 --- a/libnm-util/nm-setting-private.h +++ b/libnm-util/nm-setting-private.h @@ -42,6 +42,15 @@ GType _nm_setting_lookup_setting_type (const char *name); GType _nm_setting_lookup_setting_type_by_quark (GQuark error_quark); gint _nm_setting_compare_priority (gconstpointer a, gconstpointer b); +typedef enum NMSettingUpdateSecretResult { + NM_SETTING_UPDATE_SECRET_ERROR = FALSE, + NM_SETTING_UPDATE_SECRET_SUCCESS_MODIFIED = TRUE, + NM_SETTING_UPDATE_SECRET_SUCCESS_UNCHANGED = 2, +} NMSettingUpdateSecretResult; + +NMSettingUpdateSecretResult _nm_setting_update_secrets (NMSetting *setting, + GHashTable *secrets, + GError **error); gboolean _nm_setting_clear_secrets (NMSetting *setting); gboolean _nm_setting_clear_secrets_with_flags (NMSetting *setting, NMSettingClearSecretsWithFlagsFn func, diff --git a/libnm-util/nm-setting-vpn.c b/libnm-util/nm-setting-vpn.c index 3de47a07f1..b5cffc14b6 100644 --- a/libnm-util/nm-setting-vpn.c +++ b/libnm-util/nm-setting-vpn.c @@ -429,7 +429,7 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) return TRUE; } -static gboolean +static NMSettingUpdateSecretResult update_secret_string (NMSetting *setting, const char *key, const char *value, @@ -437,21 +437,24 @@ update_secret_string (NMSetting *setting, { NMSettingVPNPrivate *priv = NM_SETTING_VPN_GET_PRIVATE (setting); - g_return_val_if_fail (key != NULL, FALSE); - g_return_val_if_fail (value != NULL, FALSE); + g_return_val_if_fail (key != NULL, NM_SETTING_UPDATE_SECRET_ERROR); + g_return_val_if_fail (value != NULL, NM_SETTING_UPDATE_SECRET_ERROR); if (!value || !strlen (value)) { g_set_error (error, NM_SETTING_ERROR, NM_SETTING_ERROR_PROPERTY_TYPE_MISMATCH, "Secret %s was empty", key); - return FALSE; + return NM_SETTING_UPDATE_SECRET_ERROR; } + if (g_strcmp0 (g_hash_table_lookup (priv->secrets, key), value) == 0) + return NM_SETTING_UPDATE_SECRET_SUCCESS_UNCHANGED; + g_hash_table_insert (priv->secrets, g_strdup (key), g_strdup (value)); - return TRUE; + return NM_SETTING_UPDATE_SECRET_SUCCESS_MODIFIED; } -static gboolean +static NMSettingUpdateSecretResult update_secret_hash (NMSetting *setting, GHashTable *secrets, GError **error) @@ -459,8 +462,9 @@ update_secret_hash (NMSetting *setting, NMSettingVPNPrivate *priv = NM_SETTING_VPN_GET_PRIVATE (setting); GHashTableIter iter; const char *name, *value; + NMSettingUpdateSecretResult result = NM_SETTING_UPDATE_SECRET_SUCCESS_UNCHANGED; - g_return_val_if_fail (secrets != NULL, FALSE); + g_return_val_if_fail (secrets != NULL, NM_SETTING_UPDATE_SECRET_ERROR); /* Make sure the items are valid */ g_hash_table_iter_init (&iter, secrets); @@ -469,14 +473,14 @@ update_secret_hash (NMSetting *setting, g_set_error_literal (error, NM_SETTING_ERROR, NM_SETTING_ERROR_PROPERTY_TYPE_MISMATCH, "Secret name was empty"); - return FALSE; + return NM_SETTING_UPDATE_SECRET_ERROR; } if (!value || !strlen (value)) { g_set_error (error, NM_SETTING_ERROR, NM_SETTING_ERROR_PROPERTY_TYPE_MISMATCH, "Secret %s value was empty", name); - return FALSE; + return NM_SETTING_UPDATE_SECRET_ERROR; } } @@ -492,19 +496,23 @@ update_secret_hash (NMSetting *setting, continue; } + if (g_strcmp0 (g_hash_table_lookup (priv->secrets, name), value) == 0) + continue; + g_hash_table_insert (priv->secrets, g_strdup (name), g_strdup (value)); + result = NM_SETTING_UPDATE_SECRET_SUCCESS_MODIFIED; } - return TRUE; + return result; } -static gboolean +static int update_one_secret (NMSetting *setting, const char *key, GValue *value, GError **error) { - gboolean success = FALSE; + NMSettingUpdateSecretResult success = NM_SETTING_UPDATE_SECRET_ERROR; - g_return_val_if_fail (key != NULL, FALSE); - g_return_val_if_fail (value != NULL, FALSE); + g_return_val_if_fail (key != NULL, NM_SETTING_UPDATE_SECRET_ERROR); + g_return_val_if_fail (value != NULL, NM_SETTING_UPDATE_SECRET_ERROR); if (G_VALUE_HOLDS_STRING (value)) { /* Passing the string properties individually isn't correct, and won't @@ -522,7 +530,7 @@ update_one_secret (NMSetting *setting, const char *key, GValue *value, GError ** } else g_set_error_literal (error, NM_SETTING_ERROR, NM_SETTING_ERROR_PROPERTY_TYPE_MISMATCH, key); - if (success) + if (success == NM_SETTING_UPDATE_SECRET_SUCCESS_MODIFIED) g_object_notify (G_OBJECT (setting), NM_SETTING_VPN_SECRETS); return success; diff --git a/libnm-util/nm-setting.c b/libnm-util/nm-setting.c index d0c992f2f6..8b2ee5d2a9 100644 --- a/libnm-util/nm-setting.c +++ b/libnm-util/nm-setting.c @@ -951,12 +951,11 @@ nm_setting_need_secrets (NMSetting *setting) return secrets; } -static gboolean +static int update_one_secret (NMSetting *setting, const char *key, GValue *value, GError **error) { GParamSpec *prop_spec; GValue transformed_value = G_VALUE_INIT; - gboolean success = FALSE; prop_spec = g_object_class_find_property (G_OBJECT_GET_CLASS (setting), key); if (!prop_spec) { @@ -964,27 +963,40 @@ update_one_secret (NMSetting *setting, const char *key, GValue *value, GError ** NM_SETTING_ERROR, NM_SETTING_ERROR_PROPERTY_NOT_FOUND, "%s", key); - return FALSE; + return NM_SETTING_UPDATE_SECRET_ERROR; } /* Silently ignore non-secrets */ if (!(prop_spec->flags & NM_SETTING_PARAM_SECRET)) - return TRUE; + return NM_SETTING_UPDATE_SECRET_SUCCESS_UNCHANGED; if (g_value_type_compatible (G_VALUE_TYPE (value), G_PARAM_SPEC_VALUE_TYPE (prop_spec))) { + if (G_VALUE_HOLDS_STRING (value) && G_IS_PARAM_SPEC_STRING (prop_spec)) { + /* String is expected to be a common case. Handle it specially and check whether + * the value is already set. Otherwise, we just reset the property and + * assume the value got modified. */ + char *v; + + g_object_get (G_OBJECT (setting), prop_spec->name, &v, NULL); + if (g_strcmp0 (v, g_value_get_string (value)) == 0) { + g_free (v); + return NM_SETTING_UPDATE_SECRET_SUCCESS_UNCHANGED; + } + g_free (v); + } g_object_set_property (G_OBJECT (setting), prop_spec->name, value); - success = TRUE; - } else if (g_value_transform (value, &transformed_value)) { + return NM_SETTING_UPDATE_SECRET_SUCCESS_MODIFIED; + } + if (g_value_transform (value, &transformed_value)) { g_object_set_property (G_OBJECT (setting), prop_spec->name, &transformed_value); g_value_unset (&transformed_value); - success = TRUE; - } else { - g_set_error (error, - NM_SETTING_ERROR, - NM_SETTING_ERROR_PROPERTY_TYPE_MISMATCH, - "%s", key); + return NM_SETTING_UPDATE_SECRET_SUCCESS_MODIFIED; } - return success; + g_set_error (error, + NM_SETTING_ERROR, + NM_SETTING_ERROR_PROPERTY_TYPE_MISMATCH, + "%s", key); + return NM_SETTING_UPDATE_SECRET_ERROR; } /** @@ -1002,29 +1014,42 @@ update_one_secret (NMSetting *setting, const char *key, GValue *value, GError ** **/ gboolean nm_setting_update_secrets (NMSetting *setting, GHashTable *secrets, GError **error) +{ + return _nm_setting_update_secrets (setting, secrets, error) != NM_SETTING_UPDATE_SECRET_ERROR; +} + +NMSettingUpdateSecretResult +_nm_setting_update_secrets (NMSetting *setting, GHashTable *secrets, GError **error) { GHashTableIter iter; gpointer key, data; GError *tmp_error = NULL; + NMSettingUpdateSecretResult result = NM_SETTING_UPDATE_SECRET_SUCCESS_UNCHANGED; - g_return_val_if_fail (NM_IS_SETTING (setting), FALSE); - g_return_val_if_fail (secrets != NULL, FALSE); + g_return_val_if_fail (NM_IS_SETTING (setting), NM_SETTING_UPDATE_SECRET_ERROR); + g_return_val_if_fail (secrets != NULL, NM_SETTING_UPDATE_SECRET_ERROR); if (error) - g_return_val_if_fail (*error == NULL, FALSE); + g_return_val_if_fail (*error == NULL, NM_SETTING_UPDATE_SECRET_ERROR); g_hash_table_iter_init (&iter, secrets); while (g_hash_table_iter_next (&iter, &key, &data)) { + int success; const char *secret_key = (const char *) key; GValue *secret_value = (GValue *) data; - NM_SETTING_GET_CLASS (setting)->update_one_secret (setting, secret_key, secret_value, &tmp_error); - if (tmp_error) { + success = NM_SETTING_GET_CLASS (setting)->update_one_secret (setting, secret_key, secret_value, &tmp_error); + g_assert (!((success == NM_SETTING_UPDATE_SECRET_ERROR) ^ (!!tmp_error))); + + if (success == NM_SETTING_UPDATE_SECRET_ERROR) { g_propagate_error (error, tmp_error); - return FALSE; + return NM_SETTING_UPDATE_SECRET_ERROR; } + + if (success == NM_SETTING_UPDATE_SECRET_SUCCESS_MODIFIED) + result = NM_SETTING_UPDATE_SECRET_SUCCESS_MODIFIED; } - return TRUE; + return result; } static gboolean diff --git a/libnm-util/nm-setting.h b/libnm-util/nm-setting.h index b7f7d62ec3..07e07477ca 100644 --- a/libnm-util/nm-setting.h +++ b/libnm-util/nm-setting.h @@ -177,7 +177,7 @@ typedef struct { GPtrArray *(*need_secrets) (NMSetting *setting); - gboolean (*update_one_secret) (NMSetting *setting, + int (*update_one_secret) (NMSetting *setting, const char *key, GValue *value, GError **error);