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 <thaller@redhat.com>
This commit is contained in:
Thomas Haller 2013-12-11 14:33:49 +01:00
parent 0f38213129
commit 123bf9eea4
5 changed files with 108 additions and 49 deletions

View file

@ -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;
}
}

View file

@ -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,

View file

@ -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;

View file

@ -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

View file

@ -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);