From 98e4a2be3004287de1b25d12e2ca71800b9859e4 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 5 Aug 2014 16:37:02 -0400 Subject: [PATCH 1/8] libnm-core, libnm-util: remove some useless code in nm-settings.c g_object_class_list_properties() can't return NULL if called correctly. Also remove two failed attempts to use g_value_transform(): nm_setting_new_from_hash() was transforming src_value to its own type (rather than to param_spec->value_type, which was presumably intended), so it was a no-op (in addition to being unnecessary anyway, since GObject will attempt to transform the value internally if needed). And update_one_secret() was calling g_value_transform() on an uninitialized GValue, so it would have always hit a g_return_val_if_fail() in g_value_transform() if that code was ever reached (which apparently it wasn't). --- libnm-core/nm-setting.c | 22 ++-------------------- libnm-util/nm-setting.c | 22 ++-------------------- 2 files changed, 4 insertions(+), 40 deletions(-) diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index a29b6678ea..44e9ca255f 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -402,11 +402,6 @@ nm_setting_to_hash (NMSetting *setting, NMSettingHashFlags flags) g_return_val_if_fail (NM_IS_SETTING (setting), NULL); property_specs = g_object_class_list_properties (G_OBJECT_GET_CLASS (setting), &n_property_specs); - if (!property_specs) { - g_warning ("%s: couldn't find property specs for object of type '%s'", - __func__, g_type_name (G_OBJECT_TYPE (setting))); - return NULL; - } hash = g_hash_table_new_full (g_str_hash, g_str_equal, (GDestroyNotify) g_free, destroy_gvalue); @@ -502,13 +497,8 @@ nm_setting_new_from_hash (GType setting_type, GHashTable *hash) continue; g_value_init (dst_value, G_VALUE_TYPE (src_value)); - if (g_value_transform (src_value, dst_value)) - params[n_params++].name = prop_name; - else { - g_warning ("Ignoring property '%s' with invalid type (%s)", - prop_name, G_VALUE_TYPE_NAME (src_value)); - g_value_unset (dst_value); - } + g_value_copy (src_value, dst_value); + params[n_params++].name = prop_name; } setting = (NMSetting *) g_object_newv (setting_type, n_params, params); @@ -1074,7 +1064,6 @@ static int update_one_secret (NMSetting *setting, const char *key, GValue *value, GError **error) { GParamSpec *prop_spec; - GValue transformed_value = G_VALUE_INIT; prop_spec = g_object_class_find_property (G_OBJECT_GET_CLASS (setting), key); if (!prop_spec) { @@ -1106,11 +1095,6 @@ update_one_secret (NMSetting *setting, const char *key, GValue *value, GError ** g_object_set_property (G_OBJECT (setting), prop_spec->name, 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); - return NM_SETTING_UPDATE_SECRET_SUCCESS_MODIFIED; - } g_set_error (error, NM_SETTING_ERROR, NM_SETTING_ERROR_PROPERTY_TYPE_MISMATCH, @@ -1310,8 +1294,6 @@ nm_setting_to_string (NMSetting *setting) g_return_val_if_fail (NM_IS_SETTING (setting), NULL); property_specs = g_object_class_list_properties (G_OBJECT_GET_CLASS (setting), &n_property_specs); - if (!property_specs) - return NULL; string = g_string_new (nm_setting_get_name (setting)); g_string_append_c (string, '\n'); diff --git a/libnm-util/nm-setting.c b/libnm-util/nm-setting.c index b943369e2e..2508a401fa 100644 --- a/libnm-util/nm-setting.c +++ b/libnm-util/nm-setting.c @@ -304,11 +304,6 @@ nm_setting_to_hash (NMSetting *setting, NMSettingHashFlags flags) g_return_val_if_fail (NM_IS_SETTING (setting), NULL); property_specs = g_object_class_list_properties (G_OBJECT_GET_CLASS (setting), &n_property_specs); - if (!property_specs) { - g_warning ("%s: couldn't find property specs for object of type '%s'", - __func__, g_type_name (G_OBJECT_TYPE (setting))); - return NULL; - } hash = g_hash_table_new_full (g_str_hash, g_str_equal, (GDestroyNotify) g_free, destroy_gvalue); @@ -402,13 +397,8 @@ nm_setting_new_from_hash (GType setting_type, GHashTable *hash) } g_value_init (dst_value, G_VALUE_TYPE (src_value)); - if (g_value_transform (src_value, dst_value)) - params[n_params++].name = prop_name; - else { - g_warning ("Ignoring property '%s' with invalid type (%s)", - prop_name, G_VALUE_TYPE_NAME (src_value)); - g_value_unset (dst_value); - } + g_value_copy (src_value, dst_value); + params[n_params++].name = prop_name; } setting = (NMSetting *) g_object_newv (setting_type, n_params, params); @@ -974,7 +964,6 @@ static int update_one_secret (NMSetting *setting, const char *key, GValue *value, GError **error) { GParamSpec *prop_spec; - GValue transformed_value = G_VALUE_INIT; prop_spec = g_object_class_find_property (G_OBJECT_GET_CLASS (setting), key); if (!prop_spec) { @@ -1006,11 +995,6 @@ update_one_secret (NMSetting *setting, const char *key, GValue *value, GError ** g_object_set_property (G_OBJECT (setting), prop_spec->name, 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); - return NM_SETTING_UPDATE_SECRET_SUCCESS_MODIFIED; - } g_set_error (error, NM_SETTING_ERROR, NM_SETTING_ERROR_PROPERTY_TYPE_MISMATCH, @@ -1210,8 +1194,6 @@ nm_setting_to_string (NMSetting *setting) g_return_val_if_fail (NM_IS_SETTING (setting), NULL); property_specs = g_object_class_list_properties (G_OBJECT_GET_CLASS (setting), &n_property_specs); - if (!property_specs) - return NULL; string = g_string_new (nm_setting_get_name (setting)); g_string_append_c (string, '\n'); From c9653a9e67ed746e0388c38900a6e2de65f8a6bf Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Mon, 4 Aug 2014 11:23:11 -0400 Subject: [PATCH 2/8] libnm-core: make the NMSetting hash methods private Make nm_setting_to_hash() and nm_setting_new_from_hash() private, and remove the public nm_setting_update_secrets() wrapper around the existing private _nm_setting_update_secrets(). These functions should really only be called from the corresponding NMConnection-level methods, and in particular, with certain compatibility properties in the future, we will need to consider the entire connection all at once when setting properties, so it won't make sense to serialize/deserialize a single setting in isolation. --- libnm-core/nm-connection.c | 4 ++-- libnm-core/nm-setting-private.h | 6 ++++++ libnm-core/nm-setting.c | 18 ++++++------------ libnm-core/nm-setting.h | 9 --------- libnm-core/tests/test-general.c | 10 +++++----- libnm/libnm.ver | 3 --- 6 files changed, 19 insertions(+), 31 deletions(-) diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c index 8a7aa10cb8..41155a1053 100644 --- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -308,7 +308,7 @@ hash_to_connection (NMConnection *connection, GHashTable *new, GError **error) GType type = nm_setting_lookup_type (setting_name); if (type) { - NMSetting *setting = nm_setting_new_from_hash (type, setting_hash); + NMSetting *setting = _nm_setting_new_from_hash (type, setting_hash); if (setting) { _nm_connection_add_setting (connection, setting); @@ -1264,7 +1264,7 @@ nm_connection_to_hash (NMConnection *connection, NMSettingHashFlags flags) while (g_hash_table_iter_next (&iter, &key, &data)) { NMSetting *setting = NM_SETTING (data); - setting_hash = nm_setting_to_hash (setting, flags); + setting_hash = _nm_setting_to_hash (setting, flags); if (setting_hash) g_hash_table_insert (ret, g_strdup (nm_setting_get_name (setting)), setting_hash); } diff --git a/libnm-core/nm-setting-private.h b/libnm-core/nm-setting-private.h index 4c316864b1..1f589f1ffd 100644 --- a/libnm-core/nm-setting-private.h +++ b/libnm-core/nm-setting-private.h @@ -114,4 +114,10 @@ NMSetting *_nm_setting_find_in_list_base_type (GSList *all_settings); gboolean _nm_setting_slave_type_is_valid (const char *slave_type, const char **out_port_type); const char * _nm_setting_slave_type_detect_from_settings (GSList *all_settings, NMSetting **out_s_port); +GHashTable *_nm_setting_to_hash (NMSetting *setting, + NMSettingHashFlags flags); + +NMSetting *_nm_setting_new_from_hash (GType setting_type, + GHashTable *hash); + #endif /* NM_SETTING_PRIVATE_H */ diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index 44e9ca255f..a783f4890e 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -380,7 +380,7 @@ destroy_gvalue (gpointer data) } /** - * nm_setting_to_hash: + * _nm_setting_to_hash: * @setting: the #NMSetting * @flags: hash flags, e.g. %NM_SETTING_HASH_FLAG_ALL * @@ -392,7 +392,7 @@ destroy_gvalue (gpointer data) * describing the setting's properties **/ GHashTable * -nm_setting_to_hash (NMSetting *setting, NMSettingHashFlags flags) +_nm_setting_to_hash (NMSetting *setting, NMSettingHashFlags flags) { GHashTable *hash; GParamSpec **property_specs; @@ -444,7 +444,7 @@ nm_setting_to_hash (NMSetting *setting, NMSettingHashFlags flags) } /** - * nm_setting_new_from_hash: + * _nm_setting_new_from_hash: * @setting_type: the #NMSetting type which the hash contains properties for * @hash: (element-type utf8 GObject.Value): the #GHashTable containing a * string to GValue mapping of properties that apply to the setting @@ -460,7 +460,7 @@ nm_setting_to_hash (NMSetting *setting, NMSettingHashFlags flags) * hash table, or %NULL on failure **/ NMSetting * -nm_setting_new_from_hash (GType setting_type, GHashTable *hash) +_nm_setting_new_from_hash (GType setting_type, GHashTable *hash) { GHashTableIter iter; NMSetting *setting; @@ -1103,7 +1103,7 @@ update_one_secret (NMSetting *setting, const char *key, GValue *value, GError ** } /** - * nm_setting_update_secrets: + * _nm_setting_update_secrets: * @setting: the #NMSetting * @secrets: (element-type utf8 GObject.Value): a #GHashTable mapping * string to #GValue of setting property names and secrets @@ -1112,15 +1112,9 @@ update_one_secret (NMSetting *setting, const char *key, GValue *value, GError ** * Update the setting's secrets, given a hash table of secrets intended for that * setting (deserialized from D-Bus for example). * - * Returns: %TRUE if the secrets were successfully updated, %FALSE on failure to + * Returns: an #NMSettingUpdateSecretResult * update one or more of the secrets. **/ -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) { diff --git a/libnm-core/nm-setting.h b/libnm-core/nm-setting.h index 2420cb49a8..a92aadff40 100644 --- a/libnm-core/nm-setting.h +++ b/libnm-core/nm-setting.h @@ -248,12 +248,6 @@ typedef enum { NM_SETTING_HASH_FLAG_ONLY_SECRETS = 0x00000002, } NMSettingHashFlags; -GHashTable *nm_setting_to_hash (NMSetting *setting, - NMSettingHashFlags flags); - -NMSetting *nm_setting_new_from_hash (GType setting_type, - GHashTable *hash); - NMSetting *nm_setting_duplicate (NMSetting *setting); const char *nm_setting_get_name (NMSetting *setting); @@ -300,9 +294,6 @@ void nm_setting_clear_secrets_with_flags (NMSetting *setting, gpointer user_data); GPtrArray *nm_setting_need_secrets (NMSetting *setting); -gboolean nm_setting_update_secrets (NMSetting *setting, - GHashTable *secrets, - GError **error); gboolean nm_setting_get_secret_flags (NMSetting *setting, const char *secret_name, diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 2d26440b91..3048e41807 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -680,7 +680,7 @@ test_setting_to_hash_all (void) s_wsec = make_test_wsec_setting ("setting-to-hash-all"); - hash = nm_setting_to_hash (NM_SETTING (s_wsec), NM_SETTING_HASH_FLAG_ALL); + hash = _nm_setting_to_hash (NM_SETTING (s_wsec), NM_SETTING_HASH_FLAG_ALL); /* Make sure all keys are there */ ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT), @@ -704,7 +704,7 @@ test_setting_to_hash_no_secrets (void) s_wsec = make_test_wsec_setting ("setting-to-hash-no-secrets"); - hash = nm_setting_to_hash (NM_SETTING (s_wsec), NM_SETTING_HASH_FLAG_NO_SECRETS); + hash = _nm_setting_to_hash (NM_SETTING (s_wsec), NM_SETTING_HASH_FLAG_NO_SECRETS); /* Make sure non-secret keys are there */ ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT), @@ -730,7 +730,7 @@ test_setting_to_hash_only_secrets (void) s_wsec = make_test_wsec_setting ("setting-to-hash-only-secrets"); - hash = nm_setting_to_hash (NM_SETTING (s_wsec), NM_SETTING_HASH_FLAG_ONLY_SECRETS); + hash = _nm_setting_to_hash (NM_SETTING (s_wsec), NM_SETTING_HASH_FLAG_ONLY_SECRETS); /* Make sure non-secret keys are there */ ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT) == NULL, @@ -778,10 +778,10 @@ test_setting_new_from_hash (void) GHashTable *hash; s_wsec = make_test_wsec_setting ("setting-to-hash-all"); - hash = nm_setting_to_hash (NM_SETTING (s_wsec), NM_SETTING_HASH_FLAG_ALL); + hash = _nm_setting_to_hash (NM_SETTING (s_wsec), NM_SETTING_HASH_FLAG_ALL); g_object_unref (s_wsec); - s_wsec = (NMSettingWirelessSecurity *) nm_setting_new_from_hash (NM_TYPE_SETTING_WIRELESS_SECURITY, hash); + s_wsec = (NMSettingWirelessSecurity *) _nm_setting_new_from_hash (NM_TYPE_SETTING_WIRELESS_SECURITY, hash); g_hash_table_destroy (hash); g_assert (s_wsec); diff --git a/libnm/libnm.ver b/libnm/libnm.ver index 760bb42748..ec6c48fad0 100644 --- a/libnm/libnm.ver +++ b/libnm/libnm.ver @@ -676,7 +676,6 @@ global: nm_setting_lookup_type; nm_setting_lookup_type_by_quark; nm_setting_need_secrets; - nm_setting_new_from_hash; nm_setting_olpc_mesh_error_get_type; nm_setting_olpc_mesh_error_quark; nm_setting_olpc_mesh_get_channel; @@ -736,9 +735,7 @@ global: nm_setting_team_port_get_config; nm_setting_team_port_get_type; nm_setting_team_port_new; - nm_setting_to_hash; nm_setting_to_string; - nm_setting_update_secrets; nm_setting_verify; nm_setting_vlan_add_priority; nm_setting_vlan_add_priority_str; From 773d3f0ab69a2d9faba8d031adb63233ba794e9b Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 6 Aug 2014 19:35:31 -0400 Subject: [PATCH 3/8] libnm-core: rename NMConnection to/from_hash methods Rename nm_connection_to_hash() to nm_connection_to_dbus(), and nm_connection_new_from_hash() to nm_connection_new_from_dbus(). In addition to clarifying that this is specifically the D-Bus serialization format, these names will also work better in the GDBus-based future where the serialization format is GVariant, not GHashTable. Also, move NMSettingHashFlags to nm-connection.h, and rename it NMConnectionSerializationFlags. --- callouts/tests/test-dispatcher-envp.c | 2 +- clients/cli/connections.c | 2 +- clients/tui/nmt-editor.c | 2 +- examples/C/glib/add-connection-dbus-glib.c | 2 +- .../C/glib/get-active-connections-dbus-glib.c | 2 +- libnm-core/nm-connection.c | 22 +++--- libnm-core/nm-connection.h | 21 +++++- libnm-core/nm-setting-private.h | 7 +- libnm-core/nm-setting.c | 14 ++-- libnm-core/nm-setting.h | 16 ---- libnm-core/nm-simple-connection.c | 6 +- libnm-core/nm-simple-connection.h | 2 +- libnm-core/tests/test-general.c | 74 +++++++++---------- libnm-core/tests/test-secrets.c | 6 +- libnm/libnm.ver | 6 +- libnm/nm-client.c | 2 +- libnm/nm-remote-connection.c | 4 +- libnm/nm-remote-settings.c | 4 +- libnm/nm-secret-agent.c | 2 +- libnm/nm-secret-agent.h | 4 +- libnm/nm-vpn-plugin.c | 6 +- src/nm-dispatcher.c | 2 +- src/settings/nm-agent-manager.c | 2 +- src/settings/nm-secret-agent.c | 10 +-- src/settings/nm-settings-connection.c | 12 +-- src/settings/nm-settings.c | 2 +- src/vpn-manager/nm-vpn-connection.c | 4 +- 27 files changed, 119 insertions(+), 119 deletions(-) diff --git a/callouts/tests/test-dispatcher-envp.c b/callouts/tests/test-dispatcher-envp.c index 9d9ee52a4d..05c2e8ed9c 100644 --- a/callouts/tests/test-dispatcher-envp.c +++ b/callouts/tests/test-dispatcher-envp.c @@ -167,7 +167,7 @@ parse_main (GKeyFile *kf, g_free (id); nm_connection_add_setting (connection, NM_SETTING (s_con)); - *out_con_hash = nm_connection_to_hash (connection, NM_SETTING_HASH_FLAG_ALL); + *out_con_hash = nm_connection_to_dbus (connection, NM_CONNECTION_SERIALIZE_ALL); g_object_unref (connection); *out_con_props = value_hash_create (); diff --git a/clients/cli/connections.c b/clients/cli/connections.c index a1f4a923b8..59d851a162 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -5438,7 +5438,7 @@ gen_cmd_print0 (const char *text, int state) const char *setting_name; int i = 0; - settings = nm_connection_to_hash (nmc_tab_completion.connection, NM_SETTING_HASH_FLAG_NO_SECRETS); + settings = nm_connection_to_dbus (nmc_tab_completion.connection, NM_CONNECTION_SERIALIZE_NO_SECRETS); words = g_new (char *, g_hash_table_size (settings) + 2); g_hash_table_iter_init (&iter, settings); while (g_hash_table_iter_next (&iter, (gpointer) &setting_name, NULL)) diff --git a/clients/tui/nmt-editor.c b/clients/tui/nmt-editor.c index 50c8588ad4..f8b4c6d25b 100644 --- a/clients/tui/nmt-editor.c +++ b/clients/tui/nmt-editor.c @@ -188,7 +188,7 @@ build_edit_connection (NMConnection *orig_connection) if (!NM_IS_REMOTE_CONNECTION (orig_connection)) return edit_connection; - settings = nm_connection_to_hash (orig_connection, NM_SETTING_HASH_FLAG_NO_SECRETS); + settings = nm_connection_to_dbus (orig_connection, NM_CONNECTION_SERIALIZE_NO_SECRETS); g_hash_table_iter_init (&iter, settings); while (g_hash_table_iter_next (&iter, (gpointer) &setting_name, NULL)) { nmt_sync_op_init (&op); diff --git a/examples/C/glib/add-connection-dbus-glib.c b/examples/C/glib/add-connection-dbus-glib.c index beb0beca4d..d4ac7be096 100644 --- a/examples/C/glib/add-connection-dbus-glib.c +++ b/examples/C/glib/add-connection-dbus-glib.c @@ -70,7 +70,7 @@ add_connection (DBusGProxy *proxy, const char *con_name) NULL); nm_connection_add_setting (connection, NM_SETTING (s_ip4)); - hash = nm_connection_to_hash (connection, NM_SETTING_HASH_FLAG_ALL); + hash = nm_connection_to_dbus (connection, NM_CONNECTION_SERIALIZE_ALL); /* Call AddConnection with the hash as argument */ if (!dbus_g_proxy_call (proxy, "AddConnection", &error, diff --git a/examples/C/glib/get-active-connections-dbus-glib.c b/examples/C/glib/get-active-connections-dbus-glib.c index 0e81ca9c1c..e7aaff7bb3 100644 --- a/examples/C/glib/get-active-connections-dbus-glib.c +++ b/examples/C/glib/get-active-connections-dbus-glib.c @@ -66,7 +66,7 @@ print_connection (DBusGConnection *bus, const char *path) /* Using the raw configuration, create an NMConnection object for it. This * step also verifies that the data we got from NetworkManager are valid. */ - connection = nm_simple_connection_new_from_hash (hash, &error); + connection = nm_simple_connection_new_from_dbus (hash, &error); if (!connection) { g_warning ("Received invalid connection data: %s", error->message); g_error_free (error); diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c index 41155a1053..e958668a6e 100644 --- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -308,7 +308,7 @@ hash_to_connection (NMConnection *connection, GHashTable *new, GError **error) GType type = nm_setting_lookup_type (setting_name); if (type) { - NMSetting *setting = _nm_setting_new_from_hash (type, setting_hash); + NMSetting *setting = _nm_setting_new_from_dbus (type, setting_hash); if (setting) { _nm_connection_add_setting (connection, setting); @@ -983,8 +983,8 @@ nm_connection_normalize (NMConnection *connection, * Update the specified setting's secrets, given a hash table of secrets * intended for that setting (deserialized from D-Bus for example). Will also * extract the given setting's secrets hash if given a hash of hashes, as would - * be returned from nm_connection_to_hash(). If @setting_name is %NULL, expects - * a fully serialized #NMConnection as returned by nm_connection_to_hash() and + * be returned from nm_connection_to_dbus(). If @setting_name is %NULL, expects + * a fully serialized #NMConnection as returned by nm_connection_to_dbus() and * will update all secrets from all settings contained in @secrets. * * Returns: %TRUE if the secrets were successfully updated, %FALSE if the update @@ -1228,15 +1228,15 @@ nm_connection_clear_secrets_with_flags (NMConnection *connection, } /** - * nm_connection_to_hash: + * nm_connection_to_dbus: * @connection: the #NMConnection - * @flags: hash flags, e.g. %NM_SETTING_HASH_FLAG_ALL + * @flags: serialization flags, e.g. %NM_CONNECTION_SERIALIZE_ALL * * Converts the #NMConnection into a #GHashTable describing the connection, - * suitable for marshalling over D-Bus or serializing. The hash table mapping - * is string:#GHashTable with each element in the returned hash representing - * a #NMSetting object. The keys are setting object names, and the values - * are #GHashTables mapping string:GValue, each of which represents the + * suitable for marshalling over D-Bus or otherwise serializing. The hash table + * mapping is string:#GHashTable with each element in the returned hash + * representing a #NMSetting object. The keys are setting object names, and the + * values are #GHashTables mapping string:GValue, each of which represents the * properties of the #NMSetting object. * * Returns: (transfer full) (element-type utf8 GLib.HashTable): a new @@ -1245,7 +1245,7 @@ nm_connection_clear_secrets_with_flags (NMConnection *connection, * with g_hash_table_unref() when it is no longer needed. **/ GHashTable * -nm_connection_to_hash (NMConnection *connection, NMSettingHashFlags flags) +nm_connection_to_dbus (NMConnection *connection, NMConnectionSerializationFlags flags) { NMConnectionPrivate *priv; GHashTableIter iter; @@ -1264,7 +1264,7 @@ nm_connection_to_hash (NMConnection *connection, NMSettingHashFlags flags) while (g_hash_table_iter_next (&iter, &key, &data)) { NMSetting *setting = NM_SETTING (data); - setting_hash = _nm_setting_to_hash (setting, flags); + setting_hash = _nm_setting_to_dbus (setting, flags); if (setting_hash) g_hash_table_insert (ret, g_strdup (nm_setting_get_name (setting)), setting_hash); } diff --git a/libnm-core/nm-connection.h b/libnm-core/nm-connection.h index b06660ae9d..a15451e3e1 100644 --- a/libnm-core/nm-connection.h +++ b/libnm-core/nm-connection.h @@ -140,6 +140,24 @@ NMSetting *nm_connection_get_setting (NMConnection *connection, NMSetting *nm_connection_get_setting_by_name (NMConnection *connection, const char *name); +/** + * NMConnectionSerializationFlags: + * @NM_CONNECTION_SERIALIZE_ALL: serialize all properties (including secrets) + * @NM_CONNECTION_SERIALIZE_NO_SECRETS: do not include secrets + * @NM_CONNECTION_SERIALIZE_ONLY_SECRETS: only serialize secrets + * + * These flags determine which properties are serialized when calling when + * calling nm_connection_to_dbus(). + **/ +typedef enum { /*< flags >*/ + NM_CONNECTION_SERIALIZE_ALL = 0x00000000, + NM_CONNECTION_SERIALIZE_NO_SECRETS = 0x00000001, + NM_CONNECTION_SERIALIZE_ONLY_SECRETS = 0x00000002, +} NMConnectionSerializationFlags; + +GHashTable *nm_connection_to_dbus (NMConnection *connection, + NMConnectionSerializationFlags flags); + gboolean nm_connection_replace_settings (NMConnection *connection, GHashTable *new_settings, GError **error); @@ -192,9 +210,6 @@ void nm_connection_for_each_setting_value (NMConnection *connection, NMSettingValueIterFn func, gpointer user_data); -GHashTable *nm_connection_to_hash (NMConnection *connection, - NMSettingHashFlags flags); - void nm_connection_dump (NMConnection *connection); /* Helpers */ diff --git a/libnm-core/nm-setting-private.h b/libnm-core/nm-setting-private.h index 1f589f1ffd..de9628e7b1 100644 --- a/libnm-core/nm-setting-private.h +++ b/libnm-core/nm-setting-private.h @@ -22,6 +22,7 @@ #define __NM_SETTING_PRIVATE_H__ #include "nm-setting.h" +#include "nm-connection.h" #include "nm-glib-compat.h" #include "nm-core-internal.h" @@ -114,10 +115,10 @@ NMSetting *_nm_setting_find_in_list_base_type (GSList *all_settings); gboolean _nm_setting_slave_type_is_valid (const char *slave_type, const char **out_port_type); const char * _nm_setting_slave_type_detect_from_settings (GSList *all_settings, NMSetting **out_s_port); -GHashTable *_nm_setting_to_hash (NMSetting *setting, - NMSettingHashFlags flags); +GHashTable *_nm_setting_to_dbus (NMSetting *setting, + NMConnectionSerializationFlags flags); -NMSetting *_nm_setting_new_from_hash (GType setting_type, +NMSetting *_nm_setting_new_from_dbus (GType setting_type, GHashTable *hash); #endif /* NM_SETTING_PRIVATE_H */ diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index a783f4890e..9e268977e4 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -380,9 +380,9 @@ destroy_gvalue (gpointer data) } /** - * _nm_setting_to_hash: + * _nm_setting_to_dbus: * @setting: the #NMSetting - * @flags: hash flags, e.g. %NM_SETTING_HASH_FLAG_ALL + * @flags: hash flags, e.g. %NM_CONNECTION_SERIALIZE_ALL * * Converts the #NMSetting into a #GHashTable mapping each setting property * name to a GValue describing that property, suitable for marshalling over @@ -392,7 +392,7 @@ destroy_gvalue (gpointer data) * describing the setting's properties **/ GHashTable * -_nm_setting_to_hash (NMSetting *setting, NMSettingHashFlags flags) +_nm_setting_to_dbus (NMSetting *setting, NMConnectionSerializationFlags flags) { GHashTable *hash; GParamSpec **property_specs; @@ -414,11 +414,11 @@ _nm_setting_to_hash (NMSetting *setting, NMSettingHashFlags flags) if (strcmp (g_param_spec_get_name (prop_spec), NM_SETTING_NAME) == 0) continue; - if ( (flags & NM_SETTING_HASH_FLAG_NO_SECRETS) + if ( (flags & NM_CONNECTION_SERIALIZE_NO_SECRETS) && (prop_spec->flags & NM_SETTING_PARAM_SECRET)) continue; - if ( (flags & NM_SETTING_HASH_FLAG_ONLY_SECRETS) + if ( (flags & NM_CONNECTION_SERIALIZE_ONLY_SECRETS) && !(prop_spec->flags & NM_SETTING_PARAM_SECRET)) continue; @@ -444,7 +444,7 @@ _nm_setting_to_hash (NMSetting *setting, NMSettingHashFlags flags) } /** - * _nm_setting_new_from_hash: + * _nm_setting_new_from_dbus: * @setting_type: the #NMSetting type which the hash contains properties for * @hash: (element-type utf8 GObject.Value): the #GHashTable containing a * string to GValue mapping of properties that apply to the setting @@ -460,7 +460,7 @@ _nm_setting_to_hash (NMSetting *setting, NMSettingHashFlags flags) * hash table, or %NULL on failure **/ NMSetting * -_nm_setting_new_from_hash (GType setting_type, GHashTable *hash) +_nm_setting_new_from_dbus (GType setting_type, GHashTable *hash) { GHashTableIter iter; NMSetting *setting; diff --git a/libnm-core/nm-setting.h b/libnm-core/nm-setting.h index a92aadff40..deb894e892 100644 --- a/libnm-core/nm-setting.h +++ b/libnm-core/nm-setting.h @@ -232,22 +232,6 @@ GType nm_setting_get_type (void); GType nm_setting_lookup_type (const char *name); GType nm_setting_lookup_type_by_quark (GQuark error_quark); -/** - * NMSettingHashFlags: - * @NM_SETTING_HASH_FLAG_ALL: hash all properties (including secrets) - * @NM_SETTING_HASH_FLAG_NO_SECRETS: do not include secrets - * @NM_SETTING_HASH_FLAG_ONLY_SECRETS: only hash secrets - * - * These flags determine which properties are added to the resulting hash - * when calling nm_setting_to_hash(). - * - **/ -typedef enum { - NM_SETTING_HASH_FLAG_ALL = 0x00000000, - NM_SETTING_HASH_FLAG_NO_SECRETS = 0x00000001, - NM_SETTING_HASH_FLAG_ONLY_SECRETS = 0x00000002, -} NMSettingHashFlags; - NMSetting *nm_setting_duplicate (NMSetting *setting); const char *nm_setting_get_name (NMSetting *setting); diff --git a/libnm-core/nm-simple-connection.c b/libnm-core/nm-simple-connection.c index bb398179de..ccd7fccf8a 100644 --- a/libnm-core/nm-simple-connection.c +++ b/libnm-core/nm-simple-connection.c @@ -48,13 +48,13 @@ nm_simple_connection_new (void) } /** - * nm_simple_connection_new_from_hash: + * nm_simple_connection_new_from_dbus: * @hash: (element-type utf8 GLib.HashTable): the #GHashTable describing * the connection * @error: on unsuccessful return, an error * * Creates a new #NMSimpleConnection from a hash table describing the - * connection. See nm_connection_to_hash() for a description of the expected + * connection. See nm_connection_to_dbus() for a description of the expected * hash table. * * Returns: (transfer full): the new #NMSimpleConnection object, populated with @@ -62,7 +62,7 @@ nm_simple_connection_new (void) * connection failed to validate **/ NMConnection * -nm_simple_connection_new_from_hash (GHashTable *hash, GError **error) +nm_simple_connection_new_from_dbus (GHashTable *hash, GError **error) { NMConnection *connection; diff --git a/libnm-core/nm-simple-connection.h b/libnm-core/nm-simple-connection.h index e6daee66e4..b535ed9db9 100644 --- a/libnm-core/nm-simple-connection.h +++ b/libnm-core/nm-simple-connection.h @@ -51,7 +51,7 @@ GType nm_simple_connection_get_type (void); NMConnection *nm_simple_connection_new (void); -NMConnection *nm_simple_connection_new_from_hash (GHashTable *hash, +NMConnection *nm_simple_connection_new_from_dbus (GHashTable *hash, GError **error); NMConnection *nm_simple_connection_new_clone (NMConnection *connection); diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 3048e41807..0c88857052 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -673,115 +673,115 @@ make_test_wsec_setting (const char *detail) } static void -test_setting_to_hash_all (void) +test_setting_to_dbus_all (void) { NMSettingWirelessSecurity *s_wsec; GHashTable *hash; - s_wsec = make_test_wsec_setting ("setting-to-hash-all"); + s_wsec = make_test_wsec_setting ("setting-to-dbus-all"); - hash = _nm_setting_to_hash (NM_SETTING (s_wsec), NM_SETTING_HASH_FLAG_ALL); + hash = _nm_setting_to_dbus (NM_SETTING (s_wsec), NM_CONNECTION_SERIALIZE_ALL); /* Make sure all keys are there */ ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT), - "setting-to-hash-all", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_KEY_MGMT); + "setting-to-dbus-all", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_KEY_MGMT); ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_LEAP_USERNAME), - "setting-to-hash-all", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_LEAP_USERNAME); + "setting-to-dbus-all", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_LEAP_USERNAME); ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_PSK), - "setting-to-hash-all", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_PSK); + "setting-to-dbus-all", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_PSK); ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_WEP_KEY0), - "setting-to-hash-all", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_WEP_KEY0); + "setting-to-dbus-all", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_WEP_KEY0); g_hash_table_destroy (hash); g_object_unref (s_wsec); } static void -test_setting_to_hash_no_secrets (void) +test_setting_to_dbus_no_secrets (void) { NMSettingWirelessSecurity *s_wsec; GHashTable *hash; - s_wsec = make_test_wsec_setting ("setting-to-hash-no-secrets"); + s_wsec = make_test_wsec_setting ("setting-to-dbus-no-secrets"); - hash = _nm_setting_to_hash (NM_SETTING (s_wsec), NM_SETTING_HASH_FLAG_NO_SECRETS); + hash = _nm_setting_to_dbus (NM_SETTING (s_wsec), NM_CONNECTION_SERIALIZE_NO_SECRETS); /* Make sure non-secret keys are there */ ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT), - "setting-to-hash-no-secrets", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_KEY_MGMT); + "setting-to-dbus-no-secrets", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_KEY_MGMT); ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_LEAP_USERNAME), - "setting-to-hash-no-secrets", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_LEAP_USERNAME); + "setting-to-dbus-no-secrets", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_LEAP_USERNAME); /* Make sure secrets are not there */ ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_PSK) == NULL, - "setting-to-hash-no-secrets", "unexpectedly present " NM_SETTING_WIRELESS_SECURITY_PSK); + "setting-to-dbus-no-secrets", "unexpectedly present " NM_SETTING_WIRELESS_SECURITY_PSK); ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_WEP_KEY0) == NULL, - "setting-to-hash-no-secrets", "unexpectedly present " NM_SETTING_WIRELESS_SECURITY_WEP_KEY0); + "setting-to-dbus-no-secrets", "unexpectedly present " NM_SETTING_WIRELESS_SECURITY_WEP_KEY0); g_hash_table_destroy (hash); g_object_unref (s_wsec); } static void -test_setting_to_hash_only_secrets (void) +test_setting_to_dbus_only_secrets (void) { NMSettingWirelessSecurity *s_wsec; GHashTable *hash; - s_wsec = make_test_wsec_setting ("setting-to-hash-only-secrets"); + s_wsec = make_test_wsec_setting ("setting-to-dbus-only-secrets"); - hash = _nm_setting_to_hash (NM_SETTING (s_wsec), NM_SETTING_HASH_FLAG_ONLY_SECRETS); + hash = _nm_setting_to_dbus (NM_SETTING (s_wsec), NM_CONNECTION_SERIALIZE_ONLY_SECRETS); /* Make sure non-secret keys are there */ ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT) == NULL, - "setting-to-hash-only-secrets", "unexpectedly present " NM_SETTING_WIRELESS_SECURITY_KEY_MGMT); + "setting-to-dbus-only-secrets", "unexpectedly present " NM_SETTING_WIRELESS_SECURITY_KEY_MGMT); ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_LEAP_USERNAME) == NULL, - "setting-to-hash-only-secrets", "unexpectedly present " NM_SETTING_WIRELESS_SECURITY_LEAP_USERNAME); + "setting-to-dbus-only-secrets", "unexpectedly present " NM_SETTING_WIRELESS_SECURITY_LEAP_USERNAME); /* Make sure secrets are not there */ ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_PSK), - "setting-to-hash-only-secrets", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_PSK); + "setting-to-dbus-only-secrets", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_PSK); ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_WEP_KEY0), - "setting-to-hash-only-secrets", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_WEP_KEY0); + "setting-to-dbus-only-secrets", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_WEP_KEY0); g_hash_table_destroy (hash); g_object_unref (s_wsec); } static void -test_connection_to_hash_setting_name (void) +test_connection_to_dbus_setting_name (void) { NMConnection *connection; NMSettingWirelessSecurity *s_wsec; GHashTable *hash; connection = nm_simple_connection_new (); - s_wsec = make_test_wsec_setting ("connection-to-hash-setting-name"); + s_wsec = make_test_wsec_setting ("connection-to-dbus-setting-name"); nm_connection_add_setting (connection, NM_SETTING (s_wsec)); - hash = nm_connection_to_hash (connection, NM_SETTING_HASH_FLAG_ALL); + hash = nm_connection_to_dbus (connection, NM_CONNECTION_SERIALIZE_ALL); /* Make sure the keys of the first level hash are setting names, not * the GType name of the setting objects. */ ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_SETTING_NAME) != NULL, - "connection-to-hash-setting-name", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_SETTING_NAME); + "connection-to-dbus-setting-name", "unexpectedly missing " NM_SETTING_WIRELESS_SECURITY_SETTING_NAME); g_hash_table_destroy (hash); g_object_unref (connection); } static void -test_setting_new_from_hash (void) +test_setting_new_from_dbus (void) { NMSettingWirelessSecurity *s_wsec; GHashTable *hash; - s_wsec = make_test_wsec_setting ("setting-to-hash-all"); - hash = _nm_setting_to_hash (NM_SETTING (s_wsec), NM_SETTING_HASH_FLAG_ALL); + s_wsec = make_test_wsec_setting ("setting-to-dbus-all"); + hash = _nm_setting_to_dbus (NM_SETTING (s_wsec), NM_CONNECTION_SERIALIZE_ALL); g_object_unref (s_wsec); - s_wsec = (NMSettingWirelessSecurity *) _nm_setting_new_from_hash (NM_TYPE_SETTING_WIRELESS_SECURITY, hash); + s_wsec = (NMSettingWirelessSecurity *) _nm_setting_new_from_dbus (NM_TYPE_SETTING_WIRELESS_SECURITY, hash); g_hash_table_destroy (hash); g_assert (s_wsec); @@ -992,7 +992,7 @@ test_connection_replace_settings_from_connection () } static void -test_connection_new_from_hash () +test_connection_new_from_dbus () { NMConnection *connection; GHashTable *new_settings; @@ -1006,7 +1006,7 @@ test_connection_new_from_hash () g_assert (new_settings); /* Replace settings and test */ - connection = nm_simple_connection_new_from_hash (new_settings, &error); + connection = nm_simple_connection_new_from_dbus (new_settings, &error); g_assert_no_error (error); g_assert (connection); @@ -3096,9 +3096,9 @@ int main (int argc, char **argv) g_test_add_func ("/core/general/test_setting_gsm_apn_bad_chars", test_setting_gsm_apn_bad_chars); g_test_add_func ("/core/general/test_setting_gsm_apn_underscore", test_setting_gsm_apn_underscore); g_test_add_func ("/core/general/test_setting_gsm_without_number", test_setting_gsm_without_number); - g_test_add_func ("/core/general/test_setting_to_hash_all", test_setting_to_hash_all); - g_test_add_func ("/core/general/test_setting_to_hash_no_secrets", test_setting_to_hash_no_secrets); - g_test_add_func ("/core/general/test_setting_to_hash_only_secrets", test_setting_to_hash_only_secrets); + g_test_add_func ("/core/general/test_setting_to_dbus_all", test_setting_to_dbus_all); + g_test_add_func ("/core/general/test_setting_to_dbus_no_secrets", test_setting_to_dbus_no_secrets); + g_test_add_func ("/core/general/test_setting_to_dbus_only_secrets", test_setting_to_dbus_only_secrets); g_test_add_func ("/core/general/test_setting_compare_id", test_setting_compare_id); #define ADD_FUNC(func, secret_flags, comp_flags, remove_secret) \ g_test_add_data_func_full ("/core/general/" G_STRINGIFY (func), \ @@ -3114,11 +3114,11 @@ int main (int argc, char **argv) ADD_FUNC (test_setting_compare_vpn_secrets, NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_COMPARE_FLAG_EXACT, FALSE); g_test_add_func ("/core/general/test_setting_old_uuid", test_setting_old_uuid); - g_test_add_func ("/core/general/test_connection_to_hash_setting_name", test_connection_to_hash_setting_name); - g_test_add_func ("/core/general/test_setting_new_from_hash", test_setting_new_from_hash); + g_test_add_func ("/core/general/test_connection_to_dbus_setting_name", test_connection_to_dbus_setting_name); + 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_new_from_hash", test_connection_new_from_hash); + 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); g_test_add_func ("/core/general/test_connection_normalize_type", test_connection_normalize_type); diff --git a/libnm-core/tests/test-secrets.c b/libnm-core/tests/test-secrets.c index 590c2f1c57..087a30f0cc 100644 --- a/libnm-core/tests/test-secrets.c +++ b/libnm-core/tests/test-secrets.c @@ -595,7 +595,7 @@ test_update_secrets_whole_connection (void) connection = wifi_connection_new (); /* Build up the secrets hash */ - secrets = nm_connection_to_hash (connection, NM_SETTING_HASH_FLAG_ALL); + secrets = nm_connection_to_dbus (connection, NM_CONNECTION_SERIALIZE_ALL); wsec_hash = g_hash_table_lookup (secrets, NM_SETTING_WIRELESS_SECURITY_SETTING_NAME); g_assert (wsec_hash); g_hash_table_insert (wsec_hash, NM_SETTING_WIRELESS_SECURITY_WEP_KEY0, string_to_gvalue (wepkey)); @@ -645,7 +645,7 @@ test_update_secrets_whole_connection_bad_setting (void) connection = wifi_connection_new (); /* Build up the secrets hash */ - secrets = nm_connection_to_hash (connection, NM_SETTING_HASH_FLAG_ALL); + secrets = nm_connection_to_dbus (connection, NM_CONNECTION_SERIALIZE_ALL); wsec_hash = g_hash_table_lookup (secrets, NM_SETTING_WIRELESS_SECURITY_SETTING_NAME); g_assert (wsec_hash); g_hash_table_insert (wsec_hash, NM_SETTING_WIRELESS_SECURITY_WEP_KEY0, string_to_gvalue (wepkey)); @@ -678,7 +678,7 @@ test_update_secrets_whole_connection_empty_base_setting (void) */ connection = wifi_connection_new (); - secrets = nm_connection_to_hash (connection, NM_SETTING_HASH_FLAG_ONLY_SECRETS); + secrets = nm_connection_to_dbus (connection, NM_CONNECTION_SERIALIZE_ONLY_SECRETS); g_assert_cmpint (g_hash_table_size (secrets), ==, 1); g_assert (g_hash_table_lookup (secrets, NM_SETTING_WIRELESS_SETTING_NAME)); diff --git a/libnm/libnm.ver b/libnm/libnm.ver index ec6c48fad0..ed00474daf 100644 --- a/libnm/libnm.ver +++ b/libnm/libnm.ver @@ -123,8 +123,9 @@ global: nm_connection_remove_setting; nm_connection_replace_settings; nm_connection_replace_settings_from_connection; + nm_connection_serialization_flags_get_type; nm_connection_set_path; - nm_connection_to_hash; + nm_connection_to_dbus; nm_connection_update_secrets; nm_connection_verify; nm_connectivity_state_get_type; @@ -591,7 +592,6 @@ global: nm_setting_gsm_get_type; nm_setting_gsm_get_username; nm_setting_gsm_new; - nm_setting_hash_flags_get_type; nm_setting_infiniband_error_get_type; nm_setting_infiniband_error_quark; nm_setting_infiniband_get_mac_address; @@ -864,7 +864,7 @@ global: nm_simple_connection_get_type; nm_simple_connection_new; nm_simple_connection_new_clone; - nm_simple_connection_new_from_hash; + nm_simple_connection_new_from_dbus; nm_ssid_get_type; nm_state_get_type; nm_string_array_get_type; diff --git a/libnm/nm-client.c b/libnm/nm-client.c index c6133d6c22..c7a61e76cc 100644 --- a/libnm/nm-client.c +++ b/libnm/nm-client.c @@ -718,7 +718,7 @@ nm_client_add_and_activate_connection (NMClient *client, info->client = client; if (partial) - hash = nm_connection_to_hash (partial, NM_SETTING_HASH_FLAG_ALL); + hash = nm_connection_to_dbus (partial, NM_CONNECTION_SERIALIZE_ALL); if (!hash) hash = g_hash_table_new (g_str_hash, g_str_equal); diff --git a/libnm/nm-remote-connection.c b/libnm/nm-remote-connection.c index 835be01774..f821180ca1 100644 --- a/libnm/nm-remote-connection.c +++ b/libnm/nm-remote-connection.c @@ -221,7 +221,7 @@ nm_remote_connection_commit_changes (NMRemoteConnection *self, if (!call) return; - settings = nm_connection_to_hash (NM_CONNECTION (self), NM_SETTING_HASH_FLAG_ALL); + settings = nm_connection_to_dbus (NM_CONNECTION (self), NM_CONNECTION_SERIALIZE_ALL); call->call = dbus_g_proxy_begin_call (priv->proxy, "Update", remote_call_dbus_cb, call, NULL, DBUS_TYPE_G_MAP_OF_MAP_OF_VARIANT, settings, @@ -259,7 +259,7 @@ nm_remote_connection_commit_changes_unsaved (NMRemoteConnection *connection, if (!call) return; - settings = nm_connection_to_hash (NM_CONNECTION (connection), NM_SETTING_HASH_FLAG_ALL); + settings = nm_connection_to_dbus (NM_CONNECTION (connection), NM_CONNECTION_SERIALIZE_ALL); call->call = dbus_g_proxy_begin_call (priv->proxy, "UpdateUnsaved", remote_call_dbus_cb, call, NULL, DBUS_TYPE_G_MAP_OF_MAP_OF_VARIANT, settings, diff --git a/libnm/nm-remote-settings.c b/libnm/nm-remote-settings.c index 9fbc13c76d..3a5810d35e 100644 --- a/libnm/nm-remote-settings.c +++ b/libnm/nm-remote-settings.c @@ -485,7 +485,7 @@ nm_remote_settings_add_connection (NMRemoteSettings *settings, info->callback = callback; info->callback_data = user_data; - new_settings = nm_connection_to_hash (connection, NM_SETTING_HASH_FLAG_ALL); + new_settings = nm_connection_to_dbus (connection, NM_CONNECTION_SERIALIZE_ALL); dbus_g_proxy_begin_call (priv->proxy, "AddConnection", add_connection_done, info, @@ -538,7 +538,7 @@ nm_remote_settings_add_connection_unsaved (NMRemoteSettings *settings, info->callback = callback; info->callback_data = user_data; - new_settings = nm_connection_to_hash (connection, NM_SETTING_HASH_FLAG_ALL); + new_settings = nm_connection_to_dbus (connection, NM_CONNECTION_SERIALIZE_ALL); dbus_g_proxy_begin_call (priv->proxy, "AddConnectionUnsaved", add_connection_done, info, diff --git a/libnm/nm-secret-agent.c b/libnm/nm-secret-agent.c index ee8b25bef1..3fbc74286b 100644 --- a/libnm/nm-secret-agent.c +++ b/libnm/nm-secret-agent.c @@ -330,7 +330,7 @@ verify_request (NMSecretAgent *self, /* Make sure the given connection is valid */ g_assert (out_connection); - connection = nm_simple_connection_new_from_hash (connection_hash, &local); + connection = nm_simple_connection_new_from_dbus (connection_hash, &local); if (connection) { nm_connection_set_path (connection, connection_path); *out_connection = connection; diff --git a/libnm/nm-secret-agent.h b/libnm/nm-secret-agent.h index c70b21cbb7..ad62916bce 100644 --- a/libnm/nm-secret-agent.h +++ b/libnm/nm-secret-agent.h @@ -86,7 +86,7 @@ typedef struct { * has returned * @secrets: (element-type utf8 GLib.HashTable): the #GHashTable containing * the requested secrets in the same format as an #NMConnection hash (as - * created by nm_connection_to_hash() for example). Each key in @secrets + * created by nm_connection_to_dbus() for example). Each key in @secrets * should be the name of a #NMSetting object (like "802-11-wireless-security") * and each value should be a #GHashTable. The sub-hashes map string:#GValue * where the string is the setting property name (like "psk") and the value @@ -114,7 +114,7 @@ typedef struct { * NM_SETTING_WIRELESS_SECURITY_PSK, "my really cool PSK", * NULL); * nm_connection_add_setting (secrets, NM_SETTING (s_wsec)); - * secrets_hash = nm_connection_to_hash (secrets, NM_SETTING_HASH_FLAG_ALL); + * secrets_hash = nm_connection_to_dbus (secrets, NM_CONNECTION_SERIALIZE_ALL); * * (call the NMSecretAgentGetSecretsFunc with secrets_hash) * diff --git a/libnm/nm-vpn-plugin.c b/libnm/nm-vpn-plugin.c index d6b4633c3d..c47f61a5c8 100644 --- a/libnm/nm-vpn-plugin.c +++ b/libnm/nm-vpn-plugin.c @@ -451,7 +451,7 @@ _connect_generic (NMVpnPlugin *plugin, return FALSE; } - connection = nm_simple_connection_new_from_hash (properties, &local); + connection = nm_simple_connection_new_from_dbus (properties, &local); if (!connection) { g_set_error (error, NM_VPN_PLUGIN_ERROR, NM_VPN_PLUGIN_ERROR_BAD_ARGUMENTS, "Invalid connection: (%d) %s", @@ -525,7 +525,7 @@ impl_vpn_plugin_need_secrets (NMVpnPlugin *plugin, g_return_val_if_fail (NM_IS_VPN_PLUGIN (plugin), FALSE); g_return_val_if_fail (properties != NULL, FALSE); - connection = nm_simple_connection_new_from_hash (properties, &cnfh_err); + connection = nm_simple_connection_new_from_dbus (properties, &cnfh_err); if (!connection) { g_set_error (err, NM_VPN_PLUGIN_ERROR, @@ -580,7 +580,7 @@ impl_vpn_plugin_new_secrets (NMVpnPlugin *plugin, return FALSE; } - connection = nm_simple_connection_new_from_hash (properties, &local); + connection = nm_simple_connection_new_from_dbus (properties, &local); if (!connection) { g_set_error (error, NM_VPN_PLUGIN_ERROR, NM_VPN_PLUGIN_ERROR_BAD_ARGUMENTS, "Invalid connection: (%d) %s", diff --git a/src/nm-dispatcher.c b/src/nm-dispatcher.c index 141e17713f..752b1baf04 100644 --- a/src/nm-dispatcher.c +++ b/src/nm-dispatcher.c @@ -471,7 +471,7 @@ _dispatcher_call (DispatcherAction action, } if (connection) { - connection_hash = nm_connection_to_hash (connection, NM_SETTING_HASH_FLAG_NO_SECRETS); + connection_hash = nm_connection_to_dbus (connection, NM_CONNECTION_SERIALIZE_NO_SECRETS); connection_props = value_hash_create (); value_hash_add_object_path (connection_props, diff --git a/src/settings/nm-agent-manager.c b/src/settings/nm-agent-manager.c index def09f6309..dda550c5ce 100644 --- a/src/settings/nm-agent-manager.c +++ b/src/settings/nm-agent-manager.c @@ -1195,7 +1195,7 @@ nm_agent_manager_get_secrets (NMAgentManager *self, /* NOTE: a few things in the Request handling depend on existing_secrets * being NULL if there aren't any system-owned secrets for this connection. - * This in turn depends on nm_connection_to_hash() and nm_setting_to_hash() + * This in turn depends on nm_connection_to_dbus() and nm_setting_to_hash() * both returning NULL if they didn't hash anything. */ diff --git a/src/settings/nm-secret-agent.c b/src/settings/nm-secret-agent.c index b9c3619f4e..97728f804c 100644 --- a/src/settings/nm-secret-agent.c +++ b/src/settings/nm-secret-agent.c @@ -293,7 +293,7 @@ nm_secret_agent_get_secrets (NMSecretAgent *self, priv = NM_SECRET_AGENT_GET_PRIVATE (self); g_return_val_if_fail (priv->proxy != NULL, NULL); - hash = nm_connection_to_hash (connection, NM_SETTING_HASH_FLAG_ALL); + hash = nm_connection_to_dbus (connection, NM_CONNECTION_SERIALIZE_ALL); /* Mask off the private ONLY_SYSTEM flag if present */ flags &= ~NM_SECRET_AGENT_GET_SECRETS_FLAG_ONLY_SYSTEM; @@ -379,7 +379,7 @@ agent_save_delete_cb (DBusGProxy *proxy, static gpointer agent_new_save_delete (NMSecretAgent *self, NMConnection *connection, - NMSettingHashFlags hash_flags, + NMConnectionSerializationFlags flags, const char *method, NMSecretAgentCallback callback, gpointer callback_data) @@ -389,7 +389,7 @@ agent_new_save_delete (NMSecretAgent *self, Request *r; const char *cpath = nm_connection_get_path (connection); - hash = nm_connection_to_hash (connection, hash_flags); + hash = nm_connection_to_dbus (connection, flags); r = request_new (self, cpath, NULL, callback, callback_data); r->call = dbus_g_proxy_begin_call_with_timeout (priv->proxy, @@ -419,7 +419,7 @@ nm_secret_agent_save_secrets (NMSecretAgent *self, /* Caller should have ensured that only agent-owned secrets exist in 'connection' */ return agent_new_save_delete (self, connection, - NM_SETTING_HASH_FLAG_ALL, + NM_CONNECTION_SERIALIZE_ALL, "SaveSecrets", callback, callback_data); @@ -437,7 +437,7 @@ nm_secret_agent_delete_secrets (NMSecretAgent *self, /* No secrets sent; agents must be smart enough to track secrets using the UUID or something */ return agent_new_save_delete (self, connection, - NM_SETTING_HASH_FLAG_NO_SECRETS, + NM_CONNECTION_SERIALIZE_NO_SECRETS, "DeleteSecrets", callback, callback_data); diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index c534461959..33b74f9bc8 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -478,7 +478,7 @@ nm_settings_connection_replace_settings (NMSettingsConnection *self, * in the replacement connection data if it was eg reread from disk. */ if (priv->agent_secrets) { - hash = nm_connection_to_hash (priv->agent_secrets, NM_SETTING_HASH_FLAG_ONLY_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); @@ -811,7 +811,7 @@ agent_secrets_done_cb (NMAgentManager *manager, /* Update the connection with our existing secrets from backing storage */ nm_connection_clear_secrets (NM_CONNECTION (self)); - hash = nm_connection_to_hash (priv->system_secrets, NM_SETTING_HASH_FLAG_ONLY_SECRETS); + hash = nm_connection_to_dbus (priv->system_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS); if (!hash || nm_connection_update_secrets (NM_CONNECTION (self), setting_name, hash, &local)) { /* Update the connection with the agent's secrets; by this point if any * system-owned secrets exist in 'secrets' the agent that provided them @@ -915,7 +915,7 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self, return 0; } - existing_secrets = nm_connection_to_hash (priv->system_secrets, NM_SETTING_HASH_FLAG_ONLY_SECRETS); + existing_secrets = nm_connection_to_dbus (priv->system_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS); call_id = nm_agent_manager_get_secrets (priv->agent_mgr, NM_CONNECTION (self), subject, @@ -1181,7 +1181,7 @@ get_settings_auth_cb (NMSettingsConnection *self, * get returned by the GetSecrets method which can be better * protected against leakage of secrets to unprivileged callers. */ - settings = nm_connection_to_hash (NM_CONNECTION (dupl_con), NM_SETTING_HASH_FLAG_NO_SECRETS); + settings = nm_connection_to_dbus (NM_CONNECTION (dupl_con), NM_CONNECTION_SERIALIZE_NO_SECRETS); g_assert (settings); dbus_g_method_return (context, settings); g_hash_table_destroy (settings); @@ -1343,7 +1343,7 @@ impl_settings_connection_update_helper (NMSettingsConnection *self, /* Check if the settings are valid first */ if (new_settings) { - tmp = nm_simple_connection_new_from_hash (new_settings, &error); + tmp = nm_simple_connection_new_from_dbus (new_settings, &error); if (!tmp) { g_assert (error); goto error; @@ -1510,7 +1510,7 @@ dbus_get_agent_secrets_cb (NMSettingsConnection *self, * secrets from backing storage and those returned from the agent * by the time we get here. */ - hash = nm_connection_to_hash (NM_CONNECTION (self), NM_SETTING_HASH_FLAG_ONLY_SECRETS); + hash = nm_connection_to_dbus (NM_CONNECTION (self), NM_CONNECTION_SERIALIZE_ONLY_SECRETS); if (!hash) hash = g_hash_table_new (NULL, NULL); dbus_g_method_return (context, hash); diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 3e186eaa52..5499f43bb8 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -1224,7 +1224,7 @@ impl_settings_add_connection_helper (NMSettings *self, NMConnection *connection; GError *error = NULL; - connection = nm_simple_connection_new_from_hash (settings, &error); + connection = nm_simple_connection_new_from_dbus (settings, &error); if (connection) { nm_settings_add_connection_dbus (self, connection, diff --git a/src/vpn-manager/nm-vpn-connection.c b/src/vpn-manager/nm-vpn-connection.c index 5ccf294706..d11a1fe60a 100644 --- a/src/vpn-manager/nm-vpn-connection.c +++ b/src/vpn-manager/nm-vpn-connection.c @@ -1466,14 +1466,14 @@ _hash_with_username (NMConnection *connection, const char *username) g_assert (s_vpn); existing = nm_setting_vpn_get_user_name (s_vpn); if (username == NULL || existing) - return nm_connection_to_hash (connection, NM_SETTING_HASH_FLAG_ALL); + return nm_connection_to_dbus (connection, NM_CONNECTION_SERIALIZE_ALL); dup = nm_simple_connection_new_clone (connection); g_assert (dup); s_vpn = nm_connection_get_setting_vpn (dup); g_assert (s_vpn); g_object_set (s_vpn, NM_SETTING_VPN_USER_NAME, username, NULL); - hash = nm_connection_to_hash (dup, NM_SETTING_HASH_FLAG_ALL); + hash = nm_connection_to_dbus (dup, NM_CONNECTION_SERIALIZE_ALL); g_object_unref (dup); return hash; } From c191c38a5f972097b486ef48e89e9094355d35ed Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Mon, 4 Aug 2014 11:39:33 -0400 Subject: [PATCH 4/8] libnm-core: reorganize _nm_setting_new_from_dbus() Reorganize _nm_setting_new_from_dbus() to create an empty NMSetting first and then set each of its properties, rather than passing all of the properties to g_object_newv(). We don't need to pass them at construct time since no NMSetting properties are CONSTRUCT_ONLY, and organizing the function this way is a prereq for some later functionality (being able to run code when a property *isn't* present in the hash). --- libnm-core/nm-setting.c | 50 ++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index 9e268977e4..e8c8a1224e 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -462,14 +462,13 @@ _nm_setting_to_dbus (NMSetting *setting, NMConnectionSerializationFlags flags) NMSetting * _nm_setting_new_from_dbus (GType setting_type, GHashTable *hash) { - GHashTableIter iter; NMSetting *setting; - const char *prop_name; - GValue *src_value; GObjectClass *class; - guint n_params = 0; - GParameter *params; - int i; + GHashTableIter iter; + const char *prop_name; + GParamSpec **property_specs; + guint n_property_specs; + guint i; g_return_val_if_fail (G_TYPE_IS_INSTANTIATABLE (setting_type), NULL); g_return_val_if_fail (hash != NULL, NULL); @@ -478,35 +477,36 @@ _nm_setting_new_from_dbus (GType setting_type, GHashTable *hash) * already been used. */ class = g_type_class_ref (setting_type); - params = g_new0 (GParameter, g_hash_table_size (hash)); + /* Check for invalid properties first. */ g_hash_table_iter_init (&iter, hash); - while (g_hash_table_iter_next (&iter, (gpointer) &prop_name, (gpointer) &src_value)) { - GValue *dst_value = ¶ms[n_params].value; - GParamSpec *param_spec; - - param_spec = g_object_class_find_property (class, prop_name); - if (!param_spec) { + while (g_hash_table_iter_next (&iter, (gpointer) &prop_name, NULL)) { + if (!g_object_class_find_property (class, prop_name)) { /* Oh, we're so nice and only warn, maybe it should be a fatal error? */ g_warning ("Ignoring invalid property '%s'", prop_name); continue; } - - /* 'name' doesn't get deserialized */ - if (strcmp (g_param_spec_get_name (param_spec), NM_SETTING_NAME) == 0) - continue; - - g_value_init (dst_value, G_VALUE_TYPE (src_value)); - g_value_copy (src_value, dst_value); - params[n_params++].name = prop_name; } - setting = (NMSetting *) g_object_newv (setting_type, n_params, params); + /* Now build the setting object from the legitimate properties */ + setting = (NMSetting *) g_object_new (setting_type, NULL); - for (i = 0; i < n_params; i++) - g_value_unset (¶ms[i].value); + property_specs = g_object_class_list_properties (G_OBJECT_GET_CLASS (setting), &n_property_specs); + for (i = 0; i < n_property_specs; i++) { + GParamSpec *prop_spec = property_specs[i]; + GValue *value; - g_free (params); + /* 'name' doesn't get deserialized */ + if (strcmp (prop_spec->name, NM_SETTING_NAME) == 0) + continue; + + value = g_hash_table_lookup (hash, prop_spec->name); + if (!value) + continue; + + g_object_set_property (G_OBJECT (setting), prop_spec->name, value); + } + g_free (property_specs); g_type_class_unref (class); return setting; From 05f30681635157e07e7a5dd210b5c621a5a53fd9 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Fri, 22 Aug 2014 10:20:29 -0400 Subject: [PATCH 5/8] libnm-core: clarify nm_setting_lookup_type*() failure nm_setting_lookup_type() and nm_setting_lookup_type_by_quark() return G_TYPE_INVALID on failure. --- libnm-core/nm-setting.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index e8c8a1224e..748a65557e 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -231,14 +231,15 @@ _nm_setting_is_base_type (NMSetting *setting) * * Returns the #GType of the setting's class for a given setting name. * - * Returns: the #GType of the setting's class + * Returns: the #GType of the setting's class, or %G_TYPE_INVALID if + * @name is not recognized. **/ GType nm_setting_lookup_type (const char *name) { SettingInfo *info; - g_return_val_if_fail (name != NULL, G_TYPE_NONE); + g_return_val_if_fail (name != NULL, G_TYPE_INVALID); _ensure_registered (); @@ -253,7 +254,8 @@ nm_setting_lookup_type (const char *name) * Returns the #GType of the setting's class for a given setting error quark. * Useful for figuring out which setting a returned error is for. * - * Returns: the #GType of the setting's class + * Returns: the #GType of the setting's class, or %G_TYPE_INVALID if + * @error_quark is not recognized **/ GType nm_setting_lookup_type_by_quark (GQuark error_quark) From 504c292d7389e68e926a428cc5083359a6ba2e0b Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Fri, 22 Aug 2014 10:14:38 -0400 Subject: [PATCH 6/8] libnm-core: clean up nm_connection_replace_settings()'s semantics 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.) --- include/nm-test-utils.h | 5 -- libnm-core/nm-connection.c | 100 ++++++++++++++++-------------- libnm-core/nm-connection.h | 2 + libnm-core/nm-simple-connection.c | 32 ++++++++-- libnm-core/tests/test-general.c | 53 ++++++++++++++++ libnm/libnm.ver | 1 + libnm/nm-remote-connection.c | 6 +- 7 files changed, 137 insertions(+), 62 deletions(-) diff --git a/include/nm-test-utils.h b/include/nm-test-utils.h index 0bff1c2997..6fe2470ade 100644 --- a/include/nm-test-utils.h +++ b/include/nm-test-utils.h @@ -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 diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c index e958668a6e..4e72f35927 100644 --- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -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); + } } /** diff --git a/libnm-core/nm-connection.h b/libnm-core/nm-connection.h index a15451e3e1..c9d88334ae 100644 --- a/libnm-core/nm-connection.h +++ b/libnm-core/nm-connection.h @@ -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); diff --git a/libnm-core/nm-simple-connection.c b/libnm-core/nm-simple-connection.c index ccd7fccf8a..febacdc54c 100644 --- a/libnm-core/nm-simple-connection.c +++ b/libnm-core/nm-simple-connection.c @@ -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; } /** diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 0c88857052..f5f6b403f9 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -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); diff --git a/libnm/libnm.ver b/libnm/libnm.ver index ed00474daf..7ad57bcc3f 100644 --- a/libnm/libnm.ver +++ b/libnm/libnm.ver @@ -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; diff --git a/libnm/nm-remote-connection.c b/libnm/nm-remote-connection.c index f821180ca1..e42806edc1 100644 --- a/libnm/nm-remote-connection.c +++ b/libnm/nm-remote-connection.c @@ -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 { From 002b19f4f1110e90dac87f0c8876044b4ae57994 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 29 Jul 2014 18:25:10 -0400 Subject: [PATCH 7/8] libnm-core: add dbus-only properties to NMSettingClass Add _nm_setting_class_add_dbus_only_property(), for declaring properties that appear in the D-Bus serialization, but which don't correspond to GObject properties. Since some property overrides will require examining settings other than the setting that they are on (eg, the value of 802-11-wireless.security depends on whether an NMSettingWirelessSecurity setting is present, and NMSettingConnection:interface-name might sometimes be set from, eg, bond.interface-name), we also update _nm_setting_to_dbus() to take the full NMConnection as an argument, and _nm_setting_new_from_dbus() to take the full connection hash. Additionally, with some deprecated properties, we'll want to validate them on construction, but we don't need to keep the value around after that. So allow _nm_setting_new_from_dbus() to return a verification error directly, so we don't need to store the value until the verify() call. --- libnm-core/nm-connection.c | 2 +- libnm-core/nm-setting-private.h | 21 ++- libnm-core/nm-setting.c | 300 +++++++++++++++++++++++++----- libnm-core/nm-simple-connection.c | 4 +- libnm-core/tests/test-general.c | 10 +- 5 files changed, 285 insertions(+), 52 deletions(-) diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c index 4e72f35927..db9c699dc7 100644 --- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -1268,7 +1268,7 @@ nm_connection_to_dbus (NMConnection *connection, NMConnectionSerializationFlags while (g_hash_table_iter_next (&iter, &key, &data)) { NMSetting *setting = NM_SETTING (data); - setting_hash = _nm_setting_to_dbus (setting, flags); + setting_hash = _nm_setting_to_dbus (setting, connection, flags); if (setting_hash) g_hash_table_insert (ret, g_strdup (nm_setting_get_name (setting)), setting_hash); } diff --git a/libnm-core/nm-setting-private.h b/libnm-core/nm-setting-private.h index de9628e7b1..05a3ae3afe 100644 --- a/libnm-core/nm-setting-private.h +++ b/libnm-core/nm-setting-private.h @@ -116,9 +116,28 @@ gboolean _nm_setting_slave_type_is_valid (const char *slave_type, const char **o const char * _nm_setting_slave_type_detect_from_settings (GSList *all_settings, NMSetting **out_s_port); GHashTable *_nm_setting_to_dbus (NMSetting *setting, + NMConnection *connection, NMConnectionSerializationFlags flags); NMSetting *_nm_setting_new_from_dbus (GType setting_type, - GHashTable *hash); + GHashTable *setting_hash, + GHashTable *connection_hash, + GError **error); + +typedef gboolean (*NMSettingPropertyGetFunc) (NMSetting *setting, + NMConnection *connection, + const char *property, + GValue *value); +typedef gboolean (*NMSettingPropertySetFunc) (NMSetting *setting, + GHashTable *connection_hash, + const char *property, + const GValue *value, + GError **error); + +void _nm_setting_class_add_dbus_only_property (NMSettingClass *setting_class, + const char *property_name, + GType dbus_type, + NMSettingPropertyGetFunc get_func, + NMSettingPropertySetFunc set_func); #endif /* NM_SETTING_PRIVATE_H */ diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index 748a65557e..30dab04f51 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -372,18 +372,193 @@ _nm_setting_slave_type_detect_from_settings (GSList *all_settings, NMSetting **o /*************************************************************/ +typedef struct { + const char *name; + GParamSpec *param_spec; + GType dbus_type; + NMSettingPropertyGetFunc get_func; + NMSettingPropertySetFunc set_func; +} NMSettingProperty; + +static GQuark setting_property_overrides_quark; +static GQuark setting_properties_quark; + +static NMSettingProperty * +find_property (GArray *properties, const char *name) +{ + NMSettingProperty *property; + int i; + + if (!properties) + return NULL; + + for (i = 0; i < properties->len; i++) { + property = &g_array_index (properties, NMSettingProperty, i); + if (strcmp (name, property->name) == 0) + return property; + } + + return NULL; +} + +static void +add_property_override (NMSettingClass *setting_class, + const char *property_name, + GType dbus_type, + NMSettingPropertyGetFunc get_func, + NMSettingPropertySetFunc set_func) +{ + GType setting_type = G_TYPE_FROM_CLASS (setting_class); + GArray *overrides; + NMSettingProperty override; + + g_return_if_fail (g_type_get_qdata (setting_type, setting_properties_quark) == NULL); + + memset (&override, 0, sizeof (override)); + override.name = property_name; + override.dbus_type = dbus_type; + override.get_func = get_func; + override.set_func = set_func; + + overrides = g_type_get_qdata (setting_type, setting_property_overrides_quark); + if (!overrides) { + overrides = g_array_new (FALSE, FALSE, sizeof (NMSettingProperty)); + g_type_set_qdata (setting_type, setting_property_overrides_quark, overrides); + } + g_return_if_fail (find_property (overrides, property_name) == NULL); + + g_array_append_val (overrides, override); +} + +/** + * _nm_setting_class_add_dbus_only_property: + * @setting_class: the setting class + * @property_name: the name of the property to override + * @dbus_type: the type of the property + * @get_func: (allow-none): function to call to get the value of the property + * @set_func: (allow-none): function to call to set the value of the property + * + * Registers a property named @property_name, which will be used in the D-Bus + * serialization of objects of @setting_class, but which does not correspond to + * a #GObject property. + * + * When serializing a setting to D-Bus, @get_func will be called to get the + * property's value. If it returns %TRUE, the value will be added to the hash, + * and if %FALSE, it will not. (If @get_func is %NULL, the property will always + * be omitted in the serialization.) + * + * When deserializing a D-Bus representation into a setting, if @property_name + * is present, then @set_func will be called to set (and/or verify) it. If it + * returns %TRUE, the value is considered to have been successfully set; if it + * returns %FALSE then the deserializing operation as a whole will fail with the + * returned #GError. (If @set_func is %NULL then the property will be ignored + * when deserializing.) + */ +void +_nm_setting_class_add_dbus_only_property (NMSettingClass *setting_class, + const char *property_name, + GType dbus_type, + NMSettingPropertyGetFunc get_func, + NMSettingPropertySetFunc set_func) +{ + g_return_if_fail (NM_IS_SETTING_CLASS (setting_class)); + g_return_if_fail (property_name != NULL); + + /* Must not match any GObject property. */ + g_return_if_fail (!g_object_class_find_property (G_OBJECT_CLASS (setting_class), property_name)); + + add_property_override (setting_class, + property_name, dbus_type, + get_func, set_func); +} + +static GArray * +nm_setting_class_ensure_properties (NMSettingClass *setting_class) +{ + GType type = G_TYPE_FROM_CLASS (setting_class), otype; + NMSettingProperty property, *override; + GArray *overrides, *type_overrides, *properties; + GParamSpec **property_specs; + guint n_property_specs, i; + + properties = g_type_get_qdata (type, setting_properties_quark); + if (properties) + return properties; + + /* Build overrides array from @setting_class and its superclasses */ + overrides = g_array_new (FALSE, FALSE, sizeof (NMSettingProperty)); + for (otype = type; otype != G_TYPE_OBJECT; otype = g_type_parent (otype)) { + type_overrides = g_type_get_qdata (otype, setting_property_overrides_quark); + if (type_overrides) + g_array_append_vals (overrides, (NMSettingProperty *)type_overrides->data, type_overrides->len); + } + + /* Build the properties array from the GParamSpecs, obeying overrides */ + properties = g_array_new (FALSE, FALSE, sizeof (NMSettingProperty)); + + property_specs = g_object_class_list_properties (G_OBJECT_CLASS (setting_class), + &n_property_specs); + for (i = 0; i < n_property_specs; i++) { + override = find_property (overrides, property_specs[i]->name); + if (override) + property = *override; + else { + memset (&property, 0, sizeof (property)); + property.name = property_specs[i]->name; + property.param_spec = property_specs[i]; + } + g_array_append_val (properties, property); + } + g_free (property_specs); + + /* Add any remaining overrides not corresponding to GObject properties */ + for (i = 0; i < overrides->len; i++) { + override = &g_array_index (overrides, NMSettingProperty, i); + if (!g_object_class_find_property (G_OBJECT_CLASS (setting_class), override->name)) + g_array_append_val (properties, *override); + } + g_array_unref (overrides); + + g_type_set_qdata (type, setting_properties_quark, properties); + return properties; +} + +static const NMSettingProperty * +nm_setting_class_get_properties (NMSettingClass *setting_class, guint *n_properties) +{ + GArray *properties; + + properties = nm_setting_class_ensure_properties (setting_class); + + *n_properties = properties->len; + return (NMSettingProperty *) properties->data; +} + +static const NMSettingProperty * +nm_setting_class_find_property (NMSettingClass *setting_class, const char *property_name) +{ + GArray *properties; + + properties = nm_setting_class_ensure_properties (setting_class); + return find_property (properties, property_name); +} + +/*************************************************************/ + static void destroy_gvalue (gpointer data) { GValue *value = (GValue *) data; - g_value_unset (value); + if (G_IS_VALUE (value)) + g_value_unset (value); g_slice_free (GValue, value); } /** * _nm_setting_to_dbus: * @setting: the #NMSetting + * @connection: the #NMConnection containing @setting * @flags: hash flags, e.g. %NM_CONNECTION_SERIALIZE_ALL * * Converts the #NMSetting into a #GHashTable mapping each setting property @@ -394,47 +569,59 @@ destroy_gvalue (gpointer data) * describing the setting's properties **/ GHashTable * -_nm_setting_to_dbus (NMSetting *setting, NMConnectionSerializationFlags flags) +_nm_setting_to_dbus (NMSetting *setting, NMConnection *connection, NMConnectionSerializationFlags flags) { GHashTable *hash; - GParamSpec **property_specs; - guint n_property_specs; - guint i; + const NMSettingProperty *properties; + guint n_properties, i; g_return_val_if_fail (NM_IS_SETTING (setting), NULL); - property_specs = g_object_class_list_properties (G_OBJECT_GET_CLASS (setting), &n_property_specs); + properties = nm_setting_class_get_properties (NM_SETTING_GET_CLASS (setting), &n_properties); hash = g_hash_table_new_full (g_str_hash, g_str_equal, (GDestroyNotify) g_free, destroy_gvalue); - for (i = 0; i < n_property_specs; i++) { - GParamSpec *prop_spec = property_specs[i]; + for (i = 0; i < n_properties; i++) { + const NMSettingProperty *property = &properties[i]; + GParamSpec *prop_spec = property->param_spec; GValue *value; + gboolean set; - /* 'name' doesn't get serialized */ - if (strcmp (g_param_spec_get_name (prop_spec), NM_SETTING_NAME) == 0) + if (!prop_spec && !property->get_func) { + /* Override property with no get_func, so we skip it. */ + continue; + } + + if (prop_spec && !(prop_spec->flags & G_PARAM_WRITABLE)) continue; if ( (flags & NM_CONNECTION_SERIALIZE_NO_SECRETS) - && (prop_spec->flags & NM_SETTING_PARAM_SECRET)) + && (prop_spec && (prop_spec->flags & NM_SETTING_PARAM_SECRET))) continue; if ( (flags & NM_CONNECTION_SERIALIZE_ONLY_SECRETS) - && !(prop_spec->flags & NM_SETTING_PARAM_SECRET)) + && !(prop_spec && (prop_spec->flags & NM_SETTING_PARAM_SECRET))) continue; value = g_slice_new0 (GValue); - g_value_init (value, prop_spec->value_type); - g_object_get_property (G_OBJECT (setting), prop_spec->name, value); + if (property->get_func) { + g_value_init (value, property->dbus_type); + set = property->get_func (setting, connection, property->name, value); + } else if (prop_spec) { + g_value_init (value, prop_spec->value_type); + g_object_get_property (G_OBJECT (setting), prop_spec->name, value); - /* Don't serialize values with default values */ - if (!g_param_value_defaults (prop_spec, value)) - g_hash_table_insert (hash, g_strdup (prop_spec->name), value); + /* Don't serialize values with default values */ + set = !g_param_value_defaults (prop_spec, value); + } else + g_assert_not_reached (); + + if (set) + g_hash_table_insert (hash, g_strdup (property->name), value); else destroy_gvalue (value); } - g_free (property_specs); /* Don't return empty hashes, except for base types */ if (g_hash_table_size (hash) < 1 && !_nm_setting_is_base_type (setting)) { @@ -448,8 +635,11 @@ _nm_setting_to_dbus (NMSetting *setting, NMConnectionSerializationFlags flags) /** * _nm_setting_new_from_dbus: * @setting_type: the #NMSetting type which the hash contains properties for - * @hash: (element-type utf8 GObject.Value): the #GHashTable containing a - * string to GValue mapping of properties that apply to the setting + * @setting_hash: (element-type utf8 GObject.Value): the #GHashTable containing a + * string to #GValue mapping of properties that apply to the setting + * @connection_hash: (element-type utf8 GObject.Value): the #GHashTable containing a + * string to #GHashTable mapping of properties for the whole connection + * @error: location to store error, or %NULL * * Creates a new #NMSetting object and populates that object with the properties * contained in the hash table, using each hash key as the property to set, @@ -462,18 +652,21 @@ _nm_setting_to_dbus (NMSetting *setting, NMConnectionSerializationFlags flags) * hash table, or %NULL on failure **/ NMSetting * -_nm_setting_new_from_dbus (GType setting_type, GHashTable *hash) +_nm_setting_new_from_dbus (GType setting_type, + GHashTable *setting_hash, + GHashTable *connection_hash, + GError **error) { NMSetting *setting; - GObjectClass *class; + NMSettingClass *class; GHashTableIter iter; const char *prop_name; - GParamSpec **property_specs; - guint n_property_specs; + const NMSettingProperty *properties; + guint n_properties; guint i; g_return_val_if_fail (G_TYPE_IS_INSTANTIATABLE (setting_type), NULL); - g_return_val_if_fail (hash != NULL, NULL); + g_return_val_if_fail (setting_hash != NULL, NULL); /* g_type_class_ref() ensures the setting class is created if it hasn't * already been used. @@ -481,9 +674,9 @@ _nm_setting_new_from_dbus (GType setting_type, GHashTable *hash) class = g_type_class_ref (setting_type); /* Check for invalid properties first. */ - g_hash_table_iter_init (&iter, hash); + g_hash_table_iter_init (&iter, setting_hash); while (g_hash_table_iter_next (&iter, (gpointer) &prop_name, NULL)) { - if (!g_object_class_find_property (class, prop_name)) { + if (!nm_setting_class_find_property (class, prop_name)) { /* Oh, we're so nice and only warn, maybe it should be a fatal error? */ g_warning ("Ignoring invalid property '%s'", prop_name); continue; @@ -493,22 +686,32 @@ _nm_setting_new_from_dbus (GType setting_type, GHashTable *hash) /* Now build the setting object from the legitimate properties */ setting = (NMSetting *) g_object_new (setting_type, NULL); - property_specs = g_object_class_list_properties (G_OBJECT_GET_CLASS (setting), &n_property_specs); - for (i = 0; i < n_property_specs; i++) { - GParamSpec *prop_spec = property_specs[i]; - GValue *value; + properties = nm_setting_class_get_properties (class, &n_properties); + for (i = 0; i < n_properties; i++) { + const NMSettingProperty *property = &properties[i]; + GValue *value = g_hash_table_lookup (setting_hash, property->name); - /* 'name' doesn't get deserialized */ - if (strcmp (prop_spec->name, NM_SETTING_NAME) == 0) - continue; - - value = g_hash_table_lookup (hash, prop_spec->name); if (!value) continue; - g_object_set_property (G_OBJECT (setting), prop_spec->name, value); + if (property->set_func) { + if (!property->set_func (setting, + connection_hash, + property->name, + value, + error)) { + g_object_unref (setting); + setting = NULL; + break; + } + } else if (property->param_spec) { + if (!(property->param_spec->flags & G_PARAM_WRITABLE)) + continue; + + g_object_set_property (G_OBJECT (setting), property->param_spec->name, value); + } } - g_free (property_specs); + g_type_class_unref (class); return setting; @@ -1065,10 +1268,11 @@ nm_setting_need_secrets (NMSetting *setting) static int update_one_secret (NMSetting *setting, const char *key, GValue *value, GError **error) { + const NMSettingProperty *property; GParamSpec *prop_spec; - prop_spec = g_object_class_find_property (G_OBJECT_GET_CLASS (setting), key); - if (!prop_spec) { + property = nm_setting_class_find_property (NM_SETTING_GET_CLASS (setting), key); + if (!property) { g_set_error (error, NM_SETTING_ERROR, NM_SETTING_ERROR_PROPERTY_NOT_FOUND, @@ -1077,7 +1281,8 @@ update_one_secret (NMSetting *setting, const char *key, GValue *value, GError ** } /* Silently ignore non-secrets */ - if (!(prop_spec->flags & NM_SETTING_PARAM_SECRET)) + prop_spec = property->param_spec; + if (!prop_spec || !(prop_spec->flags & NM_SETTING_PARAM_SECRET)) return NM_SETTING_UPDATE_SECRET_SUCCESS_UNCHANGED; if (g_value_type_compatible (G_VALUE_TYPE (value), G_PARAM_SPEC_VALUE_TYPE (prop_spec))) { @@ -1154,10 +1359,11 @@ _nm_setting_update_secrets (NMSetting *setting, GHashTable *secrets, GError **er static gboolean is_secret_prop (NMSetting *setting, const char *secret_name, GError **error) { + const NMSettingProperty *property; GParamSpec *pspec; - pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (setting), secret_name); - if (!pspec) { + property = nm_setting_class_find_property (NM_SETTING_GET_CLASS (setting), secret_name); + if (!property) { g_set_error (error, NM_SETTING_ERROR, NM_SETTING_ERROR_PROPERTY_NOT_FOUND, @@ -1165,7 +1371,8 @@ is_secret_prop (NMSetting *setting, const char *secret_name, GError **error) return FALSE; } - if (!(pspec->flags & NM_SETTING_PARAM_SECRET)) { + pspec = property->param_spec; + if (!pspec || !(pspec->flags & NM_SETTING_PARAM_SECRET)) { g_set_error (error, NM_SETTING_ERROR, NM_SETTING_ERROR_PROPERTY_NOT_SECRET, @@ -1507,6 +1714,11 @@ nm_setting_class_init (NMSettingClass *setting_class) { GObjectClass *object_class = G_OBJECT_CLASS (setting_class); + if (!setting_property_overrides_quark) + setting_property_overrides_quark = g_quark_from_static_string ("nm-setting-property-overrides"); + if (!setting_properties_quark) + setting_properties_quark = g_quark_from_static_string ("nm-setting-properties"); + g_type_class_add_private (setting_class, sizeof (NMSettingPrivate)); /* virtual methods */ diff --git a/libnm-core/nm-simple-connection.c b/libnm-core/nm-simple-connection.c index febacdc54c..0512cfbbef 100644 --- a/libnm-core/nm-simple-connection.c +++ b/libnm-core/nm-simple-connection.c @@ -88,7 +88,9 @@ nm_simple_connection_new_from_dbus (GHashTable *hash, GError **error) goto failed; } - setting = _nm_setting_new_from_dbus (type, setting_hash); + setting = _nm_setting_new_from_dbus (type, setting_hash, hash, error); + if (!setting) + goto failed; nm_connection_add_setting (connection, setting); } diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index f5f6b403f9..797edd7526 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -680,7 +680,7 @@ test_setting_to_dbus_all (void) s_wsec = make_test_wsec_setting ("setting-to-dbus-all"); - hash = _nm_setting_to_dbus (NM_SETTING (s_wsec), NM_CONNECTION_SERIALIZE_ALL); + hash = _nm_setting_to_dbus (NM_SETTING (s_wsec), NULL, NM_CONNECTION_SERIALIZE_ALL); /* Make sure all keys are there */ ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT), @@ -704,7 +704,7 @@ test_setting_to_dbus_no_secrets (void) s_wsec = make_test_wsec_setting ("setting-to-dbus-no-secrets"); - hash = _nm_setting_to_dbus (NM_SETTING (s_wsec), NM_CONNECTION_SERIALIZE_NO_SECRETS); + hash = _nm_setting_to_dbus (NM_SETTING (s_wsec), NULL, NM_CONNECTION_SERIALIZE_NO_SECRETS); /* Make sure non-secret keys are there */ ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT), @@ -730,7 +730,7 @@ test_setting_to_dbus_only_secrets (void) s_wsec = make_test_wsec_setting ("setting-to-dbus-only-secrets"); - hash = _nm_setting_to_dbus (NM_SETTING (s_wsec), NM_CONNECTION_SERIALIZE_ONLY_SECRETS); + hash = _nm_setting_to_dbus (NM_SETTING (s_wsec), NULL, NM_CONNECTION_SERIALIZE_ONLY_SECRETS); /* Make sure non-secret keys are there */ ASSERT (g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT) == NULL, @@ -778,10 +778,10 @@ test_setting_new_from_dbus (void) GHashTable *hash; s_wsec = make_test_wsec_setting ("setting-to-dbus-all"); - hash = _nm_setting_to_dbus (NM_SETTING (s_wsec), NM_CONNECTION_SERIALIZE_ALL); + hash = _nm_setting_to_dbus (NM_SETTING (s_wsec), NULL, NM_CONNECTION_SERIALIZE_ALL); g_object_unref (s_wsec); - s_wsec = (NMSettingWirelessSecurity *) _nm_setting_new_from_dbus (NM_TYPE_SETTING_WIRELESS_SECURITY, hash); + s_wsec = (NMSettingWirelessSecurity *) _nm_setting_new_from_dbus (NM_TYPE_SETTING_WIRELESS_SECURITY, hash, NULL, NULL); g_hash_table_destroy (hash); g_assert (s_wsec); From a5ac95ca4bab34cfafd94e3f785bbe51e9b0b223 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 29 Jul 2014 18:42:02 -0400 Subject: [PATCH 8/8] libnm-core: drop previously-deprecated NMSetting properties Drop the NMSetting properties that were marked deprecated in libnm-util in 0.9.10, but use nm_setting_class_add_dbus_property() to deal with them appropriately when serializing/deserializing. --- libnm-core/nm-setting-gsm.c | 60 ++++----------------------- libnm-core/nm-setting-gsm.h | 4 -- libnm-core/nm-setting-wireless.c | 43 ++++++++----------- libnm-core/nm-setting-wireless.h | 3 -- libnm-core/tests/test-general.c | 55 ++++++++++++++++++++++++ src/settings/nm-settings-connection.c | 12 ------ 6 files changed, 80 insertions(+), 97 deletions(-) diff --git a/libnm-core/nm-setting-gsm.c b/libnm-core/nm-setting-gsm.c index 57fca83d8a..726e82ca8b 100644 --- a/libnm-core/nm-setting-gsm.c +++ b/libnm-core/nm-setting-gsm.c @@ -71,8 +71,6 @@ typedef struct { char *apn; /* NULL for dynamic */ char *network_id; /* for manual registration or NULL for automatic */ - int network_type; /* One of the NM_SETTING_GSM_NETWORK_TYPE_* */ - guint32 allowed_bands; /* A bitfield of NM_SETTING_GSM_BAND_* */ char *pin; NMSettingSecretFlags pin_flags; @@ -88,10 +86,8 @@ enum { PROP_PASSWORD_FLAGS, PROP_APN, PROP_NETWORK_ID, - PROP_NETWORK_TYPE, PROP_PIN, PROP_PIN_FLAGS, - PROP_ALLOWED_BANDS, PROP_HOME_ONLY, LAST_PROP @@ -423,12 +419,6 @@ set_property (GObject *object, guint prop_id, if (tmp) priv->network_id = g_strstrip (tmp); break; - case PROP_NETWORK_TYPE: - priv->network_type = g_value_get_int (value); - break; - case PROP_ALLOWED_BANDS: - priv->allowed_bands = g_value_get_uint (value); - break; case PROP_PIN: g_free (priv->pin); priv->pin = g_value_dup_string (value); @@ -470,12 +460,6 @@ get_property (GObject *object, guint prop_id, case PROP_NETWORK_ID: g_value_set_string (value, nm_setting_gsm_get_network_id (setting)); break; - case PROP_NETWORK_TYPE: - g_value_set_int (value, NM_SETTING_GSM_GET_PRIVATE (setting)->network_type); - break; - case PROP_ALLOWED_BANDS: - g_value_set_uint (value, NM_SETTING_GSM_GET_PRIVATE (setting)->allowed_bands); - break; case PROP_PIN: g_value_set_string (value, nm_setting_gsm_get_pin (setting)); break; @@ -600,42 +584,6 @@ nm_setting_gsm_class_init (NMSettingGsmClass *setting_class) G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)); - /** - * NMSettingGsm:network-type: - * - * (Unused) - * - * Deprecated: 0.9.10: No longer used. Network type setting should be done - * by talking to ModemManager directly. - **/ - g_object_class_install_property - (object_class, PROP_NETWORK_TYPE, - g_param_spec_int (NM_SETTING_GSM_NETWORK_TYPE, "", "", - -1 /* NM_SETTING_GSM_NETWORK_TYPE_ANY */, - 5 /* NM_SETTING_GSM_NETWORK_TYPE_4G */, - -1 /* NM_SETTING_GSM_NETWORK_TYPE_ANY */, - G_PARAM_READWRITE | - G_PARAM_CONSTRUCT | - G_PARAM_STATIC_STRINGS)); - - /** - * NMSettingGsm:allowed-bands: - * - * (Unused) - * - * Deprecated: 0.9.10: No longer used. Band setting should be done by - * talking to ModemManager directly. - **/ - g_object_class_install_property - (object_class, PROP_ALLOWED_BANDS, - g_param_spec_uint (NM_SETTING_GSM_ALLOWED_BANDS, "", "", - 0 /* NM_SETTING_GSM_BAND_UNKNOWN */, - 0x3fff /* NM_SETTING_GSM_BANDS_MAX */, - 1 /* NM_SETTING_GSM_BAND_ANY */, - G_PARAM_READWRITE | - G_PARAM_CONSTRUCT | - G_PARAM_STATIC_STRINGS)); - /** * NMSettingGsm:pin: * @@ -677,4 +625,12 @@ nm_setting_gsm_class_init (NMSettingGsmClass *setting_class) FALSE, G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)); + + /* Ignore incoming deprecated properties */ + _nm_setting_class_add_dbus_only_property (parent_class, "allowed-bands", + G_TYPE_UINT, + NULL, NULL); + _nm_setting_class_add_dbus_only_property (parent_class, "network-type", + G_TYPE_INT, + NULL, NULL); } diff --git a/libnm-core/nm-setting-gsm.h b/libnm-core/nm-setting-gsm.h index 05c529aa54..c44a03486d 100644 --- a/libnm-core/nm-setting-gsm.h +++ b/libnm-core/nm-setting-gsm.h @@ -69,10 +69,6 @@ GQuark nm_setting_gsm_error_quark (void); #define NM_SETTING_GSM_PIN_FLAGS "pin-flags" #define NM_SETTING_GSM_HOME_ONLY "home-only" -/* Deprecated */ -#define NM_SETTING_GSM_ALLOWED_BANDS "allowed-bands" -#define NM_SETTING_GSM_NETWORK_TYPE "network-type" - typedef struct { NMSetting parent; } NMSettingGsm; diff --git a/libnm-core/nm-setting-wireless.c b/libnm-core/nm-setting-wireless.c index a100208739..c117a55b68 100644 --- a/libnm-core/nm-setting-wireless.c +++ b/libnm-core/nm-setting-wireless.c @@ -81,7 +81,6 @@ typedef struct { GSList *mac_address_blacklist; guint32 mtu; GSList *seen_bssids; - char *security; gboolean hidden; } NMSettingWirelessPrivate; @@ -99,7 +98,6 @@ enum { PROP_MAC_ADDRESS_BLACKLIST, PROP_MTU, PROP_SEEN_BSSIDS, - PROP_SEC, PROP_HIDDEN, LAST_PROP @@ -824,6 +822,19 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) return TRUE; } +static gboolean +nm_setting_wireless_get_security (NMSetting *setting, + NMConnection *connection, + const char *property_name, + GValue *value) +{ + if (nm_connection_get_setting_wireless_security (connection)) { + g_value_set_string (value, NM_SETTING_WIRELESS_SECURITY_SETTING_NAME); + return TRUE; + } else + return FALSE; +} + static void nm_setting_wireless_init (NMSettingWireless *setting) { @@ -836,7 +847,6 @@ finalize (GObject *object) g_free (priv->mode); g_free (priv->band); - g_free (priv->security); if (priv->ssid) g_byte_array_free (priv->ssid, TRUE); @@ -907,10 +917,6 @@ set_property (GObject *object, guint prop_id, g_slist_free_full (priv->seen_bssids, g_free); priv->seen_bssids = g_value_dup_boxed (value); break; - case PROP_SEC: - g_free (priv->security); - priv->security = g_value_dup_string (value); - break; case PROP_HIDDEN: priv->hidden = g_value_get_boolean (value); break; @@ -963,9 +969,6 @@ get_property (GObject *object, guint prop_id, case PROP_SEEN_BSSIDS: g_value_set_boxed (value, NM_SETTING_WIRELESS_GET_PRIVATE (setting)->seen_bssids); break; - case PROP_SEC: - g_value_set_string (value, NM_SETTING_WIRELESS_GET_PRIVATE (setting)->security); - break; case PROP_HIDDEN: g_value_set_boolean (value, nm_setting_wireless_get_hidden (setting)); break; @@ -1171,22 +1174,6 @@ nm_setting_wireless_class_init (NMSettingWirelessClass *setting_class) NM_SETTING_PARAM_FUZZY_IGNORE | G_PARAM_STATIC_STRINGS)); - /** - * NMSettingWireless:security: - * - * (Unused) - * - * Deprecated: 0.9.10: No longer used. Security restrictions are recognized - * by the presence of a #NMSettingWirelessSecurity setting in the - * connection. - **/ - g_object_class_install_property - (object_class, PROP_SEC, - g_param_spec_string (NM_SETTING_WIRELESS_SEC, "", "", - NULL, - G_PARAM_READWRITE | - G_PARAM_STATIC_STRINGS)); - /** * NMSettingWireless:hidden: * @@ -1202,4 +1189,8 @@ nm_setting_wireless_class_init (NMSettingWirelessClass *setting_class) FALSE, G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)); + + /* Compatibility for deprecated property */ + _nm_setting_class_add_dbus_only_property (parent_class, "security", G_TYPE_STRING, + nm_setting_wireless_get_security, NULL); } diff --git a/libnm-core/nm-setting-wireless.h b/libnm-core/nm-setting-wireless.h index e8c5984e67..54dc6af3a8 100644 --- a/libnm-core/nm-setting-wireless.h +++ b/libnm-core/nm-setting-wireless.h @@ -78,9 +78,6 @@ GQuark nm_setting_wireless_error_quark (void); #define NM_SETTING_WIRELESS_SEEN_BSSIDS "seen-bssids" #define NM_SETTING_WIRELESS_HIDDEN "hidden" -/* Deprecated */ -#define NM_SETTING_WIRELESS_SEC "security" - /** * NM_SETTING_WIRELESS_MODE_ADHOC: * diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 797edd7526..d789729ec6 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -771,6 +771,60 @@ test_connection_to_dbus_setting_name (void) g_object_unref (connection); } +static void +test_connection_to_dbus_deprecated_props (void) +{ + NMConnection *connection; + NMSetting *s_wireless; + GByteArray *ssid; + NMSettingWirelessSecurity *s_wsec; + GHashTable *hash, *wireless_hash; + GValue *sec_val; + + connection = nmtst_create_minimal_connection ("test-connection-to-dbus-deprecated-props", + NULL, + NM_SETTING_WIRELESS_SETTING_NAME, + NULL); + + s_wireless = nm_setting_wireless_new (); + ssid = g_byte_array_new (); + g_byte_array_append (ssid, (const guint8 *) "1234567", 7); + g_object_set (s_wireless, + NM_SETTING_WIRELESS_SSID, ssid, + NULL); + g_byte_array_unref (ssid); + nm_connection_add_setting (connection, s_wireless); + + /* Hash should not have an 802-11-wireless.security property */ + hash = nm_connection_to_dbus (connection, NM_CONNECTION_SERIALIZE_ALL); + g_assert (hash != NULL); + + wireless_hash = g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SETTING_NAME); + g_assert (wireless_hash != NULL); + + sec_val = g_hash_table_lookup (wireless_hash, "security"); + g_assert (sec_val == NULL); + + g_hash_table_destroy (hash); + + /* Now add an NMSettingWirelessSecurity and try again */ + s_wsec = make_test_wsec_setting ("test-connection-to-dbus-deprecated-props"); + nm_connection_add_setting (connection, NM_SETTING (s_wsec)); + + hash = nm_connection_to_dbus (connection, NM_CONNECTION_SERIALIZE_ALL); + g_assert (hash != NULL); + + wireless_hash = g_hash_table_lookup (hash, NM_SETTING_WIRELESS_SETTING_NAME); + g_assert (wireless_hash != NULL); + + sec_val = g_hash_table_lookup (wireless_hash, "security"); + g_assert (G_VALUE_HOLDS_STRING (sec_val)); + g_assert_cmpstr (g_value_get_string (sec_val), ==, NM_SETTING_WIRELESS_SECURITY_SETTING_NAME); + + g_hash_table_destroy (hash); + g_object_unref (connection); +} + static void test_setting_new_from_dbus (void) { @@ -3167,6 +3221,7 @@ int main (int argc, char **argv) g_test_add_func ("/core/general/test_setting_old_uuid", test_setting_old_uuid); g_test_add_func ("/core/general/test_connection_to_dbus_setting_name", test_connection_to_dbus_setting_name); + g_test_add_func ("/core/general/test_connection_to_dbus_deprecated_props", test_connection_to_dbus_deprecated_props); 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); diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 33b74f9bc8..0b821ecb9b 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -1165,18 +1165,6 @@ get_settings_auth_cb (NMSettingsConnection *self, g_object_set (s_wifi, NM_SETTING_WIRELESS_SEEN_BSSIDS, bssid_list, NULL); g_slist_free (bssid_list); - /* 802-11-wireless.security property is deprecated. But we set it here so that - * we don't disturb old clients that might expect it being properly set for - * secured Wi-Fi connections. - */ - if (nm_connection_get_setting_wireless_security (NM_CONNECTION (dupl_con))) { - s_wifi = nm_connection_get_setting_wireless (NM_CONNECTION (dupl_con)); - g_assert (s_wifi); - g_object_set (s_wifi, - NM_SETTING_WIRELESS_SEC, NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, - NULL); - } - /* Secrets should *never* be returned by the GetSettings method, they * get returned by the GetSecrets method which can be better * protected against leakage of secrets to unprivileged callers.