From 866ac505a85e8f3b3e71cde58afb4c9d8674ec0f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 31 Jan 2019 09:04:01 +0100 Subject: [PATCH] libnm,core: various cleanups of _nm_connection_for_each_secret() - use cleanup attribute to free memory - return floating reference from _nm_connection_for_each_secret(). It's more idiomatic that a function that constructs a variant and returns it, returns a floating variant. --- libnm-core/nm-connection.c | 32 +++++++++++++-------------- src/settings/nm-settings-connection.c | 26 +++++++++++++++------- 2 files changed, 33 insertions(+), 25 deletions(-) diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c index 1e38029dd8..a555bd6d58 100644 --- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -1875,7 +1875,7 @@ nm_connection_clear_secrets_with_flags (NMConnection *connection, /*****************************************************************************/ -/* Returns always a non-NULL, non-floating variant that must +/* Returns always a non-NULL, floating variant that must * be unrefed by the caller. */ GVariant * _nm_connection_for_each_secret (NMConnection *self, @@ -1884,8 +1884,10 @@ _nm_connection_for_each_secret (NMConnection *self, NMConnectionForEachSecretFunc callback, gpointer callback_data) { - GVariantBuilder secrets_builder, setting_builder; - GVariantIter secrets_iter, *setting_iter; + GVariantBuilder secrets_builder; + GVariantBuilder setting_builder; + GVariantIter secrets_iter; + GVariantIter *setting_iter; const char *setting_name; /* This function, given a dict of dicts representing new secrets of @@ -1909,18 +1911,18 @@ _nm_connection_for_each_secret (NMConnection *self, g_variant_iter_init (&secrets_iter, secrets); g_variant_builder_init (&secrets_builder, NM_VARIANT_TYPE_CONNECTION); while (g_variant_iter_next (&secrets_iter, "{&sa{sv}}", &setting_name, &setting_iter)) { + _nm_unused nm_auto_free_variant_iter GVariantIter *setting_iter_free = setting_iter; NMSetting *setting; const char *secret_name; GVariant *val; setting = nm_connection_get_setting_by_name (self, setting_name); - if (setting == NULL) { - g_variant_iter_free (setting_iter); + if (!setting) continue; - } g_variant_builder_init (&setting_builder, NM_VARIANT_TYPE_SETTING); while (g_variant_iter_next (setting_iter, "{&sv}", &secret_name, &val)) { + _nm_unused gs_unref_variant GVariant *val_free = val; NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE; /* VPN secrets need slightly different treatment here since the @@ -1959,20 +1961,17 @@ _nm_connection_for_each_secret (NMConnection *self, if (!nm_setting_get_secret_flags (setting, secret_name, &secret_flags, NULL)) { if (!remove_non_secrets) g_variant_builder_add (&setting_builder, "{sv}", secret_name, val); - g_variant_unref (val); continue; } if (callback (secret_flags, callback_data)) g_variant_builder_add (&setting_builder, "{sv}", secret_name, val); } - g_variant_unref (val); } - g_variant_iter_free (setting_iter); g_variant_builder_add (&secrets_builder, "{sa{sv}}", setting_name, &setting_builder); } - return g_variant_ref_sink (g_variant_builder_end (&secrets_builder)); + return g_variant_builder_end (&secrets_builder); } /*****************************************************************************/ @@ -2000,15 +1999,14 @@ _nm_connection_find_secret (NMConnection *self, NMConnectionFindSecretFunc callback, gpointer callback_data) { - FindSecretData data; - GVariant *dummy; - - data.find_func = callback; - data.find_func_data = callback_data; - data.found = FALSE; + gs_unref_variant GVariant *dummy = NULL; + FindSecretData data = { + .find_func = callback, + .find_func_data = callback_data, + .found = FALSE, + }; dummy = _nm_connection_for_each_secret (self, secrets, FALSE, find_secret_for_each_func, &data); - g_variant_unref (dummy); return data.found; } diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index aa4ec0a90a..25eb828bbb 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -803,8 +803,8 @@ typedef struct { } ForEachSecretFlags; static gboolean -validate_secret_flags (NMSettingSecretFlags flags, - gpointer user_data) +validate_secret_flags_cb (NMSettingSecretFlags flags, + gpointer user_data) { ForEachSecretFlags *cmp_flags = user_data; @@ -815,6 +815,18 @@ validate_secret_flags (NMSettingSecretFlags flags, return TRUE; } +static GVariant * +validate_secret_flags (NMConnection *connection, + GVariant *secrets, + ForEachSecretFlags *cmp_flags) +{ + return g_variant_ref_sink (_nm_connection_for_each_secret (connection, + secrets, + TRUE, + validate_secret_flags_cb, + cmp_flags)); +} + static gboolean secret_is_system_owned (NMSettingSecretFlags flags, gpointer user_data) @@ -1016,14 +1028,14 @@ get_secrets_done_cb (NMAgentManager *manager, /* Update the connection with our existing secrets from backing storage */ nm_connection_clear_secrets (nm_settings_connection_get_connection (self)); if (!dict || nm_connection_update_secrets (nm_settings_connection_get_connection (self), setting_name, dict, &local)) { - GVariant *filtered_secrets; + gs_unref_variant GVariant *filtered_secrets = NULL; /* Update the connection with the agent's secrets; by this point if any * system-owned secrets exist in 'secrets' the agent that provided them * will have been authenticated, so those secrets can replace the existing * system secrets. */ - filtered_secrets = _nm_connection_for_each_secret (nm_settings_connection_get_connection (self), secrets, TRUE, validate_secret_flags, &cmp_flags); + filtered_secrets = validate_secret_flags (nm_settings_connection_get_connection (self), secrets, &cmp_flags); if (nm_connection_update_secrets (nm_settings_connection_get_connection (self), setting_name, filtered_secrets, &local)) { /* Now that all secrets are updated, copy and cache new secrets, * then save them to backing storage. @@ -1059,7 +1071,6 @@ get_secrets_done_cb (NMAgentManager *manager, call_id, local->message); } - g_variant_unref (filtered_secrets); } else { _LOGD ("(%s:%p) failed to update with existing secrets: %s", setting_name, @@ -1083,11 +1094,10 @@ get_secrets_done_cb (NMAgentManager *manager, nm_connection_clear_secrets (applied_connection); if (!dict || nm_connection_update_secrets (applied_connection, setting_name, dict, NULL)) { - GVariant *filtered_secrets; + gs_unref_variant GVariant *filtered_secrets = NULL; - filtered_secrets = _nm_connection_for_each_secret (applied_connection, secrets, TRUE, validate_secret_flags, &cmp_flags); + filtered_secrets = validate_secret_flags (applied_connection, secrets, &cmp_flags); nm_connection_update_secrets (applied_connection, setting_name, filtered_secrets, NULL); - g_variant_unref (filtered_secrets); } }