system/NetworkManager
system/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager synced 2024-07-23 03:04:18 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

libnm-core: clean up nm_connection_replace_settings()'s semantics

Browse source 
On failure, nm_connection_replace_settings() would leave the
connection in an undefined state. Fix it so that either (a) the
settings are replaced and the resulting connection is valid and we
return TRUE, or (b) the connection is untouched and we return FALSE
and an error. (And add a test case for this.)
This commit is contained in:
Dan Winship 2014-08-22 10:14:38 -04:00
parent 05f3068163
commit 504c292d73
7 changed files with 137 additions and 62 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
include/nm-test-utils.h
View file

@ -936,12 +936,9 @@ nmtst_assert_connection_unnormalizable (NMConnection *con,
GError *error = NULL;
gboolean success;
gboolean was_modified = FALSE;
gs_unref_object NMConnection *clone = NULL;
g_assert (NM_IS_CONNECTION (con));
clone = nm_simple_connection_new_clone (con);
success = nm_connection_verify (con, &error);
nmtst_assert_error (error, expect_error_domain, expect_error_code, NULL);
g_assert (!success);
@ -952,8 +949,6 @@ nmtst_assert_connection_unnormalizable (NMConnection *con,
g_assert (!success);
g_assert (!was_modified);
g_clear_error (&error);
nmtst_assert_connection_equals (con, FALSE, clone, FALSE);
}
#endif

100
libnm-core/nm-connection.c
View file

@ -291,38 +291,6 @@ validate_permissions_type (GHashTable *hash, GError **error)
return TRUE;
}
static gboolean
hash_to_connection (NMConnection *connection, GHashTable *new, GError **error)
{
GHashTableIter iter;
const char *setting_name;
GHashTable *setting_hash;
gboolean changed, valid;
NMConnectionPrivate *priv = NM_CONNECTION_GET_PRIVATE (connection);
if ((changed = g_hash_table_size (priv->settings) > 0))
g_hash_table_foreach_remove (priv->settings, _setting_release, connection);
g_hash_table_iter_init (&iter, new);
while (g_hash_table_iter_next (&iter, (gpointer) &setting_name, (gpointer) &setting_hash)) {
GType type = nm_setting_lookup_type (setting_name);
if (type) {
NMSetting *setting = _nm_setting_new_from_dbus (type, setting_hash);
if (setting) {
_nm_connection_add_setting (connection, setting);
changed = TRUE;
}
}
}
valid = nm_connection_verify (connection, error);
if (changed)
g_signal_emit (connection, signals[CHANGED], 0);
return valid;
}
/**
* nm_connection_replace_settings:
* @connection: a #NMConnection
@ -330,23 +298,32 @@ hash_to_connection (NMConnection *connection, GHashTable *new, GError **error)
* @error: location to store error, or %NULL
*
* Returns: %TRUE if the settings were valid and added to the connection, %FALSE
* if they were not
* if they were not (in which case @connection will be unchanged).
**/
gboolean
nm_connection_replace_settings (NMConnection *connection,
GHashTable *new_settings,
GError **error)
{
gboolean valid = FALSE;
NMConnection *new;
gboolean valid;
g_return_val_if_fail (NM_IS_CONNECTION (connection), FALSE);
g_return_val_if_fail (new_settings != NULL, FALSE);
if (error)
g_return_val_if_fail (*error == NULL, FALSE);
g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
if (validate_permissions_type (new_settings, error))
valid = hash_to_connection (connection, new_settings, error);
return valid;
if (!validate_permissions_type (new_settings, error))
return FALSE;
new = nm_simple_connection_new_from_dbus (new_settings, error);
if (!new)
return FALSE;
valid = nm_connection_replace_settings_from_connection (connection, new, error);
g_object_unref (new);
g_return_val_if_fail (valid == TRUE, FALSE);
return TRUE;
}
/**
@ -359,22 +336,24 @@ nm_connection_replace_settings (NMConnection *connection,
* with the copied settings.
*
* Returns: %TRUE if the settings were valid and added to the connection, %FALSE
* if they were not
* if they were not (in which case @connection will be unchanged).
**/
gboolean
nm_connection_replace_settings_from_connection (NMConnection *connection,
NMConnection *new_connection,
GError **error)
{
NMConnectionPrivate *priv;
NMConnectionPrivate *priv, *new_priv;
GHashTableIter iter;
NMSetting *setting;
gboolean changed, valid;
gboolean changed;
g_return_val_if_fail (NM_IS_CONNECTION (connection), FALSE);
g_return_val_if_fail (NM_IS_CONNECTION (new_connection), FALSE);
if (error)
g_return_val_if_fail (*error == NULL, FALSE);
g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
if (!nm_connection_verify (new_connection, error))
return FALSE;
/* When 'connection' and 'new_connection' are the same object simply return
* in order not to destroy 'connection' */
@ -386,20 +365,45 @@ nm_connection_replace_settings_from_connection (NMConnection *connection,
*/
priv = NM_CONNECTION_GET_PRIVATE (connection);
new_priv = NM_CONNECTION_GET_PRIVATE (new_connection);
if ((changed = g_hash_table_size (priv->settings) > 0))
g_hash_table_foreach_remove (priv->settings, _setting_release, connection);
if (g_hash_table_size (NM_CONNECTION_GET_PRIVATE (new_connection)->settings)) {
g_hash_table_iter_init (&iter, NM_CONNECTION_GET_PRIVATE (new_connection)->settings);
if (g_hash_table_size (new_priv->settings)) {
g_hash_table_iter_init (&iter, new_priv->settings);
while (g_hash_table_iter_next (&iter, NULL, (gpointer) &setting))
_nm_connection_add_setting (connection, nm_setting_duplicate (setting));
changed = TRUE;
}
valid = nm_connection_verify (connection, error);
/* Since new_connection verified before, this shouldn't ever fail */
g_return_val_if_fail (nm_connection_verify (connection, error), FALSE);
if (changed)
g_signal_emit (connection, signals[CHANGED], 0);
return valid;
return TRUE;
}
/**
* nm_connection_clear_settings:
* @connection: a #NMConnection
*
* Deletes all of @connection's settings.
**/
void
nm_connection_clear_settings (NMConnection *connection)
{
NMConnectionPrivate *priv;
g_return_if_fail (NM_IS_CONNECTION (connection));
priv = NM_CONNECTION_GET_PRIVATE (connection);
if (g_hash_table_size (priv->settings) > 0) {
g_hash_table_foreach_remove (priv->settings, _setting_release, connection);
g_signal_emit (connection, signals[CHANGED], 0);
}
}
/**

2
libnm-core/nm-connection.h
View file

@ -166,6 +166,8 @@ gboolean nm_connection_replace_settings_from_connection (NMConnection *conn
NMConnection *new_connection,
GError **error);
void nm_connection_clear_settings (NMConnection *connection);
gboolean nm_connection_compare (NMConnection *a,
NMConnection *b,
NMSettingCompareFlags flags);

32
libnm-core/nm-simple-connection.c
View file

@ -20,6 +20,7 @@
*/
#include "nm-simple-connection.h"
#include "nm-setting-private.h"
static void nm_simple_connection_interface_init (NMConnectionInterface *iface);
@ -65,15 +66,38 @@ NMConnection *
nm_simple_connection_new_from_dbus (GHashTable *hash, GError **error)
{
NMConnection *connection;
GHashTableIter iter;
const char *setting_name;
GHashTable *setting_hash;
g_return_val_if_fail (hash != NULL, NULL);
connection = nm_simple_connection_new ();
if (!nm_connection_replace_settings (connection, hash, error)) {
g_object_unref (connection);
return NULL;
g_hash_table_iter_init (&iter, hash);
while (g_hash_table_iter_next (&iter, (gpointer) &setting_name, (gpointer) &setting_hash)) {
NMSetting *setting;
GType type;
type = nm_setting_lookup_type (setting_name);
if (type == G_TYPE_INVALID) {
g_set_error (error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_INVALID_SETTING,
"unknown setting name '%s'", setting_name);
goto failed;
}
setting = _nm_setting_new_from_dbus (type, setting_hash);
nm_connection_add_setting (connection, setting);
}
return connection;
if (nm_connection_verify (connection, error))
return connection;
failed:
g_object_unref (connection);
return NULL;
}
/**

53
libnm-core/tests/test-general.c
View file

@ -991,6 +991,58 @@ test_connection_replace_settings_from_connection ()
g_object_unref (connection);
}
static void
test_connection_replace_settings_bad (void)
{
NMConnection *connection, *clone, *new_connection;
GHashTable *new_settings;
GError *error = NULL;
gboolean success;
NMSettingConnection *s_con;
NMSettingWired *s_wired;
connection = new_test_connection ();
clone = nm_simple_connection_new_clone (connection);
g_assert (nm_connection_compare (connection, clone, NM_SETTING_COMPARE_FLAG_EXACT));
new_connection = new_test_connection ();
g_assert (nm_connection_verify (new_connection, NULL));
s_con = nm_connection_get_setting_connection (new_connection);
g_object_set (s_con,
NM_SETTING_CONNECTION_UUID, NULL,
NM_SETTING_CONNECTION_ID, "bad-connection",
NULL);
g_assert (!nm_connection_verify (new_connection, NULL));
s_wired = nm_connection_get_setting_wired (new_connection);
g_object_set (s_wired,
NM_SETTING_WIRED_MTU, 12,
NULL);
/* nm_connection_replace_settings_from_connection() should fail */
success = nm_connection_replace_settings_from_connection (connection, new_connection, &error);
g_assert (error != NULL);
g_assert (!success);
g_clear_error (&error);
g_assert (nm_connection_compare (connection, clone, NM_SETTING_COMPARE_FLAG_EXACT));
/* nm_connection_replace_settings() should fail */
new_settings = nm_connection_to_dbus (new_connection, NM_CONNECTION_SERIALIZE_ALL);
g_assert (new_settings != NULL);
success = nm_connection_replace_settings (connection, new_settings, &error);
g_assert (error != NULL);
g_assert (!success);
g_clear_error (&error);
g_assert (nm_connection_compare (connection, clone, NM_SETTING_COMPARE_FLAG_EXACT));
g_hash_table_unref (new_settings);
g_object_unref (connection);
g_object_unref (clone);
g_object_unref (new_connection);
}
static void
test_connection_new_from_dbus ()
{
@ -3118,6 +3170,7 @@ int main (int argc, char **argv)
g_test_add_func ("/core/general/test_setting_new_from_dbus", test_setting_new_from_dbus);
g_test_add_func ("/core/general/test_connection_replace_settings", test_connection_replace_settings);
g_test_add_func ("/core/general/test_connection_replace_settings_from_connection", test_connection_replace_settings_from_connection);
g_test_add_func ("/core/general/test_connection_replace_settings_bad", test_connection_replace_settings_bad);
g_test_add_func ("/core/general/test_connection_new_from_dbus", test_connection_new_from_dbus);
g_test_add_func ("/core/general/test_connection_normalize_connection_interface_name", test_connection_normalize_connection_interface_name);
g_test_add_func ("/core/general/test_connection_normalize_virtual_iface_name", test_connection_normalize_virtual_iface_name);

1
libnm/libnm.ver
View file

@ -75,6 +75,7 @@ global:
nm_connection_add_setting;
nm_connection_clear_secrets;
nm_connection_clear_secrets_with_flags;
nm_connection_clear_settings;
nm_connection_compare;
nm_connection_diff;
nm_connection_dump;

6
libnm/nm-remote-connection.c
View file

@ -457,14 +457,10 @@ updated_get_settings_cb (DBusGProxy *proxy,
DBUS_TYPE_G_MAP_OF_MAP_OF_VARIANT, &new_settings,
G_TYPE_INVALID);
if (error) {
GHashTable *hash;
g_error_free (error);
/* Connection is no longer visible to this user. */
hash = g_hash_table_new (g_str_hash, g_str_equal);
nm_connection_replace_settings (NM_CONNECTION (self), hash, NULL);
g_hash_table_destroy (hash);
nm_connection_clear_settings (NM_CONNECTION (self));
visible = FALSE;
} else {