diff --git a/clients/cli/connections.c b/clients/cli/connections.c index cb94f4f34..89ff12b7f 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -7377,8 +7377,7 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t } else { /* Save/update already saved (existing) connection */ nm_connection_replace_settings_from_connection (NM_CONNECTION (rem_con), - connection, - NULL); + connection); update_connection (persistent, rem_con, update_connection_editor_cb, NULL); } @@ -7417,8 +7416,7 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t /* Update settings in the local connection */ nm_connection_replace_settings_from_connection (connection, - NM_CONNECTION (con_tmp), - NULL); + NM_CONNECTION (con_tmp)); /* Also update setting for menu context and TAB-completion */ menu_ctx.curr_setting = s_name ? nm_connection_get_setting_by_name (connection, s_name) : NULL; diff --git a/clients/tui/nm-editor-bindings.c b/clients/tui/nm-editor-bindings.c index 1b0bd4a6c..e3b210dec 100644 --- a/clients/tui/nm-editor-bindings.c +++ b/clients/tui/nm-editor-bindings.c @@ -230,11 +230,9 @@ ip4_addresses_check_and_copy (GBinding *binding, strings = g_value_get_boxed (source_value); - if (strings) { - for (i = 0; strings[i]; i++) { - if (!ip_string_parse (strings[i], AF_INET, &addr, NULL)) - return FALSE; - } + for (i = 0; strings[i]; i++) { + if (!ip_string_parse (strings[i], AF_INET, &addr, NULL)) + return FALSE; } g_value_set_boxed (target_value, strings); @@ -692,11 +690,9 @@ ip6_addresses_check_and_copy (GBinding *binding, strings = g_value_get_boxed (source_value); - if (strings) { - for (i = 0; strings[i]; i++) { - if (!ip_string_parse (strings[i], AF_INET6, &addr, NULL)) - return FALSE; - } + for (i = 0; strings[i]; i++) { + if (!ip_string_parse (strings[i], AF_INET6, &addr, NULL)) + return FALSE; } g_value_set_boxed (target_value, strings); diff --git a/clients/tui/nmt-editor.c b/clients/tui/nmt-editor.c index a69b762de..ab364f4b3 100644 --- a/clients/tui/nmt-editor.c +++ b/clients/tui/nmt-editor.c @@ -128,13 +128,8 @@ save_connection_and_exit (NmtNewtButton *button, NmtSyncOp op; GError *error = NULL; - if (!nm_connection_replace_settings_from_connection (priv->orig_connection, - priv->edit_connection, - &error)) { - nmt_newt_message_dialog (_("Error saving connection: %s"), error->message); - g_error_free (error); - return; - } + nm_connection_replace_settings_from_connection (priv->orig_connection, + priv->edit_connection); nmt_sync_op_init (&op); if (NM_IS_REMOTE_CONNECTION (priv->orig_connection)) { diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c index e02ccf98f..553230440 100644 --- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -274,32 +274,68 @@ validate_permissions_type (GHashTable *hash, GError **error) * @new_settings: (element-type utf8 GLib.HashTable): a #GHashTable of settings * @error: location to store error, or %NULL * - * Returns: %TRUE if the settings were valid and added to the connection, %FALSE - * if they were not (in which case @connection will be unchanged). + * Replaces @connection's settings with @new_settings (which must be + * syntactically valid, and describe a known type of connection, but does not + * need to result in a connection that passes nm_connection_verify()). + * + * Returns: %TRUE if connection was updated, %FALSE if @new_settings could not + * be deserialized (in which case @connection will be unchanged). **/ gboolean nm_connection_replace_settings (NMConnection *connection, GHashTable *new_settings, GError **error) { - NMConnection *new; - gboolean valid; + NMConnectionPrivate *priv; + GHashTableIter iter; + const char *setting_name; + GHashTable *setting_hash; + GSList *settings = NULL, *s; + gboolean changed; g_return_val_if_fail (NM_IS_CONNECTION (connection), FALSE); g_return_val_if_fail (new_settings != NULL, FALSE); g_return_val_if_fail (error == NULL || *error == NULL, FALSE); + priv = NM_CONNECTION_GET_PRIVATE (connection); + if (!validate_permissions_type (new_settings, error)) return FALSE; - new = nm_simple_connection_new_from_dbus (new_settings, error); - if (!new) - return FALSE; + g_hash_table_iter_init (&iter, new_settings); + while (g_hash_table_iter_next (&iter, (gpointer) &setting_name, (gpointer) &setting_hash)) { + NMSetting *setting; + GType type; - valid = nm_connection_replace_settings_from_connection (connection, new, error); - g_object_unref (new); + 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); + g_slist_free_full (settings, g_object_unref); + return FALSE; + } - g_return_val_if_fail (valid == TRUE, FALSE); + setting = _nm_setting_new_from_dbus (type, setting_hash, new_settings, error); + if (!setting) { + g_slist_free_full (settings, g_object_unref); + return FALSE; + } + settings = g_slist_prepend (settings, setting); + } + + if (g_hash_table_size (priv->settings) > 0) { + g_hash_table_foreach_remove (priv->settings, _setting_release, connection); + changed = TRUE; + } else + changed = (settings != NULL); + + for (s = settings; s; s = s->next) + nm_connection_add_setting (connection, s->data); + + if (changed) + g_signal_emit (connection, signals[CHANGED], 0); return TRUE; } @@ -307,35 +343,27 @@ nm_connection_replace_settings (NMConnection *connection, * nm_connection_replace_settings_from_connection: * @connection: a #NMConnection * @new_connection: a #NMConnection to replace the settings of @connection with - * @error: location to store error, or %NULL * - * Deep-copies the settings of @new_conenction and replaces the settings of @connection + * Deep-copies the settings of @new_connection and replaces the settings of @connection * with the copied settings. - * - * Returns: %TRUE if the settings were valid and added to the connection, %FALSE - * if they were not (in which case @connection will be unchanged). **/ -gboolean +void nm_connection_replace_settings_from_connection (NMConnection *connection, - NMConnection *new_connection, - GError **error) + NMConnection *new_connection) { NMConnectionPrivate *priv, *new_priv; GHashTableIter iter; NMSetting *setting; gboolean changed; - g_return_val_if_fail (NM_IS_CONNECTION (connection), FALSE); - g_return_val_if_fail (NM_IS_CONNECTION (new_connection), FALSE); - g_return_val_if_fail (error == NULL || *error == NULL, FALSE); - - if (!nm_connection_verify (new_connection, error)) - return FALSE; + g_return_if_fail (NM_IS_CONNECTION (connection)); + g_return_if_fail (NM_IS_CONNECTION (new_connection)); /* When 'connection' and 'new_connection' are the same object simply return - * in order not to destroy 'connection' */ + * in order not to destroy 'connection'. + */ if (connection == new_connection) - return TRUE; + return; /* No need to validate permissions like nm_connection_replace_settings() * since we're dealing with an NMConnection which has already done that. @@ -354,12 +382,8 @@ nm_connection_replace_settings_from_connection (NMConnection *connection, changed = TRUE; } - /* 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 TRUE; } /** diff --git a/libnm-core/nm-connection.h b/libnm-core/nm-connection.h index 1baae8771..3987f61b4 100644 --- a/libnm-core/nm-connection.h +++ b/libnm-core/nm-connection.h @@ -162,9 +162,8 @@ gboolean nm_connection_replace_settings (NMConnection *connection, GHashTable *new_settings, GError **error); -gboolean nm_connection_replace_settings_from_connection (NMConnection *connection, - NMConnection *new_connection, - GError **error); +void nm_connection_replace_settings_from_connection (NMConnection *connection, + NMConnection *new_connection); void nm_connection_clear_settings (NMConnection *connection); diff --git a/libnm-core/nm-setting-bond.c b/libnm-core/nm-setting-bond.c index 22fa19f3e..22ef5dd21 100644 --- a/libnm-core/nm-setting-bond.c +++ b/libnm-core/nm-setting-bond.c @@ -740,5 +740,5 @@ nm_setting_bond_class_init (NMSettingBondClass *setting_class) _nm_setting_class_add_dbus_only_property (parent_class, "interface-name", G_TYPE_STRING, _nm_setting_get_deprecated_virtual_interface_name, - _nm_setting_set_deprecated_virtual_interface_name); + NULL); } diff --git a/libnm-core/nm-setting-bridge.c b/libnm-core/nm-setting-bridge.c index cd682a49d..49bca2e81 100644 --- a/libnm-core/nm-setting-bridge.c +++ b/libnm-core/nm-setting-bridge.c @@ -480,5 +480,5 @@ nm_setting_bridge_class_init (NMSettingBridgeClass *setting_class) _nm_setting_class_add_dbus_only_property (parent_class, "interface-name", G_TYPE_STRING, _nm_setting_get_deprecated_virtual_interface_name, - _nm_setting_set_deprecated_virtual_interface_name); + NULL); } diff --git a/libnm-core/nm-setting-connection.c b/libnm-core/nm-setting-connection.c index 9fea63b78..403c03653 100644 --- a/libnm-core/nm-setting-connection.c +++ b/libnm-core/nm-setting-connection.c @@ -948,17 +948,12 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) return TRUE; } -static gboolean -nm_setting_connection_no_interface_name (NMSetting *setting, - GHashTable *connection_hash, - const char *property, - GError **error) +static const char * +find_virtual_interface_name (GHashTable *connection_hash) { GHashTable *setting_hash; - const char *interface_name; GValue *value; - /* Check if there's a deprecated virtual interface-name property to steal */ setting_hash = g_hash_table_lookup (connection_hash, NM_SETTING_BOND_SETTING_NAME); if (!setting_hash) setting_hash = g_hash_table_lookup (connection_hash, NM_SETTING_BRIDGE_SETTING_NAME); @@ -968,30 +963,48 @@ nm_setting_connection_no_interface_name (NMSetting *setting, setting_hash = g_hash_table_lookup (connection_hash, NM_SETTING_VLAN_SETTING_NAME); if (!setting_hash) - return TRUE; + return NULL; /* All of the deprecated virtual interface name properties were named "interface-name". */ value = g_hash_table_lookup (setting_hash, "interface-name"); - if (!value) - return TRUE; + if (!value || !G_VALUE_HOLDS_STRING (value)) + return NULL; - interface_name = g_value_get_string (value); - if (!interface_name) - return TRUE; + return g_value_get_string (value); +} - if (!nm_utils_iface_valid_name (interface_name)) { - g_set_error_literal (error, - NM_SETTING_CONNECTION_ERROR, - NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY, - _("property is invalid")); - g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_INTERFACE_NAME); - return FALSE; - } +static void +nm_setting_connection_set_interface_name (NMSetting *setting, + GHashTable *connection_hash, + const char *property, + const GValue *value) +{ + const char *interface_name; + + /* For compatibility reasons, if there is an invalid virtual interface name, + * we need to make verification fail, even if that virtual name would be + * overridden by a valid connection.interface-name. + */ + interface_name = find_virtual_interface_name (connection_hash); + if (!interface_name || nm_utils_iface_valid_name (interface_name)) + interface_name = g_value_get_string (value); g_object_set (G_OBJECT (setting), NM_SETTING_CONNECTION_INTERFACE_NAME, interface_name, NULL); - return TRUE; +} + +static void +nm_setting_connection_no_interface_name (NMSetting *setting, + GHashTable *connection_hash, + const char *property) +{ + const char *virtual_interface_name; + + virtual_interface_name = find_virtual_interface_name (connection_hash); + g_object_set (G_OBJECT (setting), + NM_SETTING_CONNECTION_INTERFACE_NAME, virtual_interface_name, + NULL); } static gboolean @@ -1256,7 +1269,8 @@ nm_setting_connection_class_init (NMSettingConnectionClass *setting_class) G_PARAM_STATIC_STRINGS)); _nm_setting_class_override_property (parent_class, NM_SETTING_CONNECTION_INTERFACE_NAME, G_TYPE_STRING, - NULL, NULL, + NULL, + nm_setting_connection_set_interface_name, nm_setting_connection_no_interface_name); /** diff --git a/libnm-core/nm-setting-private.h b/libnm-core/nm-setting-private.h index aad7e5169..e7c141a05 100644 --- a/libnm-core/nm-setting-private.h +++ b/libnm-core/nm-setting-private.h @@ -108,11 +108,6 @@ gboolean _nm_setting_get_deprecated_virtual_interface_name (NMSetting *setting, NMConnection *connection, const char *property, GValue *value); -gboolean _nm_setting_set_deprecated_virtual_interface_name (NMSetting *setting, - GHashTable *connection_hash, - const char *property, - const GValue *value, - GError **error); NMSettingVerifyResult _nm_setting_verify (NMSetting *setting, GSList *all_settings, @@ -135,15 +130,13 @@ typedef gboolean (*NMSettingPropertyGetFunc) (NMSetting *setting, NMConnection *connection, const char *property, GValue *value); -typedef gboolean (*NMSettingPropertySetFunc) (NMSetting *setting, +typedef void (*NMSettingPropertySetFunc) (NMSetting *setting, GHashTable *connection_hash, const char *property, - const GValue *value, - GError **error); -typedef gboolean (*NMSettingPropertyNotSetFunc) (NMSetting *setting, + const GValue *value); +typedef void (*NMSettingPropertyNotSetFunc) (NMSetting *setting, GHashTable *connection_hash, - const char *property, - GError **error); + const char *property); void _nm_setting_class_add_dbus_only_property (NMSettingClass *setting_class, const char *property_name, diff --git a/libnm-core/nm-setting-team.c b/libnm-core/nm-setting-team.c index 5946ec75a..8fb94c399 100644 --- a/libnm-core/nm-setting-team.c +++ b/libnm-core/nm-setting-team.c @@ -185,5 +185,5 @@ nm_setting_team_class_init (NMSettingTeamClass *setting_class) _nm_setting_class_add_dbus_only_property (parent_class, "interface-name", G_TYPE_STRING, _nm_setting_get_deprecated_virtual_interface_name, - _nm_setting_set_deprecated_virtual_interface_name); + NULL); } diff --git a/libnm-core/nm-setting-vlan.c b/libnm-core/nm-setting-vlan.c index f1781027d..55262264b 100644 --- a/libnm-core/nm-setting-vlan.c +++ b/libnm-core/nm-setting-vlan.c @@ -779,5 +779,5 @@ nm_setting_vlan_class_init (NMSettingVlanClass *setting_class) _nm_setting_class_add_dbus_only_property (parent_class, "interface-name", G_TYPE_STRING, _nm_setting_get_deprecated_virtual_interface_name, - _nm_setting_set_deprecated_virtual_interface_name); + NULL); } diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index 535706d82..04eee7511 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -274,19 +274,6 @@ nm_setting_lookup_type_by_quark (GQuark error_quark) return G_TYPE_INVALID; } -static GQuark -_nm_setting_lookup_error_quark (const char *name) -{ - SettingInfo *info; - - g_return_val_if_fail (name != NULL, 0); - - _ensure_registered (); - - info = g_hash_table_lookup (registered_settings, name); - return info ? info->error_quark : 0; -} - gint _nm_setting_compare_priority (gconstpointer a, gconstpointer b) { @@ -764,7 +751,7 @@ _nm_setting_to_dbus (NMSetting *setting, NMConnection *connection, NMConnectionS * property names and value types. * * Returns: a new #NMSetting object populated with the properties from the - * hash table, or %NULL on failure + * hash table, or %NULL if @setting_hash could not be deserialized. **/ NMSetting * _nm_setting_new_from_dbus (GType setting_type, @@ -807,24 +794,14 @@ _nm_setting_new_from_dbus (GType setting_type, GValue *value = g_hash_table_lookup (setting_hash, property->name); if (value && property->set_func) { - if (!property->set_func (setting, - connection_hash, - property->name, - value, - error)) { - g_object_unref (setting); - setting = NULL; - break; - } + property->set_func (setting, + connection_hash, + property->name, + value); } else if (!value && property->not_set_func) { - if (!property->not_set_func (setting, - connection_hash, - property->name, - error)) { - g_object_unref (setting); - setting = NULL; - break; - } + property->not_set_func (setting, + connection_hash, + property->name); } else if (value && property->param_spec) { if (!(property->param_spec->flags & G_PARAM_WRITABLE)) continue; @@ -1748,47 +1725,6 @@ _nm_setting_get_deprecated_virtual_interface_name (NMSetting *setting, return FALSE; } -gboolean -_nm_setting_set_deprecated_virtual_interface_name (NMSetting *setting, - GHashTable *connection_hash, - const char *property, - const GValue *value, - GError **error) -{ - const char *interface_name; - GQuark error_domain; - char *error_enum_name; - GEnumClass *enum_class; - GEnumValue *enum_val; - int error_code = 0; - - /* If the virtual setting type hash contains an interface name, it must be - * valid (even if it's going to be ignored in favor of - * NMSettingConnection:interface-name). Other than that, we don't have to - * check anything here; NMSettingConnection:interface-name will do the rest. - */ - interface_name = g_value_get_string (value); - if (!interface_name || nm_utils_iface_valid_name (interface_name)) - return TRUE; - - /* For compatibility reasons, we have to use the right error domain... */ - error_domain = _nm_setting_lookup_error_quark (nm_setting_get_name (setting)); - error_enum_name = g_strdup_printf ("%sError", G_OBJECT_TYPE_NAME (setting)); - enum_class = g_type_class_ref (g_type_from_name (error_enum_name)); - g_free (error_enum_name); - if (enum_class) { - enum_val = g_enum_get_value_by_nick (enum_class, "InvalidProperty"); - if (enum_val) - error_code = enum_val->value; - g_type_class_unref (enum_class); - } - - g_set_error_literal (error, error_domain, error_code, - _("invalid value in compatibility property")); - g_prefix_error (error, "%s.%s: ", nm_setting_get_name (setting), property); - return FALSE; -} - /*****************************************************************************/ static void diff --git a/libnm-core/nm-simple-connection.c b/libnm-core/nm-simple-connection.c index 0512cfbbe..3c05cccf4 100644 --- a/libnm-core/nm-simple-connection.c +++ b/libnm-core/nm-simple-connection.c @@ -66,40 +66,14 @@ 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 (); - - 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, hash, error); - if (!setting) - goto failed; - nm_connection_add_setting (connection, setting); - } - - if (nm_connection_verify (connection, error)) - return connection; - -failed: - g_object_unref (connection); - return NULL; + if ( !nm_connection_replace_settings (connection, hash, error) + || !nm_connection_verify (connection, error)) + g_clear_object (&connection); + return connection; } /** @@ -120,7 +94,7 @@ nm_simple_connection_new_clone (NMConnection *connection) clone = nm_simple_connection_new (); nm_connection_set_path (clone, nm_connection_get_path (connection)); - nm_connection_replace_settings_from_connection (clone, connection, NULL); + nm_connection_replace_settings_from_connection (clone, connection); return clone; } diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index a52822669..d38644060 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -618,6 +618,9 @@ _nm_utils_copy_array_to_slist (const GPtrArray *array, gpointer item; int i; + if (!array) + return NULL; + for (i = 0; i < array->len; i++) { item = array->pdata[i]; slist = g_slist_prepend (slist, copy_func (item)); @@ -634,6 +637,9 @@ _nm_utils_copy_array (const GPtrArray *array, GPtrArray *copy; int i; + if (!array) + return g_ptr_array_new_with_free_func (free_func); + copy = g_ptr_array_new_full (array->len, free_func); for (i = 0; i < array->len; i++) g_ptr_array_add (copy, copy_func (array->pdata[i])); @@ -681,8 +687,10 @@ _nm_utils_strv_to_slist (char **strv) int i; GSList *list = NULL; - for (i = 0; strv && strv[i]; i++) - list = g_slist_prepend (list, g_strdup (strv[i])); + if (strv) { + for (i = 0; strv[i]; i++) + list = g_slist_prepend (list, g_strdup (strv[i])); + } return g_slist_reverse (list); } @@ -694,9 +702,6 @@ _nm_utils_slist_to_strv (GSList *slist) char **strv; int len, i = 0; - if (slist == NULL) - return NULL; - len = g_slist_length (slist); strv = g_new (char *, len + 1); diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index f0fe4ae0a..a46a3cf90 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -957,8 +957,6 @@ static void test_connection_replace_settings_from_connection () { NMConnection *connection, *replacement; - GError *error = NULL; - gboolean success; NMSettingConnection *s_con; NMSetting *setting; GBytes *ssid; @@ -996,9 +994,7 @@ test_connection_replace_settings_from_connection () nm_connection_add_setting (replacement, setting); /* Replace settings and test */ - success = nm_connection_replace_settings_from_connection (connection, replacement, &error); - g_assert_no_error (error); - g_assert (success); + nm_connection_replace_settings_from_connection (connection, replacement); s_con = nm_connection_get_setting_connection (connection); g_assert (s_con); @@ -1017,16 +1013,11 @@ test_connection_replace_settings_from_connection () static void test_connection_replace_settings_bad (void) { - NMConnection *connection, *clone, *new_connection; - GHashTable *new_settings; + NMConnection *connection, *new_connection; + GHashTable *new_settings, *setting_hash; 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)); @@ -1036,33 +1027,40 @@ test_connection_replace_settings_bad (void) 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); + /* nm_connection_replace_settings_from_connection() should succeed */ + connection = new_test_connection (); + nm_connection_replace_settings_from_connection (connection, new_connection); + g_assert_cmpstr (nm_connection_get_id (connection), ==, "bad-connection"); + g_assert (!nm_connection_verify (connection, NULL)); + g_object_unref (connection); - g_assert (nm_connection_compare (connection, clone, NM_SETTING_COMPARE_FLAG_EXACT)); - - /* nm_connection_replace_settings() should fail */ + /* nm_connection_replace_settings() should succeed */ new_settings = nm_connection_to_dbus (new_connection, NM_CONNECTION_SERIALIZE_ALL); g_assert (new_settings != NULL); + connection = new_test_connection (); success = nm_connection_replace_settings (connection, new_settings, &error); - g_assert (error != NULL); - g_assert (!success); - g_clear_error (&error); + g_assert_no_error (error); + g_assert (success); - g_assert (nm_connection_compare (connection, clone, NM_SETTING_COMPARE_FLAG_EXACT)); + g_assert_cmpstr (nm_connection_get_id (connection), ==, "bad-connection"); + g_assert (!nm_connection_verify (connection, NULL)); + g_object_unref (connection); + + /* But given an invalid hash, it should fail */ + setting_hash = g_hash_table_new (g_str_hash, g_str_equal); + g_hash_table_insert (new_settings, g_strdup ("ip-over-avian-carrier"), setting_hash); + + connection = new_test_connection (); + success = nm_connection_replace_settings (connection, new_settings, &error); + g_assert_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_SETTING); + g_assert (!success); + + g_assert (nm_connection_verify (connection, NULL)); + g_object_unref (connection); g_hash_table_unref (new_settings); - g_object_unref (connection); - g_object_unref (clone); g_object_unref (new_connection); } @@ -2684,12 +2682,10 @@ test_connection_normalize_virtual_iface_name (void) g_assert (G_VALUE_HOLDS_STRING (value)); g_assert_cmpstr (g_value_get_string (value), ==, IFACE_NAME); - /* If vlan.interface-name is invalid, deserialization should fail, with the - * correct error. - */ + /* If vlan.interface-name is invalid, deserialization will fail. */ g_value_set_string (value, ":::this-is-not-a-valid-interface-name:::"); con = nm_simple_connection_new_from_dbus (connection_hash, &error); - g_assert_error (error, NM_SETTING_VLAN_ERROR, NM_SETTING_VLAN_ERROR_INVALID_PROPERTY); + g_assert_error (error, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY); g_clear_error (&error); /* If vlan.interface-name is valid, but doesn't match, it will be ignored. */ diff --git a/libnm/nm-object.c b/libnm/nm-object.c index 8a014a65b..6f5232192 100644 --- a/libnm/nm-object.c +++ b/libnm/nm-object.c @@ -554,11 +554,18 @@ _nm_object_create (GType type, DBusGConnection *connection, const char *path) object = g_object_new (type, NM_OBJECT_PATH, path, NULL); - if (NM_IS_OBJECT (object)) - _nm_object_cache_add (NM_OBJECT (object)); + /* Cache the object before initializing it (and in particular, loading its + * property values); this is necessary to make circular references work (eg, + * when creating an NMActiveConnection, it will create an NMDevice which + * will in turn try to create the parent NMActiveConnection). Since we don't + * support multi-threaded use, we know that we will have inited the object + * before any external code sees it. + */ + _nm_object_cache_add (NM_OBJECT (object)); if (!g_initable_init (G_INITABLE (object), NULL, &error)) { dbgmsg ("Could not create object for %s: %s", path, error->message); g_error_free (error); + g_clear_object (&object); } return object; @@ -586,12 +593,26 @@ async_inited (GObject *object, GAsyncResult *result, gpointer user_data) NMObjectTypeAsyncData *async_data = user_data; GError *error = NULL; - if (!g_async_initable_init_finish (G_ASYNC_INITABLE (object), result, &error)) { + if (g_async_initable_init_finish (G_ASYNC_INITABLE (object), result, &error)) { + NMObject *cached; + + /* Unlike in the sync case, in the async case we can't cache the object + * until it is completely initialized. But that means someone else might + * have been creating it at the same time, and they might have finished + * before us. + */ + cached = _nm_object_cache_get (nm_object_get_path (NM_OBJECT (object))); + if (cached) { + g_object_unref (object); + object = G_OBJECT (cached); + } else + _nm_object_cache_add (NM_OBJECT (object)); + } else { dbgmsg ("Could not create object for %s: %s", nm_object_get_path (NM_OBJECT (object)), error->message); g_error_free (error); - object = NULL; + g_clear_object (&object); } create_async_complete (object, async_data); @@ -603,17 +624,6 @@ async_got_type (GType type, gpointer user_data) NMObjectTypeAsyncData *async_data = user_data; GObject *object; - /* Ensure we don't have the object already; we may get multiple type - * requests for the same object if there are multiple properties on - * other objects that refer to the object at this path. One of those - * other requests may have already completed. - */ - object = (GObject *) _nm_object_cache_get (async_data->path); - if (object) { - create_async_complete (object, async_data); - return; - } - if (type == G_TYPE_INVALID) { /* Don't know how to create this object */ create_async_complete (NULL, async_data); @@ -623,8 +633,6 @@ async_got_type (GType type, gpointer user_data) object = g_object_new (type, NM_OBJECT_PATH, async_data->path, NULL); - if (NM_IS_OBJECT (object)) - _nm_object_cache_add (NM_OBJECT (object)); g_async_initable_init_async (G_ASYNC_INITABLE (object), G_PRIORITY_DEFAULT, NULL, async_inited, async_data); } diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 9e9f56b28..513e6efc1 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -463,38 +463,35 @@ nm_settings_connection_replace_settings (NMSettingsConnection *self, */ g_signal_handlers_block_by_func (self, G_CALLBACK (changed_cb), GUINT_TO_POINTER (TRUE)); - if (nm_connection_replace_settings_from_connection (NM_CONNECTION (self), - new_connection, - error)) { - priv->nm_generated = FALSE; + nm_connection_replace_settings_from_connection (NM_CONNECTION (self), new_connection); + priv->nm_generated = FALSE; - /* Cache the just-updated system secrets in case something calls - * nm_connection_clear_secrets() and clears them. - */ - update_system_secrets_cache (self); - success = TRUE; + /* Cache the just-updated system secrets in case something calls + * nm_connection_clear_secrets() and clears them. + */ + update_system_secrets_cache (self); + success = TRUE; - /* Add agent and always-ask secrets back; they won't necessarily be - * in the replacement connection data if it was eg reread from disk. - */ - if (priv->agent_secrets) { - hash = nm_connection_to_dbus (priv->agent_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS); - if (hash) { - (void) nm_connection_update_secrets (NM_CONNECTION (self), NULL, hash, NULL); - g_hash_table_destroy (hash); - } + /* Add agent and always-ask secrets back; they won't necessarily be + * in the replacement connection data if it was eg reread from disk. + */ + if (priv->agent_secrets) { + hash = nm_connection_to_dbus (priv->agent_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS); + if (hash) { + (void) nm_connection_update_secrets (NM_CONNECTION (self), NULL, hash, NULL); + g_hash_table_destroy (hash); } - - nm_settings_connection_recheck_visibility (self); - - /* Manually emit changed signal since we disconnected the handler, but - * only update Unsaved if the caller wanted us to. - */ - changed_cb (self, GUINT_TO_POINTER (update_unsaved)); - - g_signal_emit (self, signals[UPDATED_BY_USER], 0); } + nm_settings_connection_recheck_visibility (self); + + /* Manually emit changed signal since we disconnected the handler, but + * only update Unsaved if the caller wanted us to. + */ + changed_cb (self, GUINT_TO_POINTER (update_unsaved)); + + g_signal_emit (self, signals[UPDATED_BY_USER], 0); + g_signal_handlers_unblock_by_func (self, G_CALLBACK (changed_cb), GUINT_TO_POINTER (TRUE)); return success;