From d80be7825d47ac24a3e14a37a1eb7d9940b222b2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 11:54:47 +0100 Subject: [PATCH 01/30] shared: add nm_clear_g_cancellable_disconnect() --- shared/nm-utils/nm-macros-internal.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/shared/nm-utils/nm-macros-internal.h b/shared/nm-utils/nm-macros-internal.h index 5a4975b539..3537b37d50 100644 --- a/shared/nm-utils/nm-macros-internal.h +++ b/shared/nm-utils/nm-macros-internal.h @@ -1149,6 +1149,28 @@ nm_clear_g_cancellable (GCancellable **cancellable) return FALSE; } +/* If @cancellable_id is not 0, clear it and call g_cancellable_disconnect(). + * @cancellable may be %NULL, if there is nothing to disconnect. + * + * It's like nm_clear_g_signal_handler(), except that it uses g_cancellable_disconnect() + * instead of g_signal_handler_disconnect(). + * + * Note the warning in glib documentation about dead-lock and what g_cancellable_disconnect() + * actually does. */ +static inline gboolean +nm_clear_g_cancellable_disconnect (GCancellable *cancellable, gulong *cancellable_id) +{ + gulong id; + + if ( cancellable_id + && (id = *cancellable_id) != 0) { + *cancellable_id = 0; + g_cancellable_disconnect (cancellable, id); + return TRUE; + } + return FALSE; +} + /*****************************************************************************/ static inline GVariant * From 7fb18004b36e4715afe0b6dedb2f914527583ae4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 13:30:28 +0100 Subject: [PATCH 02/30] libnm/trival: fix indention in "libnm/nm-secret-agent-old.c" --- libnm/nm-secret-agent-old.c | 68 ++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/libnm/nm-secret-agent-old.c b/libnm/nm-secret-agent-old.c index 1b9e8915bb..a688282f6f 100644 --- a/libnm/nm-secret-agent-old.c +++ b/libnm/nm-secret-agent-old.c @@ -138,8 +138,8 @@ name_owner_changed (GObject *proxy, GetSecretsInfo *info = iter->data; NM_SECRET_AGENT_OLD_GET_CLASS (self)->cancel_get_secrets (self, - info->path, - info->setting_name); + info->path, + info->setting_name); } g_slist_free (priv->pending_gets); priv->pending_gets = NULL; @@ -338,13 +338,13 @@ impl_secret_agent_old_get_secrets (NMSecretAgentOld *self, priv->pending_gets = g_slist_append (priv->pending_gets, info); NM_SECRET_AGENT_OLD_GET_CLASS (self)->get_secrets (self, - connection, - connection_path, - setting_name, - (const char **) hints, - flags, - get_secrets_cb, - info); + connection, + connection_path, + setting_name, + (const char **) hints, + flags, + get_secrets_cb, + info); g_object_unref (connection); } @@ -391,8 +391,8 @@ impl_secret_agent_old_cancel_get_secrets (NMSecretAgentOld *self, /* Send the cancel request up to the subclass and finalize it */ NM_SECRET_AGENT_OLD_GET_CLASS (self)->cancel_get_secrets (self, - info->path, - info->setting_name); + info->path, + info->setting_name); g_dbus_method_invocation_return_value (context, NULL); } @@ -427,10 +427,10 @@ impl_secret_agent_old_save_secrets (NMSecretAgentOld *self, } NM_SECRET_AGENT_OLD_GET_CLASS (self)->save_secrets (self, - connection, - connection_path, - save_secrets_cb, - context); + connection, + connection_path, + save_secrets_cb, + context); g_object_unref (connection); } @@ -465,10 +465,10 @@ impl_secret_agent_old_delete_secrets (NMSecretAgentOld *self, } NM_SECRET_AGENT_OLD_GET_CLASS (self)->delete_secrets (self, - connection, - connection_path, - delete_secrets_cb, - context); + connection, + connection_path, + delete_secrets_cb, + context); g_object_unref (connection); } @@ -916,13 +916,13 @@ nm_secret_agent_old_get_secrets (NMSecretAgentOld *self, g_return_if_fail (callback != NULL); NM_SECRET_AGENT_OLD_GET_CLASS (self)->get_secrets (self, - connection, - nm_connection_get_path (connection), - setting_name, - hints, - flags, - callback, - user_data); + connection, + nm_connection_get_path (connection), + setting_name, + hints, + flags, + callback, + user_data); } /** @@ -946,10 +946,10 @@ nm_secret_agent_old_save_secrets (NMSecretAgentOld *self, g_return_if_fail (nm_connection_get_path (connection)); NM_SECRET_AGENT_OLD_GET_CLASS (self)->save_secrets (self, - connection, - nm_connection_get_path (connection), - callback, - user_data); + connection, + nm_connection_get_path (connection), + callback, + user_data); } /** @@ -973,10 +973,10 @@ nm_secret_agent_old_delete_secrets (NMSecretAgentOld *self, g_return_if_fail (nm_connection_get_path (connection)); NM_SECRET_AGENT_OLD_GET_CLASS (self)->delete_secrets (self, - connection, - nm_connection_get_path (connection), - callback, - user_data); + connection, + nm_connection_get_path (connection), + callback, + user_data); } /*****************************************************************************/ From 2626eb6d8c5341ce8d14b254a305a8323cdf42bf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 12:52:49 +0100 Subject: [PATCH 03/30] cli: clear fields in nmc_cleanup() Don't leave dangling pointers. --- clients/cli/nmcli.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/clients/cli/nmcli.c b/clients/cli/nmcli.c index 6217477cc2..a554a382a4 100644 --- a/clients/cli/nmcli.c +++ b/clients/cli/nmcli.c @@ -1013,15 +1013,15 @@ nmc_cleanup (NmCli *nmc) g_clear_object (&nmc->client); - g_string_free (nmc->return_text, TRUE); + if (nmc->return_text) + g_string_free (g_steal_pointer (&nmc->return_text), TRUE); if (nmc->secret_agent) { - /* Destroy secret agent if we have one. */ nm_secret_agent_old_unregister (nmc->secret_agent, NULL, NULL); - g_object_unref (nmc->secret_agent); + g_clear_object (&nmc->secret_agent); } - if (nmc->pwds_hash) - g_hash_table_destroy (nmc->pwds_hash); + + nm_clear_pointer (&nmc->pwds_hash, g_hash_table_destroy); nm_clear_g_free (&nmc->required_fields); From a6600f5ae68bedcdc48826226f77a5326f2a2e66 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 10:14:21 +0100 Subject: [PATCH 04/30] clients/secret-agent: reorder code in nm-secret-agent-simple.c --- clients/common/nm-secret-agent-simple.c | 150 +++++++++++++----------- clients/common/nm-secret-agent-simple.h | 39 +++--- 2 files changed, 98 insertions(+), 91 deletions(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index 9fcf4200cd..f89d225902 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -31,26 +31,16 @@ #include "nm-default.h" +#include "nm-secret-agent-simple.h" + #include #include #include #include "nm-vpn-service-plugin.h" - #include "nm-vpn-helpers.h" -#include "nm-secret-agent-simple.h" -G_DEFINE_TYPE (NMSecretAgentSimple, nm_secret_agent_simple, NM_TYPE_SECRET_AGENT_OLD) - -#define NM_SECRET_AGENT_SIMPLE_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_SECRET_AGENT_SIMPLE, NMSecretAgentSimplePrivate)) - -enum { - REQUEST_SECRETS, - - LAST_SIGNAL -}; - -static guint signals[LAST_SIGNAL] = { 0 }; +/*****************************************************************************/ typedef struct { NMSecretAgentSimple *self; @@ -64,6 +54,14 @@ typedef struct { NMSecretAgentGetSecretsFlags flags; } NMSecretAgentSimpleRequest; +enum { + REQUEST_SECRETS, + + LAST_SIGNAL +}; + +static guint signals[LAST_SIGNAL] = { 0 }; + typedef struct { /* */ GHashTable *requests; @@ -72,6 +70,12 @@ typedef struct { gboolean enabled; } NMSecretAgentSimplePrivate; +G_DEFINE_TYPE (NMSecretAgentSimple, nm_secret_agent_simple, NM_TYPE_SECRET_AGENT_OLD) + +#define NM_SECRET_AGENT_SIMPLE_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_SECRET_AGENT_SIMPLE, NMSecretAgentSimplePrivate)) + +/*****************************************************************************/ + static void nm_secret_agent_simple_request_free (gpointer data) { @@ -96,45 +100,7 @@ nm_secret_agent_simple_request_cancel (NMSecretAgentSimpleRequest *request, g_hash_table_remove (priv->requests, request->request_id); } -static void -nm_secret_agent_simple_init (NMSecretAgentSimple *agent) -{ - NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (agent); - - priv->requests = g_hash_table_new_full (nm_str_hash, g_str_equal, - g_free, nm_secret_agent_simple_request_free); -} - -static void -nm_secret_agent_simple_finalize (GObject *object) -{ - NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (object); - GError *error; - GHashTableIter iter; - gpointer key; - gpointer value; - - error = g_error_new (NM_SECRET_AGENT_ERROR, - NM_SECRET_AGENT_ERROR_AGENT_CANCELED, - "The secret agent is going away"); - - g_hash_table_iter_init (&iter, priv->requests); - while (g_hash_table_iter_next (&iter, &key, &value)) { - NMSecretAgentSimpleRequest *request = value; - - request->callback (NM_SECRET_AGENT_OLD (object), - request->connection, - NULL, error, - request->callback_data); - } - - g_hash_table_destroy (priv->requests); - g_error_free (error); - - g_free (priv->path); - - G_OBJECT_CLASS (nm_secret_agent_simple_parent_class)->finalize (object); -} +/*****************************************************************************/ static gboolean strv_has (char **haystack, @@ -1127,6 +1093,67 @@ nm_secret_agent_simple_enable (NMSecretAgentSimple *self, const char *path) g_list_free (requests); } +/*****************************************************************************/ + +static void +nm_secret_agent_simple_init (NMSecretAgentSimple *agent) +{ + NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (agent); + + priv->requests = g_hash_table_new_full (nm_str_hash, g_str_equal, + g_free, nm_secret_agent_simple_request_free); +} + +/** + * nm_secret_agent_simple_new: + * @name: the identifier of secret agent + * + * Creates a new #NMSecretAgentSimple. It does not serve any requests until + * nm_secret_agent_simple_enable() is called. + * + * Returns: a new #NMSecretAgentSimple if the agent creation is successful + * or %NULL in case of a failure. + */ +NMSecretAgentOld * +nm_secret_agent_simple_new (const char *name) +{ + return g_initable_new (NM_TYPE_SECRET_AGENT_SIMPLE, NULL, NULL, + NM_SECRET_AGENT_OLD_IDENTIFIER, name, + NM_SECRET_AGENT_OLD_CAPABILITIES, NM_SECRET_AGENT_CAPABILITY_VPN_HINTS, + NULL); +} + +static void +nm_secret_agent_simple_finalize (GObject *object) +{ + NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (object); + GError *error; + GHashTableIter iter; + gpointer key; + gpointer value; + + error = g_error_new (NM_SECRET_AGENT_ERROR, + NM_SECRET_AGENT_ERROR_AGENT_CANCELED, + "The secret agent is going away"); + + g_hash_table_iter_init (&iter, priv->requests); + while (g_hash_table_iter_next (&iter, &key, &value)) { + NMSecretAgentSimpleRequest *request = value; + + request->callback (NM_SECRET_AGENT_OLD (object), + request->connection, + NULL, error, + request->callback_data); + } + + g_hash_table_destroy (priv->requests); + g_error_free (error); + + g_free (priv->path); + + G_OBJECT_CLASS (nm_secret_agent_simple_parent_class)->finalize (object); +} + void nm_secret_agent_simple_class_init (NMSecretAgentSimpleClass *klass) { @@ -1174,22 +1201,3 @@ nm_secret_agent_simple_class_init (NMSecretAgentSimpleClass *klass) G_TYPE_STRING, /* prompt */ G_TYPE_PTR_ARRAY); } - -/** - * nm_secret_agent_simple_new: - * @name: the identifier of secret agent - * - * Creates a new #NMSecretAgentSimple. It does not serve any requests until - * nm_secret_agent_simple_enable() is called. - * - * Returns: a new #NMSecretAgentSimple if the agent creation is successful - * or %NULL in case of a failure. - */ -NMSecretAgentOld * -nm_secret_agent_simple_new (const char *name) -{ - return g_initable_new (NM_TYPE_SECRET_AGENT_SIMPLE, NULL, NULL, - NM_SECRET_AGENT_OLD_IDENTIFIER, name, - NM_SECRET_AGENT_OLD_CAPABILITIES, NM_SECRET_AGENT_CAPABILITY_VPN_HINTS, - NULL); -} diff --git a/clients/common/nm-secret-agent-simple.h b/clients/common/nm-secret-agent-simple.h index 529aaeaca9..0ad57a0531 100644 --- a/clients/common/nm-secret-agent-simple.h +++ b/clients/common/nm-secret-agent-simple.h @@ -21,26 +21,6 @@ #include "nm-secret-agent-old.h" -#define NM_TYPE_SECRET_AGENT_SIMPLE (nm_secret_agent_simple_get_type ()) -#define NM_SECRET_AGENT_SIMPLE(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_SECRET_AGENT_SIMPLE, NMSecretAgentSimple)) -#define NM_SECRET_AGENT_SIMPLE_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), NM_TYPE_SECRET_AGENT_SIMPLE, NMSecretAgentSimpleClass)) -#define NM_IS_SECRET_AGENT_SIMPLE(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), NM_TYPE_SECRET_AGENT_SIMPLE)) -#define NM_IS_SECRET_AGENT_SIMPLE_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), NM_TYPE_SECRET_AGENT_SIMPLE)) -#define NM_SECRET_AGENT_SIMPLE_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_SECRET_AGENT_SIMPLE, NMSecretAgentSimpleClass)) - -/* Signals */ -#define NM_SECRET_AGENT_SIMPLE_REQUEST_SECRETS "request-secrets" - -typedef struct { - NMSecretAgentOld parent; - -} NMSecretAgentSimple; - -typedef struct { - NMSecretAgentOldClass parent; - -} NMSecretAgentSimpleClass; - typedef enum { NM_SECRET_AGENT_SECRET_TYPE_PROPERTY, NM_SECRET_AGENT_SECRET_TYPE_SECRET, @@ -60,6 +40,25 @@ typedef struct { #define NM_SECRET_AGENT_VPN_TYPE_OPENCONNECT NM_DBUS_INTERFACE".openconnect" +/*****************************************************************************/ + +#define NM_TYPE_SECRET_AGENT_SIMPLE (nm_secret_agent_simple_get_type ()) +#define NM_SECRET_AGENT_SIMPLE(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_SECRET_AGENT_SIMPLE, NMSecretAgentSimple)) +#define NM_SECRET_AGENT_SIMPLE_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), NM_TYPE_SECRET_AGENT_SIMPLE, NMSecretAgentSimpleClass)) +#define NM_IS_SECRET_AGENT_SIMPLE(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), NM_TYPE_SECRET_AGENT_SIMPLE)) +#define NM_IS_SECRET_AGENT_SIMPLE_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), NM_TYPE_SECRET_AGENT_SIMPLE)) +#define NM_SECRET_AGENT_SIMPLE_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_SECRET_AGENT_SIMPLE, NMSecretAgentSimpleClass)) + +#define NM_SECRET_AGENT_SIMPLE_REQUEST_SECRETS "request-secrets" + +typedef struct { + NMSecretAgentOld parent; +} NMSecretAgentSimple; + +typedef struct { + NMSecretAgentOldClass parent; +} NMSecretAgentSimpleClass; + GType nm_secret_agent_simple_get_type (void); NMSecretAgentOld *nm_secret_agent_simple_new (const char *name); From 9d1becb0ddaa3fd2b9e313c4eea72aac038d1a79 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 10:19:35 +0100 Subject: [PATCH 05/30] clients/secret-agent: embed private data in NMSecretAgentSimple class --- clients/common/nm-secret-agent-simple.c | 13 ++++++++++--- clients/common/nm-secret-agent-simple.h | 9 ++------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index f89d225902..e2502768d7 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -70,9 +70,18 @@ typedef struct { gboolean enabled; } NMSecretAgentSimplePrivate; +struct _NMSecretAgentSimple { + NMSecretAgentOld parent; + NMSecretAgentSimplePrivate _priv; +}; + +struct _NMSecretAgentSimpleClass { + NMSecretAgentOldClass parent; +}; + G_DEFINE_TYPE (NMSecretAgentSimple, nm_secret_agent_simple, NM_TYPE_SECRET_AGENT_OLD) -#define NM_SECRET_AGENT_SIMPLE_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_SECRET_AGENT_SIMPLE, NMSecretAgentSimplePrivate)) +#define NM_SECRET_AGENT_SIMPLE_GET_PRIVATE(self) _NM_GET_PRIVATE (self, NMSecretAgentSimple, NM_IS_SECRET_AGENT_SIMPLE, NMSecretAgentOld) /*****************************************************************************/ @@ -1160,8 +1169,6 @@ nm_secret_agent_simple_class_init (NMSecretAgentSimpleClass *klass) GObjectClass *gobject_class = G_OBJECT_CLASS (klass); NMSecretAgentOldClass *agent_class = NM_SECRET_AGENT_OLD_CLASS (klass); - g_type_class_add_private (klass, sizeof (NMSecretAgentSimplePrivate)); - gobject_class->finalize = nm_secret_agent_simple_finalize; agent_class->get_secrets = nm_secret_agent_simple_get_secrets; diff --git a/clients/common/nm-secret-agent-simple.h b/clients/common/nm-secret-agent-simple.h index 0ad57a0531..89e4cf3ccf 100644 --- a/clients/common/nm-secret-agent-simple.h +++ b/clients/common/nm-secret-agent-simple.h @@ -51,13 +51,8 @@ typedef struct { #define NM_SECRET_AGENT_SIMPLE_REQUEST_SECRETS "request-secrets" -typedef struct { - NMSecretAgentOld parent; -} NMSecretAgentSimple; - -typedef struct { - NMSecretAgentOldClass parent; -} NMSecretAgentSimpleClass; +typedef struct _NMSecretAgentSimple NMSecretAgentSimple; +typedef struct _NMSecretAgentSimpleClass NMSecretAgentSimpleClass; GType nm_secret_agent_simple_get_type (void); From aaaa8902fd57b1fc85d0e495ddb76560e5b83b76 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 10:26:22 +0100 Subject: [PATCH 06/30] clients/secret-agent/trival: rename internal types, functions and variables Code that is internal to a source file should not have a "nm" prefix. That is what differenciates it from declarations in header files. It makes it clearer that these names are only defined in the current file. Also, our implementations of virtual functions shall have the same name as the function pointer of the VTable (or at least, it shouldn't have a "nm" prefix). --- clients/common/nm-secret-agent-simple.c | 318 ++++++++++++------------ 1 file changed, 159 insertions(+), 159 deletions(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index e2502768d7..86bc092180 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -52,7 +52,7 @@ typedef struct { gpointer callback_data; GCancellable *cancellable; NMSecretAgentGetSecretsFlags flags; -} NMSecretAgentSimpleRequest; +} RequestData; enum { REQUEST_SECRETS, @@ -63,7 +63,7 @@ enum { static guint signals[LAST_SIGNAL] = { 0 }; typedef struct { - /* */ + /* */ GHashTable *requests; char *path; @@ -86,21 +86,21 @@ G_DEFINE_TYPE (NMSecretAgentSimple, nm_secret_agent_simple, NM_TYPE_SECRET_AGENT /*****************************************************************************/ static void -nm_secret_agent_simple_request_free (gpointer data) +_request_data_free (gpointer data) { - NMSecretAgentSimpleRequest *request = data; + RequestData *request = data; g_clear_object (&request->cancellable); g_object_unref (request->self); g_object_unref (request->connection); g_strfreev (request->hints); - g_slice_free (NMSecretAgentSimpleRequest, request); + g_slice_free (RequestData, request); } static void -nm_secret_agent_simple_request_cancel (NMSecretAgentSimpleRequest *request, - GError *error) +_request_data_cancel (RequestData *request, + GError *error) { NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (request->self); @@ -139,12 +139,12 @@ typedef struct { NMSecretAgentSimpleSecret base; NMSetting *setting; char *property; -} NMSecretAgentSimpleSecretReal; +} SecretReal; static void -nm_secret_agent_simple_secret_free (NMSecretAgentSimpleSecret *secret) +_secret_real_free (NMSecretAgentSimpleSecret *secret) { - NMSecretAgentSimpleSecretReal *real = (NMSecretAgentSimpleSecretReal *)secret; + SecretReal *real = (SecretReal *)secret; g_free ((char *) secret->pretty_name); g_free ((char *) secret->entry_id); @@ -153,24 +153,24 @@ nm_secret_agent_simple_secret_free (NMSecretAgentSimpleSecret *secret) g_free (real->property); g_clear_object (&real->setting); - g_slice_free (NMSecretAgentSimpleSecretReal, real); + g_slice_free (SecretReal, real); } static NMSecretAgentSimpleSecret * -nm_secret_agent_simple_secret_new (NMSecretAgentSecretType secret_type, - const char *pretty_name, - NMSetting *setting, - const char *property, - const char *vpn_type) +_secret_real_new (NMSecretAgentSecretType secret_type, + const char *pretty_name, + NMSetting *setting, + const char *property, + const char *vpn_type) { - NMSecretAgentSimpleSecretReal *real; + SecretReal *real; const char *vpn_prefix; const char *value; nm_assert (property); nm_assert (NM_IS_SETTING (setting)); - real = g_slice_new0 (NMSecretAgentSimpleSecretReal); + real = g_slice_new0 (SecretReal); *((NMSecretAgentSecretType *) &real->base.secret_type) = secret_type; real->setting = g_object_ref (setting); real->base.pretty_name = g_strdup (pretty_name); @@ -200,9 +200,11 @@ nm_secret_agent_simple_secret_new (NMSecretAgentSecretType secret_type, return &real->base; } +/*****************************************************************************/ + static gboolean -add_8021x_secrets (NMSecretAgentSimpleRequest *request, - GPtrArray *secrets) +add_8021x_secrets (RequestData *request, + GPtrArray *secrets) { NMSetting8021x *s_8021x = nm_connection_get_setting_802_1x (request->connection); const char *eap_method; @@ -213,11 +215,11 @@ add_8021x_secrets (NMSecretAgentSimpleRequest *request, char **iter; for (iter = request->hints; *iter; iter++) { - secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, - _(*iter), - NM_SETTING (s_8021x), - *iter, - NULL); + secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, + _(*iter), + NM_SETTING (s_8021x), + *iter, + NULL); g_ptr_array_add (secrets, secret); } @@ -236,33 +238,33 @@ add_8021x_secrets (NMSecretAgentSimpleRequest *request, * is not visible here since we only care about phase2 authentication * (and don't even care of which one) */ - secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_PROPERTY, - _("Username"), - NM_SETTING (s_8021x), - NM_SETTING_802_1X_IDENTITY, - NULL); + secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_PROPERTY, + _("Username"), + NM_SETTING (s_8021x), + NM_SETTING_802_1X_IDENTITY, + NULL); g_ptr_array_add (secrets, secret); - secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, - _("Password"), - NM_SETTING (s_8021x), - NM_SETTING_802_1X_PASSWORD, - NULL); + secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, + _("Password"), + NM_SETTING (s_8021x), + NM_SETTING_802_1X_PASSWORD, + NULL); g_ptr_array_add (secrets, secret); return TRUE; } if (!strcmp (eap_method, "tls")) { - secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_PROPERTY, - _("Identity"), - NM_SETTING (s_8021x), - NM_SETTING_802_1X_IDENTITY, - NULL); + secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_PROPERTY, + _("Identity"), + NM_SETTING (s_8021x), + NM_SETTING_802_1X_IDENTITY, + NULL); g_ptr_array_add (secrets, secret); - secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, - _("Private key password"), - NM_SETTING (s_8021x), - NM_SETTING_802_1X_PRIVATE_KEY_PASSWORD, - NULL); + secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, + _("Private key password"), + NM_SETTING (s_8021x), + NM_SETTING_802_1X_PRIVATE_KEY_PASSWORD, + NULL); g_ptr_array_add (secrets, secret); return TRUE; } @@ -271,7 +273,7 @@ add_8021x_secrets (NMSecretAgentSimpleRequest *request, } static gboolean -add_wireless_secrets (NMSecretAgentSimpleRequest *request, +add_wireless_secrets (RequestData *request, GPtrArray *secrets) { NMSettingWirelessSecurity *s_wsec = nm_connection_get_setting_wireless_security (request->connection); @@ -282,11 +284,11 @@ add_wireless_secrets (NMSecretAgentSimpleRequest *request, return FALSE; if (!strcmp (key_mgmt, "wpa-none") || !strcmp (key_mgmt, "wpa-psk")) { - secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, - _("Password"), - NM_SETTING (s_wsec), - NM_SETTING_WIRELESS_SECURITY_PSK, - NULL); + secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, + _("Password"), + NM_SETTING (s_wsec), + NM_SETTING_WIRELESS_SECURITY_PSK, + NULL); g_ptr_array_add (secrets, secret); return TRUE; } @@ -297,11 +299,11 @@ add_wireless_secrets (NMSecretAgentSimpleRequest *request, index = nm_setting_wireless_security_get_wep_tx_keyidx (s_wsec); key = g_strdup_printf ("wep-key%d", index); - secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, - _("Key"), - NM_SETTING (s_wsec), - key, - NULL); + secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, + _("Key"), + NM_SETTING (s_wsec), + key, + NULL); g_free (key); g_ptr_array_add (secrets, secret); @@ -310,11 +312,11 @@ add_wireless_secrets (NMSecretAgentSimpleRequest *request, if (!strcmp (key_mgmt, "iee8021x")) { if (!g_strcmp0 (nm_setting_wireless_security_get_auth_alg (s_wsec), "leap")) { - secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, - _("Password"), - NM_SETTING (s_wsec), - NM_SETTING_WIRELESS_SECURITY_LEAP_PASSWORD, - NULL); + secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, + _("Password"), + NM_SETTING (s_wsec), + NM_SETTING_WIRELESS_SECURITY_LEAP_PASSWORD, + NULL); g_ptr_array_add (secrets, secret); return TRUE; } else @@ -328,29 +330,29 @@ add_wireless_secrets (NMSecretAgentSimpleRequest *request, } static gboolean -add_pppoe_secrets (NMSecretAgentSimpleRequest *request, +add_pppoe_secrets (RequestData *request, GPtrArray *secrets) { NMSettingPppoe *s_pppoe = nm_connection_get_setting_pppoe (request->connection); NMSecretAgentSimpleSecret *secret; - secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_PROPERTY, - _("Username"), - NM_SETTING (s_pppoe), - NM_SETTING_PPPOE_USERNAME, - NULL); + secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_PROPERTY, + _("Username"), + NM_SETTING (s_pppoe), + NM_SETTING_PPPOE_USERNAME, + NULL); g_ptr_array_add (secrets, secret); - secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_PROPERTY, - _("Service"), - NM_SETTING (s_pppoe), - NM_SETTING_PPPOE_SERVICE, - NULL); + secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_PROPERTY, + _("Service"), + NM_SETTING (s_pppoe), + NM_SETTING_PPPOE_SERVICE, + NULL); g_ptr_array_add (secrets, secret); - secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, - _("Password"), - NM_SETTING (s_pppoe), - NM_SETTING_PPPOE_PASSWORD, - NULL); + secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, + _("Password"), + NM_SETTING (s_pppoe), + NM_SETTING_PPPOE_PASSWORD, + NULL); g_ptr_array_add (secrets, secret); return TRUE; } @@ -378,11 +380,11 @@ add_vpn_secret_helper (GPtrArray *secrets, NMSettingVpn *s_vpn, const char *name flags = get_vpn_secret_flags (s_vpn, name); if ( flags & NM_SETTING_SECRET_FLAG_AGENT_OWNED || flags & NM_SETTING_SECRET_FLAG_NOT_SAVED) { - secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_VPN_SECRET, - ui_name, - NM_SETTING (s_vpn), - name, - nm_setting_vpn_get_service_type (s_vpn)); + secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_VPN_SECRET, + ui_name, + NM_SETTING (s_vpn), + name, + nm_setting_vpn_get_service_type (s_vpn)); /* Check for duplicates */ for (i = 0; i < secrets->len; i++) { @@ -391,7 +393,7 @@ add_vpn_secret_helper (GPtrArray *secrets, NMSettingVpn *s_vpn, const char *name if ( s->secret_type == secret->secret_type && nm_streq0 (s->vpn_type, secret->vpn_type) && nm_streq0 (s->entry_id, secret->entry_id)) { - nm_secret_agent_simple_secret_free (secret); + _secret_real_free (secret); return; } } @@ -403,7 +405,7 @@ add_vpn_secret_helper (GPtrArray *secrets, NMSettingVpn *s_vpn, const char *name #define VPN_MSG_TAG "x-vpn-message:" static gboolean -add_vpn_secrets (NMSecretAgentSimpleRequest *request, +add_vpn_secrets (RequestData *request, GPtrArray *secrets, char **msg) { @@ -438,7 +440,7 @@ typedef struct { GPid auth_dialog_pid; char read_buf[5]; GString *auth_dialog_response; - NMSecretAgentSimpleRequest *request; + RequestData *request; GPtrArray *secrets; guint child_watch_id; gulong cancellable_id; @@ -457,7 +459,7 @@ static void _auth_dialog_exited (GPid pid, int status, gpointer user_data) { AuthDialogData *data = user_data; - NMSecretAgentSimpleRequest *request = data->request; + RequestData *request = data->request; GPtrArray *secrets = data->secrets; NMSettingVpn *s_vpn = nm_connection_get_setting_vpn (request->connection); GKeyFile *keyfile = NULL; @@ -504,13 +506,11 @@ _auth_dialog_exited (GPid pid, int status, gpointer user_data) if (!g_key_file_get_boolean (keyfile, groups[i], "ShouldAsk", NULL)) continue; - g_ptr_array_add (secrets, - nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_VPN_SECRET, - g_key_file_get_string (keyfile, groups[i], "Label", NULL), - NM_SETTING (s_vpn), - groups[i], - nm_setting_vpn_get_service_type (s_vpn))); - + g_ptr_array_add (secrets, _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_VPN_SECRET, + g_key_file_get_string (keyfile, groups[i], "Label", NULL), + NM_SETTING (s_vpn), + groups[i], + nm_setting_vpn_get_service_type (s_vpn))); } out: @@ -526,7 +526,7 @@ out: } if (error) { - nm_secret_agent_simple_request_cancel (request, error); + _request_data_cancel (request, error); } else { g_signal_emit (request->self, signals[REQUEST_SECRETS], 0, request->request_id, title, message, secrets); @@ -559,7 +559,7 @@ _auth_dialog_read_done (GObject *source_object, switch (read_size) { case -1: if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) - nm_secret_agent_simple_request_cancel (data->request, error); + _request_data_cancel (data->request, error); _auth_dialog_data_free (data); break; case 0: @@ -636,7 +636,7 @@ _add_secret_to_string (const char *key, const char *value, gpointer user_data) } static gboolean -try_spawn_vpn_auth_helper (NMSecretAgentSimpleRequest *request, +try_spawn_vpn_auth_helper (RequestData *request, GPtrArray *secrets) { NMSettingVpn *s_vpn = nm_connection_get_setting_vpn (request->connection); @@ -722,7 +722,7 @@ try_spawn_vpn_auth_helper (NMSecretAgentSimpleRequest *request, } static void -request_secrets_from_ui (NMSecretAgentSimpleRequest *request) +request_secrets_from_ui (RequestData *request) { GPtrArray *secrets; NMSecretAgentSimplePrivate *priv; @@ -741,11 +741,11 @@ request_secrets_from_ui (NMSecretAgentSimpleRequest *request) error = g_error_new (NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_FAILED, "Request for %s secrets doesn't match path %s", request->request_id, priv->path); - nm_secret_agent_simple_request_cancel (request, error); + _request_data_cancel (request, error); return; } - secrets = g_ptr_array_new_with_free_func ((GDestroyNotify) nm_secret_agent_simple_secret_free); + secrets = g_ptr_array_new_with_free_func ((GDestroyNotify) _secret_real_free); if (nm_connection_is_type (request->connection, NM_SETTING_WIRELESS_SETTING_NAME)) { NMSettingWireless *s_wireless; @@ -780,22 +780,22 @@ request_secrets_from_ui (NMSecretAgentSimpleRequest *request) title = _("PIN code required"); msg = g_strdup (_("PIN code is needed for the mobile broadband device")); - secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_PROPERTY, - _("PIN"), - NM_SETTING (s_gsm), - NM_SETTING_GSM_PIN, - NULL); + secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_PROPERTY, + _("PIN"), + NM_SETTING (s_gsm), + NM_SETTING_GSM_PIN, + NULL); g_ptr_array_add (secrets, secret); } else { title = _("Mobile broadband network password"); msg = g_strdup_printf (_("A password is required to connect to '%s'."), nm_connection_get_id (request->connection)); - secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, - _("Password"), - NM_SETTING (s_gsm), - NM_SETTING_GSM_PASSWORD, - NULL); + secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, + _("Password"), + NM_SETTING (s_gsm), + NM_SETTING_GSM_PASSWORD, + NULL); g_ptr_array_add (secrets, secret); } } else if (nm_connection_is_type (request->connection, NM_SETTING_MACSEC_SETTING_NAME)) { @@ -806,11 +806,11 @@ request_secrets_from_ui (NMSecretAgentSimpleRequest *request) if (nm_setting_macsec_get_mode (s_macsec) == NM_SETTING_MACSEC_MODE_PSK) { title = _("MACsec PSK authentication"); - secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, - _("MKA CAK"), - NM_SETTING (s_macsec), - NM_SETTING_MACSEC_MKA_CAK, - NULL); + secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, + _("MKA CAK"), + NM_SETTING (s_macsec), + NM_SETTING_MACSEC_MKA_CAK, + NULL); g_ptr_array_add (secrets, secret); } else { title = _("MACsec EAP authentication"); @@ -823,11 +823,11 @@ request_secrets_from_ui (NMSecretAgentSimpleRequest *request) msg = g_strdup_printf (_("A password is required to connect to '%s'."), nm_connection_get_id (request->connection)); - secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, - _("Password"), - NM_SETTING (s_cdma), - NM_SETTING_CDMA_PASSWORD, - NULL); + secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, + _("Password"), + NM_SETTING (s_cdma), + NM_SETTING_CDMA_PASSWORD, + NULL); g_ptr_array_add (secrets, secret); } else if (nm_connection_is_type (request->connection, NM_SETTING_BLUETOOTH_SETTING_NAME)) { NMSetting *setting = NULL; @@ -845,11 +845,11 @@ request_secrets_from_ui (NMSecretAgentSimpleRequest *request) msg = g_strdup_printf (_("A password is required to connect to '%s'."), nm_connection_get_id (request->connection)); - secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, - _("Password"), - setting, - "password", - NULL); + secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, + _("Password"), + setting, + "password", + NULL); g_ptr_array_add (secrets, secret); } else ok = FALSE; @@ -876,7 +876,7 @@ request_secrets_from_ui (NMSecretAgentSimpleRequest *request) "Cannot service a secrets request %s for a %s connection", request->request_id, nm_connection_get_connection_type (request->connection)); - nm_secret_agent_simple_request_cancel (request, error); + _request_data_cancel (request, error); g_ptr_array_unref (secrets); return; } @@ -886,18 +886,18 @@ request_secrets_from_ui (NMSecretAgentSimpleRequest *request) } static void -nm_secret_agent_simple_get_secrets (NMSecretAgentOld *agent, - NMConnection *connection, - const char *connection_path, - const char *setting_name, - const char **hints, - NMSecretAgentGetSecretsFlags flags, - NMSecretAgentOldGetSecretsFunc callback, - gpointer callback_data) +get_secrets (NMSecretAgentOld *agent, + NMConnection *connection, + const char *connection_path, + const char *setting_name, + const char **hints, + NMSecretAgentGetSecretsFlags flags, + NMSecretAgentOldGetSecretsFunc callback, + gpointer callback_data) { NMSecretAgentSimple *self = NM_SECRET_AGENT_SIMPLE (agent); NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (self); - NMSecretAgentSimpleRequest *request; + RequestData *request; char *request_id; GError *error; @@ -920,7 +920,7 @@ nm_secret_agent_simple_get_secrets (NMSecretAgentOld *agent, goto nope; } - request = g_slice_new (NMSecretAgentSimpleRequest); + request = g_slice_new (RequestData); request->self = g_object_ref (self); request->connection = g_object_ref (connection); request->hints = g_strdupv ((char **)hints); @@ -955,7 +955,7 @@ nm_secret_agent_simple_response (NMSecretAgentSimple *self, GPtrArray *secrets) { NMSecretAgentSimplePrivate *priv; - NMSecretAgentSimpleRequest *request; + RequestData *request; GVariant *dict = NULL; GError *error = NULL; int i; @@ -976,7 +976,7 @@ nm_secret_agent_simple_response (NMSecretAgentSimple *self, settings = g_hash_table_new (nm_str_hash, g_str_equal); for (i = 0; i < secrets->len; i++) { - NMSecretAgentSimpleSecretReal *secret = secrets->pdata[i]; + SecretReal *secret = secrets->pdata[i]; setting_builder = g_hash_table_lookup (settings, nm_setting_get_name (secret->setting)); if (!setting_builder) { @@ -1027,9 +1027,9 @@ nm_secret_agent_simple_response (NMSecretAgentSimple *self, } static void -nm_secret_agent_simple_cancel_get_secrets (NMSecretAgentOld *agent, - const char *connection_path, - const char *setting_name) +cancel_get_secrets (NMSecretAgentOld *agent, + const char *connection_path, + const char *setting_name) { NMSecretAgentSimple *self = NM_SECRET_AGENT_SIMPLE (agent); NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (self); @@ -1040,22 +1040,22 @@ nm_secret_agent_simple_cancel_get_secrets (NMSecretAgentOld *agent, } static void -nm_secret_agent_simple_save_secrets (NMSecretAgentOld *agent, - NMConnection *connection, - const char *connection_path, - NMSecretAgentOldSaveSecretsFunc callback, - gpointer callback_data) +save_secrets (NMSecretAgentOld *agent, + NMConnection *connection, + const char *connection_path, + NMSecretAgentOldSaveSecretsFunc callback, + gpointer callback_data) { /* We don't support secret storage */ callback (agent, connection, NULL, callback_data); } static void -nm_secret_agent_simple_delete_secrets (NMSecretAgentOld *agent, - NMConnection *connection, - const char *connection_path, - NMSecretAgentOldDeleteSecretsFunc callback, - gpointer callback_data) +delete_secrets (NMSecretAgentOld *agent, + NMConnection *connection, + const char *connection_path, + NMSecretAgentOldDeleteSecretsFunc callback, + gpointer callback_data) { /* We don't support secret storage, so there's nothing to delete. */ callback (agent, connection, NULL, callback_data); @@ -1110,7 +1110,7 @@ nm_secret_agent_simple_init (NMSecretAgentSimple *agent) NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (agent); priv->requests = g_hash_table_new_full (nm_str_hash, g_str_equal, - g_free, nm_secret_agent_simple_request_free); + g_free, _request_data_free); } /** @@ -1133,7 +1133,7 @@ nm_secret_agent_simple_new (const char *name) } static void -nm_secret_agent_simple_finalize (GObject *object) +finalize (GObject *object) { NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (object); GError *error; @@ -1147,7 +1147,7 @@ nm_secret_agent_simple_finalize (GObject *object) g_hash_table_iter_init (&iter, priv->requests); while (g_hash_table_iter_next (&iter, &key, &value)) { - NMSecretAgentSimpleRequest *request = value; + RequestData *request = value; request->callback (NM_SECRET_AGENT_OLD (object), request->connection, @@ -1166,15 +1166,15 @@ nm_secret_agent_simple_finalize (GObject *object) void nm_secret_agent_simple_class_init (NMSecretAgentSimpleClass *klass) { - GObjectClass *gobject_class = G_OBJECT_CLASS (klass); + GObjectClass *object_class = G_OBJECT_CLASS (klass); NMSecretAgentOldClass *agent_class = NM_SECRET_AGENT_OLD_CLASS (klass); - gobject_class->finalize = nm_secret_agent_simple_finalize; + object_class->finalize = finalize; - agent_class->get_secrets = nm_secret_agent_simple_get_secrets; - agent_class->cancel_get_secrets = nm_secret_agent_simple_cancel_get_secrets; - agent_class->save_secrets = nm_secret_agent_simple_save_secrets; - agent_class->delete_secrets = nm_secret_agent_simple_delete_secrets; + agent_class->get_secrets = get_secrets; + agent_class->cancel_get_secrets = cancel_get_secrets; + agent_class->save_secrets = save_secrets; + agent_class->delete_secrets = delete_secrets; /** * NMSecretAgentSimple::request-secrets: From 378a4a8e1a58ac04758842a1a0a6ef117b562dca Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 10:28:20 +0100 Subject: [PATCH 07/30] clients/secret-agent: drop strv_has() implementation --- clients/common/nm-secret-agent-simple.c | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index 86bc092180..56d4a32361 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -111,20 +111,6 @@ _request_data_cancel (RequestData *request, /*****************************************************************************/ -static gboolean -strv_has (char **haystack, - char *needle) -{ - char **iter; - - for (iter = haystack; iter && *iter; iter++) { - if (g_strcmp0 (*iter, needle) == 0) - return TRUE; - } - - return FALSE; -} - /** * NMSecretAgentSimpleSecret: * @name: the user-visible name of the secret. Eg, "WEP Passphrase". @@ -776,7 +762,7 @@ request_secrets_from_ui (RequestData *request) } else if (nm_connection_is_type (request->connection, NM_SETTING_GSM_SETTING_NAME)) { NMSettingGsm *s_gsm = nm_connection_get_setting_gsm (request->connection); - if (strv_has (request->hints, "pin")) { + if (g_strv_contains (NM_CAST_STRV_CC (request->hints), "pin")) { title = _("PIN code required"); msg = g_strdup (_("PIN code is needed for the mobile broadband device")); From 73f423c5e514433448cee94489c54e5ba479acbb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 10:40:26 +0100 Subject: [PATCH 08/30] clients/secret-agent: various cleanups in secret-agent-simple --- clients/common/nm-secret-agent-simple.c | 113 +++++++++++------------- 1 file changed, 54 insertions(+), 59 deletions(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index 56d4a32361..b5ba3598fc 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -90,7 +90,7 @@ _request_data_free (gpointer data) { RequestData *request = data; - g_clear_object (&request->cancellable); + g_object_unref (request->cancellable); g_object_unref (request->self); g_object_unref (request->connection); g_strfreev (request->hints); @@ -216,10 +216,10 @@ add_8021x_secrets (RequestData *request, if (!eap_method) return FALSE; - if ( !strcmp (eap_method, "md5") - || !strcmp (eap_method, "leap") - || !strcmp (eap_method, "ttls") - || !strcmp (eap_method, "peap")) { + if (NM_IN_STRSET (eap_method, "md5", + "leap", + "ttls", + "peap")) { /* TTLS and PEAP are actually much more complicated, but this complication * is not visible here since we only care about phase2 authentication * (and don't even care of which one) @@ -239,7 +239,7 @@ add_8021x_secrets (RequestData *request, return TRUE; } - if (!strcmp (eap_method, "tls")) { + if (nm_streq (eap_method, "tls")) { secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_PROPERTY, _("Identity"), NM_SETTING (s_8021x), @@ -269,7 +269,8 @@ add_wireless_secrets (RequestData *request, if (!key_mgmt) return FALSE; - if (!strcmp (key_mgmt, "wpa-none") || !strcmp (key_mgmt, "wpa-psk")) { + if (NM_IN_STRSET (key_mgmt, "wpa-none", + "wpa-psk")) { secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, _("Password"), NM_SETTING (s_wsec), @@ -279,25 +280,22 @@ add_wireless_secrets (RequestData *request, return TRUE; } - if (!strcmp (key_mgmt, "none")) { - int index; - char *key; + if (nm_streq (key_mgmt, "none")) { + guint32 index; + char key[100]; index = nm_setting_wireless_security_get_wep_tx_keyidx (s_wsec); - key = g_strdup_printf ("wep-key%d", index); secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, _("Key"), NM_SETTING (s_wsec), - key, + nm_sprintf_buf (key, "wep-key%u", (guint) index), NULL); - g_free (key); - g_ptr_array_add (secrets, secret); return TRUE; } - if (!strcmp (key_mgmt, "iee8021x")) { - if (!g_strcmp0 (nm_setting_wireless_security_get_auth_alg (s_wsec), "leap")) { + if (nm_streq (key_mgmt, "iee8021x")) { + if (nm_streq0 (nm_setting_wireless_security_get_auth_alg (s_wsec), "leap")) { secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, _("Password"), NM_SETTING (s_wsec), @@ -309,7 +307,7 @@ add_wireless_secrets (RequestData *request, return add_8021x_secrets (request, secrets); } - if (!strcmp (key_mgmt, "wpa-eap")) + if (nm_streq (key_mgmt, "wpa-eap")) return add_8021x_secrets (request, secrets); return FALSE; @@ -448,7 +446,7 @@ _auth_dialog_exited (GPid pid, int status, gpointer user_data) RequestData *request = data->request; GPtrArray *secrets = data->secrets; NMSettingVpn *s_vpn = nm_connection_get_setting_vpn (request->connection); - GKeyFile *keyfile = NULL; + gs_unref_keyfile GKeyFile *keyfile = NULL; gs_strfreev char **groups = NULL; gs_free char *title = NULL; gs_free char *message = NULL; @@ -472,7 +470,7 @@ _auth_dialog_exited (GPid pid, int status, gpointer user_data) } groups = g_key_file_get_groups (keyfile, NULL); - if (g_strcmp0 (groups[0], "VPN Plugin UI") != 0) { + if (!nm_streq0 (groups[0], "VPN Plugin UI")) { g_set_error (&error, NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_FAILED, "Expected [VPN Plugin UI] in auth dialog response"); goto out; @@ -511,14 +509,13 @@ out: } } - if (error) { + if (error) _request_data_cancel (request, error); - } else { + else { g_signal_emit (request->self, signals[REQUEST_SECRETS], 0, request->request_id, title, message, secrets); } - g_clear_pointer (&keyfile, g_key_file_unref); _auth_dialog_data_free (data); } @@ -576,12 +573,11 @@ _auth_dialog_write_done (GObject *source_object, gpointer user_data) { GOutputStream *auth_dialog_out = G_OUTPUT_STREAM (source_object); - GString *auth_dialog_request = user_data; + _nm_unused nm_auto_free_gstring GString *auth_dialog_request_free = user_data; /* We don't care about write errors. If there are any problems, the * reader shall notice. */ g_output_stream_write_finish (auth_dialog_out, res, NULL); - g_string_free (auth_dialog_request, TRUE); g_output_stream_close (auth_dialog_out, NULL, NULL); } @@ -681,12 +677,13 @@ try_spawn_vpn_auth_helper (RequestData *request, nm_setting_vpn_foreach_data_item (s_vpn, _add_data_item_to_string, auth_dialog_request); nm_setting_vpn_foreach_secret (s_vpn, _add_secret_to_string, auth_dialog_request); g_string_append (auth_dialog_request, "DONE\nQUIT\n"); - - data = g_slice_alloc0 (sizeof (AuthDialogData)); - data->auth_dialog_response = g_string_new_len (NULL, sizeof (data->read_buf)); - data->auth_dialog_pid = auth_dialog_pid; - data->request = request; - data->secrets = secrets; + data = g_slice_new (AuthDialogData); + *data = (AuthDialogData) { + .auth_dialog_response = g_string_new_len (NULL, sizeof (data->read_buf)), + .auth_dialog_pid = auth_dialog_pid, + .request = request, + .secrets = secrets, + }; g_output_stream_write_async (auth_dialog_in, auth_dialog_request->str, @@ -884,18 +881,16 @@ get_secrets (NMSecretAgentOld *agent, NMSecretAgentSimple *self = NM_SECRET_AGENT_SIMPLE (agent); NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (self); RequestData *request; - char *request_id; - GError *error; + gs_free_error GError *error = NULL; + gs_free char *request_id = NULL; request_id = g_strdup_printf ("%s/%s", connection_path, setting_name); - if (g_hash_table_lookup (priv->requests, request_id) != NULL) { + + if (g_hash_table_contains (priv->requests, request_id)) { /* We already have a request pending for this (connection, setting) */ error = g_error_new (NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_FAILED, "Request for %s secrets already pending", request_id); - nope: callback (agent, connection, NULL, error, callback_data); - g_error_free (error); - g_free (request_id); return; } @@ -903,18 +898,21 @@ get_secrets (NMSecretAgentOld *agent, /* We don't do stored passwords */ error = g_error_new (NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_NO_SECRETS, "Stored passwords not supported"); - goto nope; + callback (agent, connection, NULL, error, callback_data); + return; } request = g_slice_new (RequestData); - request->self = g_object_ref (self); - request->connection = g_object_ref (connection); - request->hints = g_strdupv ((char **)hints); - request->callback = callback; - request->callback_data = callback_data; - request->request_id = request_id; - request->flags = flags; - request->cancellable = g_cancellable_new (); + *request = (RequestData) { + .self = g_object_ref (self), + .connection = g_object_ref (connection), + .hints = g_strdupv ((char **) hints), + .callback = callback, + .callback_data = callback_data, + .request_id = g_steal_pointer (&request_id), + .flags = flags, + .cancellable = g_cancellable_new (), + }; g_hash_table_replace (priv->requests, request->request_id, request); if (priv->enabled) @@ -1070,10 +1068,9 @@ nm_secret_agent_simple_enable (NMSecretAgentSimple *self, const char *path) */ path_full = path ? g_strdup_printf ("%s/", path) : NULL; - if (g_strcmp0 (path_full, priv->path) != 0) { + if (!nm_streq0 (path_full, priv->path)) { g_free (priv->path); - priv->path = path_full; - path_full = NULL; + priv->path = g_steal_pointer (&path_full); } if (priv->enabled) @@ -1122,19 +1119,18 @@ static void finalize (GObject *object) { NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (object); - GError *error; + gs_free_error GError *error = NULL; GHashTableIter iter; - gpointer key; - gpointer value; - - error = g_error_new (NM_SECRET_AGENT_ERROR, - NM_SECRET_AGENT_ERROR_AGENT_CANCELED, - "The secret agent is going away"); + RequestData *request; g_hash_table_iter_init (&iter, priv->requests); - while (g_hash_table_iter_next (&iter, &key, &value)) { - RequestData *request = value; - + while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &request)) { + if (!error) { + g_set_error (&error, + NM_SECRET_AGENT_ERROR, + NM_SECRET_AGENT_ERROR_AGENT_CANCELED, + "The secret agent is going away"); + } request->callback (NM_SECRET_AGENT_OLD (object), request->connection, NULL, error, @@ -1142,7 +1138,6 @@ finalize (GObject *object) } g_hash_table_destroy (priv->requests); - g_error_free (error); g_free (priv->path); From 5572c8f81c19ed8a1152d8f76a719c2b647c9150 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 10:55:19 +0100 Subject: [PATCH 09/30] clients/secret-agent: only pass char buffer to _auth_dialog_write_done() We don't need the entire GString. It's only to keep the buffer alive for long enough. --- clients/common/nm-secret-agent-simple.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index b5ba3598fc..2b0883e5c2 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -573,7 +573,7 @@ _auth_dialog_write_done (GObject *source_object, gpointer user_data) { GOutputStream *auth_dialog_out = G_OUTPUT_STREAM (source_object); - _nm_unused nm_auto_free_gstring GString *auth_dialog_request_free = user_data; + _nm_unused gs_free char *auth_dialog_request_free = user_data; /* We don't care about write errors. If there are any problems, the * reader shall notice. */ @@ -640,6 +640,8 @@ try_spawn_vpn_auth_helper (RequestData *request, GInputStream *auth_dialog_out; GError *error = NULL; GString *auth_dialog_request; + char *auth_dialog_request_str; + gsize auth_dialog_request_len; AuthDialogData *data; plugin_info = nm_vpn_plugin_info_list_find_by_service (nm_vpn_get_plugin_infos (), @@ -677,6 +679,9 @@ try_spawn_vpn_auth_helper (RequestData *request, nm_setting_vpn_foreach_data_item (s_vpn, _add_data_item_to_string, auth_dialog_request); nm_setting_vpn_foreach_secret (s_vpn, _add_secret_to_string, auth_dialog_request); g_string_append (auth_dialog_request, "DONE\nQUIT\n"); + auth_dialog_request_len = auth_dialog_request->len; + auth_dialog_request_str = g_string_free (auth_dialog_request, FALSE); + data = g_slice_new (AuthDialogData); *data = (AuthDialogData) { .auth_dialog_response = g_string_new_len (NULL, sizeof (data->read_buf)), @@ -686,12 +691,12 @@ try_spawn_vpn_auth_helper (RequestData *request, }; g_output_stream_write_async (auth_dialog_in, - auth_dialog_request->str, - auth_dialog_request->len, + auth_dialog_request_str, + auth_dialog_request_len, G_PRIORITY_DEFAULT, request->cancellable, _auth_dialog_write_done, - auth_dialog_request); + auth_dialog_request_str); g_input_stream_read_async (auth_dialog_out, data->read_buf, From 4157092a8a886a8158b8e0d7e152b3311aed258a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 10:55:19 +0100 Subject: [PATCH 10/30] clients/secret-agent: rework tracking of requests in secret-agent-simple Note that previously the @requests hash took the request-id as key and the RequestData as value. Likewise, the destroy functions of the head would destroy the key and the value. However, RequestData also had a field "request_id". But that pointer was not owned (nor freed) by the RequestData structure. Instead, it was relied that the hash kept the request-id alive long enough. That is confusing. Let RequestData own the request-id. Also, we don't need to track a separate key. Just move the request-id as first filed in RequestData, and use compare/hash functions that handle that correctly (nm_pstr_*()). --- clients/common/nm-secret-agent-simple.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index 2b0883e5c2..28d510c78f 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -43,9 +43,10 @@ /*****************************************************************************/ typedef struct { + char *request_id; + NMSecretAgentSimple *self; - char *request_id; NMConnection *connection; char **hints; NMSecretAgentOldGetSecretsFunc callback; @@ -63,7 +64,6 @@ enum { static guint signals[LAST_SIGNAL] = { 0 }; typedef struct { - /* */ GHashTable *requests; char *path; @@ -90,6 +90,7 @@ _request_data_free (gpointer data) { RequestData *request = data; + g_free (request->request_id); g_object_unref (request->cancellable); g_object_unref (request->self); g_object_unref (request->connection); @@ -106,7 +107,7 @@ _request_data_cancel (RequestData *request, request->callback (NM_SECRET_AGENT_OLD (request->self), request->connection, NULL, error, request->callback_data); - g_hash_table_remove (priv->requests, request->request_id); + g_hash_table_remove (priv->requests, request); } /*****************************************************************************/ @@ -891,7 +892,7 @@ get_secrets (NMSecretAgentOld *agent, request_id = g_strdup_printf ("%s/%s", connection_path, setting_name); - if (g_hash_table_contains (priv->requests, request_id)) { + if (g_hash_table_contains (priv->requests, &request_id)) { /* We already have a request pending for this (connection, setting) */ error = g_error_new (NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_FAILED, "Request for %s secrets already pending", request_id); @@ -918,7 +919,7 @@ get_secrets (NMSecretAgentOld *agent, .flags = flags, .cancellable = g_cancellable_new (), }; - g_hash_table_replace (priv->requests, request->request_id, request); + g_hash_table_add (priv->requests, request); if (priv->enabled) request_secrets_from_ui (request); @@ -952,7 +953,7 @@ nm_secret_agent_simple_response (NMSecretAgentSimple *self, g_return_if_fail (NM_IS_SECRET_AGENT_SIMPLE (self)); priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (self); - request = g_hash_table_lookup (priv->requests, request_id); + request = g_hash_table_lookup (priv->requests, &request_id); g_return_if_fail (request != NULL); if (secrets) { @@ -1012,7 +1013,7 @@ nm_secret_agent_simple_response (NMSecretAgentSimple *self, request->callback (NM_SECRET_AGENT_OLD (self), request->connection, dict, error, request->callback_data); g_clear_error (&error); - g_hash_table_remove (priv->requests, request_id); + g_hash_table_remove (priv->requests, request); } static void @@ -1025,7 +1026,7 @@ cancel_get_secrets (NMSecretAgentOld *agent, gs_free char *request_id = NULL; request_id = g_strdup_printf ("%s/%s", connection_path, setting_name); - g_hash_table_remove (priv->requests, request_id); + g_hash_table_remove (priv->requests, &request_id); } static void @@ -1097,8 +1098,9 @@ nm_secret_agent_simple_init (NMSecretAgentSimple *agent) { NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (agent); - priv->requests = g_hash_table_new_full (nm_str_hash, g_str_equal, - g_free, _request_data_free); + G_STATIC_ASSERT_EXPR (G_STRUCT_OFFSET (RequestData, request_id) == 0); + priv->requests = g_hash_table_new_full (nm_pstr_hash, nm_pstr_equal, + NULL, _request_data_free); } /** From f2973fd72ee18136a2a320246a5b8cec2a39815d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 11:29:12 +0100 Subject: [PATCH 11/30] clients/secret-agent: remove request in finalize loop early It's ugly to keep the request in the list. Just remove it right away. --- clients/common/nm-secret-agent-simple.c | 1 + 1 file changed, 1 insertion(+) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index 28d510c78f..ba20d489a3 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -1142,6 +1142,7 @@ finalize (GObject *object) request->connection, NULL, error, request->callback_data); + g_hash_table_iter_remove (&iter); } g_hash_table_destroy (priv->requests); From 16e0f38c3eee08e03cb71342d9208c1b41a725a5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 11:40:59 +0100 Subject: [PATCH 12/30] clients/secret-agent: cancel pending operations We must actually cancel the GCancellable. Otherwise, the pending async operations are not cancelled. _auth_dialog_write_done() doesn't care about that, but _auth_dialog_read_done() does. It must not touch the destroyed data, after the operation is cancelled. --- clients/common/nm-secret-agent-simple.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index ba20d489a3..72c5057cc1 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -91,7 +91,7 @@ _request_data_free (gpointer data) RequestData *request = data; g_free (request->request_id); - g_object_unref (request->cancellable); + nm_clear_g_cancellable (&request->cancellable); g_object_unref (request->self); g_object_unref (request->connection); g_strfreev (request->hints); From 8b951afac9d4d057cee974b9da67ae93925564d9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 11:40:59 +0100 Subject: [PATCH 13/30] clients/secret-agent: don't let request keep secret-agent alive Don't let RequestData keep the parent NMSecretAgentSimple instance alive. Previously, the code in finalize() was never actually reached. Also, move the final callback from finalize() to dispose(). It feels wrong to invoke callbacks from finalize(). --- clients/common/nm-secret-agent-simple.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index 72c5057cc1..aae0196165 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -92,7 +92,6 @@ _request_data_free (gpointer data) g_free (request->request_id); nm_clear_g_cancellable (&request->cancellable); - g_object_unref (request->self); g_object_unref (request->connection); g_strfreev (request->hints); @@ -910,7 +909,7 @@ get_secrets (NMSecretAgentOld *agent, request = g_slice_new (RequestData); *request = (RequestData) { - .self = g_object_ref (self), + .self = self, .connection = g_object_ref (connection), .hints = g_strdupv ((char **) hints), .callback = callback, @@ -1123,7 +1122,7 @@ nm_secret_agent_simple_new (const char *name) } static void -finalize (GObject *object) +dispose (GObject *object) { NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (object); gs_free_error GError *error = NULL; @@ -1145,6 +1144,14 @@ finalize (GObject *object) g_hash_table_iter_remove (&iter); } + G_OBJECT_CLASS (nm_secret_agent_simple_parent_class)->dispose (object); +} + +static void +finalize (GObject *object) +{ + NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (object); + g_hash_table_destroy (priv->requests); g_free (priv->path); @@ -1158,6 +1165,7 @@ nm_secret_agent_simple_class_init (NMSecretAgentSimpleClass *klass) GObjectClass *object_class = G_OBJECT_CLASS (klass); NMSecretAgentOldClass *agent_class = NM_SECRET_AGENT_OLD_CLASS (klass); + object_class->dispose = dispose; object_class->finalize = finalize; agent_class->get_secrets = get_secrets; From d68bdce206d97e4f1a08c7568b7f2f3318a4ee88 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 11:46:20 +0100 Subject: [PATCH 14/30] clients/secret-agent: minor cleanup of child-watch-id for secret-agent The code was correct. But it's hard to follow when and whether the child-watch-id was destroyed at the right time. Instead, always let _auth_dialog_data_free() clear the signal handlers. --- clients/common/nm-secret-agent-simple.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index aae0196165..2c83de44b3 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -422,17 +422,21 @@ add_vpn_secrets (RequestData *request, typedef struct { GPid auth_dialog_pid; - char read_buf[5]; GString *auth_dialog_response; RequestData *request; GPtrArray *secrets; - guint child_watch_id; + GCancellable *cancellable; gulong cancellable_id; + guint child_watch_id; + char read_buf[5]; } AuthDialogData; static void _auth_dialog_data_free (AuthDialogData *data) { + nm_clear_g_signal_handler (data->cancellable, &data->cancellable_id); + g_clear_object (&data->cancellable); + nm_clear_g_source (&data->child_watch_id); g_ptr_array_unref (data->secrets); g_spawn_close_pid (data->auth_dialog_pid); g_string_free (data->auth_dialog_response, TRUE); @@ -453,7 +457,9 @@ _auth_dialog_exited (GPid pid, int status, gpointer user_data) int i; gs_free_error GError *error = NULL; - g_cancellable_disconnect (request->cancellable, data->cancellable_id); + data->child_watch_id = 0; + + nm_clear_g_cancellable_disconnect (data->cancellable, &data->cancellable_id); if (status != 0) { g_set_error (&error, NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_FAILED, @@ -522,10 +528,7 @@ out: static void _request_cancelled (GObject *object, gpointer user_data) { - AuthDialogData *data = user_data; - - g_source_remove (data->child_watch_id); - _auth_dialog_data_free (data); + _auth_dialog_data_free (user_data); } static void @@ -549,7 +552,8 @@ _auth_dialog_read_done (GObject *source_object, /* Done reading. Let's wait for the auth dialog to exit so that we're able to collect the status. * Remember we can be cancelled in between. */ data->child_watch_id = g_child_watch_add (data->auth_dialog_pid, _auth_dialog_exited, data); - data->cancellable_id = g_cancellable_connect (data->request->cancellable, + data->cancellable = g_object_ref (data->request->cancellable); + data->cancellable_id = g_cancellable_connect (data->cancellable, G_CALLBACK (_request_cancelled), data, NULL); break; default: From 82472c557cc966b14d3e1186605a7fc3e37dc17d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 12:15:03 +0100 Subject: [PATCH 15/30] clients/secret-agent: use nm-utils error reason for callback while disposing NMSecretAgentSimple No caller cared about the NM_SECRET_AGENT_ERROR_AGENT_CANCELED reason. In particular, because previously the requests would keep the secret-agent instance alive, and this never happend. Also, NM_SECRET_AGENT_ERROR_AGENT_CANCELED precicley exists for NMSecretAgentOld:cancel_get_secrets() (as documented). During finalize we are not cancelled -- at least not the same way as cancel_get_secrets(). Setting NM_SECRET_AGENT_ERROR_AGENT_CANCELED is wrong. Anyway, we have a default error for such cases already. --- clients/common/nm-secret-agent-simple.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index 2c83de44b3..b4530af61d 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -1135,15 +1135,12 @@ dispose (GObject *object) g_hash_table_iter_init (&iter, priv->requests); while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &request)) { - if (!error) { - g_set_error (&error, - NM_SECRET_AGENT_ERROR, - NM_SECRET_AGENT_ERROR_AGENT_CANCELED, - "The secret agent is going away"); - } + if (!error) + nm_utils_error_set_cancelled (&error, TRUE, "NMSecretAgentSimple"); request->callback (NM_SECRET_AGENT_OLD (object), request->connection, - NULL, error, + NULL, + error, request->callback_data); g_hash_table_iter_remove (&iter); } From 93c848ca036d96396bbc5ecd9486b682f08b4fef Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 12:51:10 +0100 Subject: [PATCH 16/30] clients: don't tread secret agent as NMSecretAgentOld Most of the times we actually need a NMSecretAgentSimple typed pointer. This way, need need to cast less. But even if we would need to cast more, it's better to have pointers point to the actual type, not merely to avoid shortcomings of C. --- clients/cli/agent.c | 2 +- clients/cli/common.c | 2 +- clients/cli/connections.c | 5 ++--- clients/cli/devices.c | 2 +- clients/cli/nmcli.c | 2 +- clients/cli/nmcli.h | 4 ++-- clients/common/nm-secret-agent-simple.c | 2 +- clients/common/nm-secret-agent-simple.h | 2 +- clients/tui/nmtui-connect.c | 8 ++++---- 9 files changed, 14 insertions(+), 15 deletions(-) diff --git a/clients/cli/agent.c b/clients/cli/agent.c index 05cc5dca5a..55a5ba6b85 100644 --- a/clients/cli/agent.c +++ b/clients/cli/agent.c @@ -149,7 +149,7 @@ do_agent_secret (NmCli *nmc, int argc, char **argv) /* We keep running */ nmc->should_wait++; - nm_secret_agent_simple_enable (NM_SECRET_AGENT_SIMPLE (nmc->secret_agent), NULL); + nm_secret_agent_simple_enable (nmc->secret_agent, NULL); g_signal_connect (nmc->secret_agent, NM_SECRET_AGENT_SIMPLE_REQUEST_SECRETS, G_CALLBACK (secrets_requested), diff --git a/clients/cli/common.c b/clients/cli/common.c index 85b5005e44..56e5062f52 100644 --- a/clients/cli/common.c +++ b/clients/cli/common.c @@ -778,7 +778,7 @@ nmc_secrets_requested (NMSecretAgentSimple *agent, /* Unregister our secret agent on failure, so that another agent * may be tried */ if (nmc->secret_agent) { - nm_secret_agent_old_unregister (nmc->secret_agent, NULL, NULL); + nm_secret_agent_old_unregister (NM_SECRET_AGENT_OLD (nmc->secret_agent), NULL, NULL); g_clear_object (&nmc->secret_agent); } } diff --git a/clients/cli/connections.c b/clients/cli/connections.c index e2c23cb8c1..b8fac54e5c 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -2474,7 +2474,7 @@ check_activated (ActivateConnectionInfo *info) if (nmc->secret_agent) { NMRemoteConnection *connection = nm_active_connection_get_connection (info->active); - nm_secret_agent_simple_enable (NM_SECRET_AGENT_SIMPLE (nmc->secret_agent), + nm_secret_agent_simple_enable (nmc->secret_agent, nm_connection_get_path (NM_CONNECTION (connection))); } break; @@ -2776,7 +2776,6 @@ nmc_activate_connection (NmCli *nmc, g_hash_table_destroy (nmc->pwds_hash); nmc->pwds_hash = pwds_hash; - /* Create secret agent */ nmc->secret_agent = nm_secret_agent_simple_new ("nmcli-connect"); if (nmc->secret_agent) { g_signal_connect (nmc->secret_agent, @@ -6728,7 +6727,7 @@ progress_activation_editor_cb (gpointer user_data) NMRemoteConnection *connection; connection = nm_active_connection_get_connection (ac); - nm_secret_agent_simple_enable (NM_SECRET_AGENT_SIMPLE (info->nmc->secret_agent), + nm_secret_agent_simple_enable (info->nmc->secret_agent, nm_object_get_path (NM_OBJECT (connection))); } diff --git a/clients/cli/devices.c b/clients/cli/devices.c index 44573fb649..d17cf79980 100644 --- a/clients/cli/devices.c +++ b/clients/cli/devices.c @@ -1961,7 +1961,7 @@ connect_device_cb (GObject *client, GAsyncResult *result, gpointer user_data) if (nmc->secret_agent) { NMRemoteConnection *connection = nm_active_connection_get_connection (active); - nm_secret_agent_simple_enable (NM_SECRET_AGENT_SIMPLE (nmc->secret_agent), + nm_secret_agent_simple_enable (nmc->secret_agent, nm_connection_get_path (NM_CONNECTION (connection))); } diff --git a/clients/cli/nmcli.c b/clients/cli/nmcli.c index a554a382a4..6d38a58857 100644 --- a/clients/cli/nmcli.c +++ b/clients/cli/nmcli.c @@ -1017,7 +1017,7 @@ nmc_cleanup (NmCli *nmc) g_string_free (g_steal_pointer (&nmc->return_text), TRUE); if (nmc->secret_agent) { - nm_secret_agent_old_unregister (nmc->secret_agent, NULL, NULL); + nm_secret_agent_old_unregister (NM_SECRET_AGENT_OLD (nmc->secret_agent), NULL, NULL); g_clear_object (&nmc->secret_agent); } diff --git a/clients/cli/nmcli.h b/clients/cli/nmcli.h index 0ccf1653d8..cd50333a99 100644 --- a/clients/cli/nmcli.h +++ b/clients/cli/nmcli.h @@ -20,7 +20,7 @@ #ifndef NMC_NMCLI_H #define NMC_NMCLI_H -#include "nm-secret-agent-old.h" +#include "nm-secret-agent-simple.h" #include "nm-meta-setting-desc.h" struct _NMPolkitListener; @@ -129,7 +129,7 @@ typedef struct _NmCli { int timeout; /* Operation timeout */ - NMSecretAgentOld *secret_agent; /* Secret agent */ + NMSecretAgentSimple *secret_agent; /* Secret agent */ GHashTable *pwds_hash; /* Hash table with passwords in passwd-file */ struct _NMPolkitListener *pk_listener; /* polkit agent listener */ diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index b4530af61d..034f6052aa 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -1116,7 +1116,7 @@ nm_secret_agent_simple_init (NMSecretAgentSimple *agent) * Returns: a new #NMSecretAgentSimple if the agent creation is successful * or %NULL in case of a failure. */ -NMSecretAgentOld * +NMSecretAgentSimple * nm_secret_agent_simple_new (const char *name) { return g_initable_new (NM_TYPE_SECRET_AGENT_SIMPLE, NULL, NULL, diff --git a/clients/common/nm-secret-agent-simple.h b/clients/common/nm-secret-agent-simple.h index 89e4cf3ccf..8cae17a8c2 100644 --- a/clients/common/nm-secret-agent-simple.h +++ b/clients/common/nm-secret-agent-simple.h @@ -56,7 +56,7 @@ typedef struct _NMSecretAgentSimpleClass NMSecretAgentSimpleClass; GType nm_secret_agent_simple_get_type (void); -NMSecretAgentOld *nm_secret_agent_simple_new (const char *name); +NMSecretAgentSimple *nm_secret_agent_simple_new (const char *name); void nm_secret_agent_simple_response (NMSecretAgentSimple *self, const char *request_id, diff --git a/clients/tui/nmtui-connect.c b/clients/tui/nmtui-connect.c index 6f29e13e9e..e43c22e4fd 100644 --- a/clients/tui/nmtui-connect.c +++ b/clients/tui/nmtui-connect.c @@ -240,7 +240,7 @@ activate_connection (NMConnection *connection, NMObject *specific_object) { NmtNewtForm *form; - gs_unref_object NMSecretAgentOld *agent = NULL; + gs_unref_object NMSecretAgentSimple *agent = NULL; NmtNewtWidget *label; NmtSyncOp op; const char *specific_object_path; @@ -257,7 +257,7 @@ activate_connection (NMConnection *connection, agent = nm_secret_agent_simple_new ("nmtui"); if (agent) { if (connection) { - nm_secret_agent_simple_enable (NM_SECRET_AGENT_SIMPLE (agent), + nm_secret_agent_simple_enable (agent, nm_object_get_path (NM_OBJECT (connection))); } g_signal_connect (agent, @@ -303,7 +303,7 @@ activate_connection (NMConnection *connection, if (agent && !connection) { connection = NM_CONNECTION (nm_active_connection_get_connection (ac)); if (connection) { - nm_secret_agent_simple_enable (NM_SECRET_AGENT_SIMPLE (agent), + nm_secret_agent_simple_enable (agent, nm_object_get_path (NM_OBJECT (connection))); } } @@ -341,7 +341,7 @@ activate_connection (NMConnection *connection, g_object_unref (form); if (agent) - nm_secret_agent_old_unregister (agent, NULL, NULL); + nm_secret_agent_old_unregister (NM_SECRET_AGENT_OLD (agent), NULL, NULL); } static void From c9ca1186c2b6d06f1b03415de96d21e619df7cd6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 13:11:49 +0100 Subject: [PATCH 17/30] clients/secret-agent: add complete function for invoking secret callback The completion of the request and the deletion usually goes hand in hand. Add a function to unify them. --- clients/common/nm-secret-agent-simple.c | 46 ++++++++++++++----------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index 034f6052aa..30545d9c64 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -99,14 +99,26 @@ _request_data_free (gpointer data) } static void -_request_data_cancel (RequestData *request, - GError *error) +_request_data_complete (RequestData *request, + GVariant *secrets, + GError *error, + GHashTableIter *iter_to_remove) { - NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (request->self); + NMSecretAgentSimple *self = request->self; + NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (self); - request->callback (NM_SECRET_AGENT_OLD (request->self), request->connection, - NULL, error, request->callback_data); - g_hash_table_remove (priv->requests, request); + nm_assert ((secrets != NULL) != (error != NULL)); + + request->callback (NM_SECRET_AGENT_OLD (request->self), + request->connection, + secrets, + error, + request->callback_data); + + if (iter_to_remove) + g_hash_table_iter_remove (iter_to_remove); + else + g_hash_table_remove (priv->requests, request); } /*****************************************************************************/ @@ -516,7 +528,7 @@ out: } if (error) - _request_data_cancel (request, error); + _request_data_complete (request, NULL, error, NULL); else { g_signal_emit (request->self, signals[REQUEST_SECRETS], 0, request->request_id, title, message, secrets); @@ -545,7 +557,7 @@ _auth_dialog_read_done (GObject *source_object, switch (read_size) { case -1: if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) - _request_data_cancel (data->request, error); + _request_data_complete (data->request, NULL, error, NULL); _auth_dialog_data_free (data); break; case 0: @@ -733,7 +745,7 @@ request_secrets_from_ui (RequestData *request) error = g_error_new (NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_FAILED, "Request for %s secrets doesn't match path %s", request->request_id, priv->path); - _request_data_cancel (request, error); + _request_data_complete (request, NULL, error, NULL); return; } @@ -868,7 +880,7 @@ request_secrets_from_ui (RequestData *request) "Cannot service a secrets request %s for a %s connection", request->request_id, nm_connection_get_connection_type (request->connection)); - _request_data_cancel (request, error); + _request_data_complete (request, NULL, error, NULL); g_ptr_array_unref (secrets); return; } @@ -950,7 +962,7 @@ nm_secret_agent_simple_response (NMSecretAgentSimple *self, NMSecretAgentSimplePrivate *priv; RequestData *request; GVariant *dict = NULL; - GError *error = NULL; + gs_free_error GError *error = NULL; int i; g_return_if_fail (NM_IS_SECRET_AGENT_SIMPLE (self)); @@ -1013,10 +1025,7 @@ nm_secret_agent_simple_response (NMSecretAgentSimple *self, "User cancelled"); } - request->callback (NM_SECRET_AGENT_OLD (self), request->connection, dict, error, request->callback_data); - - g_clear_error (&error); - g_hash_table_remove (priv->requests, request); + _request_data_complete (request, dict, error, NULL); } static void @@ -1137,12 +1146,7 @@ dispose (GObject *object) while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &request)) { if (!error) nm_utils_error_set_cancelled (&error, TRUE, "NMSecretAgentSimple"); - request->callback (NM_SECRET_AGENT_OLD (object), - request->connection, - NULL, - error, - request->callback_data); - g_hash_table_iter_remove (&iter); + _request_data_complete (request, NULL, error, &iter); } G_OBJECT_CLASS (nm_secret_agent_simple_parent_class)->dispose (object); From 99497a7674928df103c50a83b8f0cf7372fa92c2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 13:24:25 +0100 Subject: [PATCH 18/30] clients/secret-agent: sink reference for variant passed to callback NMSecretAgentOld's get_secrets_cb() gets this right and takes a floating reference. So this was correct. However, make this a bit more robust, and don't pass on floating references. This was, we don't require the callee to consume the reference. --- clients/common/nm-secret-agent-simple.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index 30545d9c64..184f89e274 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -961,7 +961,7 @@ nm_secret_agent_simple_response (NMSecretAgentSimple *self, { NMSecretAgentSimplePrivate *priv; RequestData *request; - GVariant *dict = NULL; + gs_unref_variant GVariant *secrets_dict = NULL; gs_free_error GError *error = NULL; int i; @@ -1018,14 +1018,14 @@ nm_secret_agent_simple_response (NMSecretAgentSimple *self, g_hash_table_iter_init (&iter, settings); while (g_hash_table_iter_next (&iter, (gpointer *) &name, (gpointer *) &setting_builder)) g_variant_builder_add (&conn_builder, "{sa{sv}}", name, setting_builder); - dict = g_variant_builder_end (&conn_builder); + secrets_dict = g_variant_ref_sink (g_variant_builder_end (&conn_builder)); g_hash_table_destroy (settings); } else { error = g_error_new (NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_USER_CANCELED, "User cancelled"); } - _request_data_complete (request, dict, error, NULL); + _request_data_complete (request, secrets_dict, error, NULL); } static void From 99ae86d824ffeddf2840e1781e622010d9a01e60 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 13:41:00 +0100 Subject: [PATCH 19/30] libnm/secret-agent: reorder code --- libnm/nm-secret-agent-old.c | 198 +++++++++++++++++++----------------- 1 file changed, 104 insertions(+), 94 deletions(-) diff --git a/libnm/nm-secret-agent-old.c b/libnm/nm-secret-agent-old.c index a688282f6f..f88cd752a2 100644 --- a/libnm/nm-secret-agent-old.c +++ b/libnm/nm-secret-agent-old.c @@ -20,10 +20,11 @@ #include "nm-default.h" +#include "nm-secret-agent-old.h" + #include #include "nm-dbus-interface.h" -#include "nm-secret-agent-old.h" #include "nm-enum-types.h" #include "nm-dbus-helpers.h" #include "nm-simple-connection.h" @@ -32,14 +33,23 @@ #include "introspection/org.freedesktop.NetworkManager.SecretAgent.h" #include "introspection/org.freedesktop.NetworkManager.AgentManager.h" -static void nm_secret_agent_old_initable_iface_init (GInitableIface *iface); -static void nm_secret_agent_old_async_initable_iface_init (GAsyncInitableIface *iface); -G_DEFINE_ABSTRACT_TYPE_WITH_CODE (NMSecretAgentOld, nm_secret_agent_old, G_TYPE_OBJECT, - G_IMPLEMENT_INTERFACE (G_TYPE_INITABLE, nm_secret_agent_old_initable_iface_init); - G_IMPLEMENT_INTERFACE (G_TYPE_ASYNC_INITABLE, nm_secret_agent_old_async_initable_iface_init); - ) +/*****************************************************************************/ -#define NM_SECRET_AGENT_OLD_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_SECRET_AGENT_OLD, NMSecretAgentOldPrivate)) +typedef struct { + char *path; + char *setting_name; + GDBusMethodInvocation *context; +} GetSecretsInfo; + +enum { + PROP_0, + PROP_IDENTIFIER, + PROP_AUTO_REGISTER, + PROP_REGISTERED, + PROP_CAPABILITIES, + + LAST_PROP +}; typedef struct { gboolean registered; @@ -60,15 +70,15 @@ typedef struct { gboolean suppress_auto; } NMSecretAgentOldPrivate; -enum { - PROP_0, - PROP_IDENTIFIER, - PROP_AUTO_REGISTER, - PROP_REGISTERED, - PROP_CAPABILITIES, +static void nm_secret_agent_old_initable_iface_init (GInitableIface *iface); +static void nm_secret_agent_old_async_initable_iface_init (GAsyncInitableIface *iface); - LAST_PROP -}; +G_DEFINE_ABSTRACT_TYPE_WITH_CODE (NMSecretAgentOld, nm_secret_agent_old, G_TYPE_OBJECT, + G_IMPLEMENT_INTERFACE (G_TYPE_INITABLE, nm_secret_agent_old_initable_iface_init); + G_IMPLEMENT_INTERFACE (G_TYPE_ASYNC_INITABLE, nm_secret_agent_old_async_initable_iface_init); + ) + +#define NM_SECRET_AGENT_OLD_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_SECRET_AGENT_OLD, NMSecretAgentOldPrivate)) /*****************************************************************************/ @@ -85,12 +95,6 @@ _internal_unregister (NMSecretAgentOld *self) } } -typedef struct { - char *path; - char *setting_name; - GDBusMethodInvocation *context; -} GetSecretsInfo; - static void get_secrets_info_finalize (NMSecretAgentOld *self, GetSecretsInfo *info) { @@ -1007,20 +1011,7 @@ validate_identifier (const char *identifier) return TRUE; } -static void -nm_secret_agent_old_init (NMSecretAgentOld *self) -{ - NMSecretAgentOldPrivate *priv = NM_SECRET_AGENT_OLD_GET_PRIVATE (self); - - priv->dbus_secret_agent = nmdbus_secret_agent_skeleton_new (); - _nm_dbus_bind_properties (self, priv->dbus_secret_agent); - _nm_dbus_bind_methods (self, priv->dbus_secret_agent, - "GetSecrets", impl_secret_agent_old_get_secrets, - "CancelGetSecrets", impl_secret_agent_old_cancel_get_secrets, - "DeleteSecrets", impl_secret_agent_old_delete_secrets, - "SaveSecrets", impl_secret_agent_old_save_secrets, - NULL); -} +/*****************************************************************************/ static void init_common (NMSecretAgentOld *self) @@ -1037,34 +1028,6 @@ init_common (NMSecretAgentOld *self) } } -static gboolean -init_sync (GInitable *initable, GCancellable *cancellable, GError **error) -{ - NMSecretAgentOld *self = NM_SECRET_AGENT_OLD (initable); - NMSecretAgentOldPrivate *priv = NM_SECRET_AGENT_OLD_GET_PRIVATE (self); - - priv->bus = _nm_dbus_new_connection (cancellable, error); - if (!priv->bus) - return FALSE; - - priv->manager_proxy = nmdbus_agent_manager_proxy_new_sync (priv->bus, - G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES - | G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START, - NM_DBUS_SERVICE, - NM_DBUS_PATH_AGENT_MANAGER, - cancellable, - error); - if (!priv->manager_proxy) - return FALSE; - - init_common (self); - - if (priv->auto_register) - return nm_secret_agent_old_register (self, cancellable, error); - else - return TRUE; -} - typedef struct { NMSecretAgentOld *self; GCancellable *cancellable; @@ -1141,36 +1104,7 @@ init_async_got_bus (GObject *initable, GAsyncResult *result, gpointer user_data) init_async_got_proxy, init_data); } -static void -init_async (GAsyncInitable *initable, int io_priority, - GCancellable *cancellable, GAsyncReadyCallback callback, - gpointer user_data) -{ - NMSecretAgentOld *self = NM_SECRET_AGENT_OLD (initable); - NMSecretAgentOldInitData *init_data; - - init_data = g_slice_new (NMSecretAgentOldInitData); - init_data->self = self; - init_data->cancellable = cancellable ? g_object_ref (cancellable) : NULL; - - init_data->simple = g_simple_async_result_new (G_OBJECT (initable), callback, - user_data, init_async); - if (cancellable) - g_simple_async_result_set_check_cancellable (init_data->simple, cancellable); - - _nm_dbus_new_connection_async (cancellable, init_async_got_bus, init_data); -} - -static gboolean -init_finish (GAsyncInitable *initable, GAsyncResult *result, GError **error) -{ - GSimpleAsyncResult *simple = G_SIMPLE_ASYNC_RESULT (result); - - if (g_simple_async_result_propagate_error (simple, error)) - return FALSE; - else - return TRUE; -} +/*****************************************************************************/ static void get_property (GObject *object, @@ -1229,6 +1163,82 @@ set_property (GObject *object, } } +/*****************************************************************************/ + +static void +nm_secret_agent_old_init (NMSecretAgentOld *self) +{ + NMSecretAgentOldPrivate *priv = NM_SECRET_AGENT_OLD_GET_PRIVATE (self); + + priv->dbus_secret_agent = nmdbus_secret_agent_skeleton_new (); + _nm_dbus_bind_properties (self, priv->dbus_secret_agent); + _nm_dbus_bind_methods (self, priv->dbus_secret_agent, + "GetSecrets", impl_secret_agent_old_get_secrets, + "CancelGetSecrets", impl_secret_agent_old_cancel_get_secrets, + "DeleteSecrets", impl_secret_agent_old_delete_secrets, + "SaveSecrets", impl_secret_agent_old_save_secrets, + NULL); +} + +static gboolean +init_sync (GInitable *initable, GCancellable *cancellable, GError **error) +{ + NMSecretAgentOld *self = NM_SECRET_AGENT_OLD (initable); + NMSecretAgentOldPrivate *priv = NM_SECRET_AGENT_OLD_GET_PRIVATE (self); + + priv->bus = _nm_dbus_new_connection (cancellable, error); + if (!priv->bus) + return FALSE; + + priv->manager_proxy = nmdbus_agent_manager_proxy_new_sync (priv->bus, + G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES + | G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START, + NM_DBUS_SERVICE, + NM_DBUS_PATH_AGENT_MANAGER, + cancellable, + error); + if (!priv->manager_proxy) + return FALSE; + + init_common (self); + + if (priv->auto_register) + return nm_secret_agent_old_register (self, cancellable, error); + else + return TRUE; +} + +static void +init_async (GAsyncInitable *initable, int io_priority, + GCancellable *cancellable, GAsyncReadyCallback callback, + gpointer user_data) +{ + NMSecretAgentOld *self = NM_SECRET_AGENT_OLD (initable); + NMSecretAgentOldInitData *init_data; + + init_data = g_slice_new (NMSecretAgentOldInitData); + init_data->self = self; + init_data->cancellable = cancellable ? g_object_ref (cancellable) : NULL; + + init_data->simple = g_simple_async_result_new (G_OBJECT (initable), callback, + user_data, init_async); + if (cancellable) + g_simple_async_result_set_check_cancellable (init_data->simple, cancellable); + + _nm_dbus_new_connection_async (cancellable, init_async_got_bus, init_data); +} + +static gboolean +init_finish (GAsyncInitable *initable, GAsyncResult *result, GError **error) +{ + GSimpleAsyncResult *simple = G_SIMPLE_ASYNC_RESULT (result); + + if (g_simple_async_result_propagate_error (simple, error)) + return FALSE; + else + return TRUE; +} + static void dispose (GObject *object) { From fb4a18835060a4f016fbd1c0c18091d93d14b60c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 13:44:01 +0100 Subject: [PATCH 20/30] libnm/secret-agent: refactor GObject properties in NMSecretAgentOld Use NM_GOBJECT_PROPERTIES_DEFINE() and _notify() and get rid of the extra tab for indentation. --- libnm/nm-secret-agent-old.c | 67 +++++++++++++++++-------------------- 1 file changed, 31 insertions(+), 36 deletions(-) diff --git a/libnm/nm-secret-agent-old.c b/libnm/nm-secret-agent-old.c index f88cd752a2..148d7c34cd 100644 --- a/libnm/nm-secret-agent-old.c +++ b/libnm/nm-secret-agent-old.c @@ -41,15 +41,12 @@ typedef struct { GDBusMethodInvocation *context; } GetSecretsInfo; -enum { - PROP_0, +NM_GOBJECT_PROPERTIES_DEFINE (NMSecretAgentOld, PROP_IDENTIFIER, PROP_AUTO_REGISTER, PROP_REGISTERED, PROP_CAPABILITIES, - - LAST_PROP -}; +); typedef struct { gboolean registered; @@ -91,7 +88,7 @@ _internal_unregister (NMSecretAgentOld *self) g_dbus_interface_skeleton_unexport (G_DBUS_INTERFACE_SKELETON (priv->dbus_secret_agent)); priv->registered = FALSE; priv->registering = FALSE; - g_object_notify (G_OBJECT (self), NM_SECRET_AGENT_OLD_REGISTERED); + _notify (self, PROP_REGISTERED); } } @@ -572,7 +569,7 @@ nm_secret_agent_old_register (NMSecretAgentOld *self, success: priv->registering = FALSE; priv->registered = TRUE; - g_object_notify (G_OBJECT (self), NM_SECRET_AGENT_OLD_REGISTERED); + _notify (self, PROP_REGISTERED); return TRUE; } @@ -591,7 +588,7 @@ reg_result (NMSecretAgentOld *self, GSimpleAsyncResult *simple, GError *error) _internal_unregister (self); } else { priv->registered = TRUE; - g_object_notify (G_OBJECT (self), NM_SECRET_AGENT_OLD_REGISTERED); + _notify (self, PROP_REGISTERED); g_simple_async_result_set_op_res_gboolean (simple, TRUE); g_simple_async_result_complete (simple); @@ -1286,13 +1283,12 @@ nm_secret_agent_old_class_init (NMSecretAgentOldClass *class) * of 3 characters. An example valid identifier is 'org.gnome.nm-applet' * (without quotes). **/ - g_object_class_install_property - (object_class, PROP_IDENTIFIER, - g_param_spec_string (NM_SECRET_AGENT_OLD_IDENTIFIER, "", "", - NULL, - G_PARAM_READWRITE | - G_PARAM_CONSTRUCT_ONLY | - G_PARAM_STATIC_STRINGS)); + obj_properties[PROP_IDENTIFIER] = + g_param_spec_string (NM_SECRET_AGENT_OLD_IDENTIFIER, "", "", + NULL, + G_PARAM_READWRITE | + G_PARAM_CONSTRUCT_ONLY | + G_PARAM_STATIC_STRINGS); /** * NMSecretAgentOld:auto-register: @@ -1315,39 +1311,38 @@ nm_secret_agent_old_class_init (NMSecretAgentOldClass *class) * auto-registration. This ensures that the agent remains un-registered when * you expect it to be unregistered. **/ - g_object_class_install_property - (object_class, PROP_AUTO_REGISTER, - g_param_spec_boolean (NM_SECRET_AGENT_OLD_AUTO_REGISTER, "", "", - TRUE, - G_PARAM_READWRITE | - G_PARAM_CONSTRUCT | - G_PARAM_STATIC_STRINGS)); + obj_properties[PROP_AUTO_REGISTER] = + g_param_spec_boolean (NM_SECRET_AGENT_OLD_AUTO_REGISTER, "", "", + TRUE, + G_PARAM_READWRITE | + G_PARAM_CONSTRUCT | + G_PARAM_STATIC_STRINGS); /** * NMSecretAgentOld:registered: * * %TRUE if the agent is registered with NetworkManager, %FALSE if not. **/ - g_object_class_install_property - (object_class, PROP_REGISTERED, - g_param_spec_boolean (NM_SECRET_AGENT_OLD_REGISTERED, "", "", - FALSE, - G_PARAM_READABLE | - G_PARAM_STATIC_STRINGS)); + obj_properties[PROP_REGISTERED] = + g_param_spec_boolean (NM_SECRET_AGENT_OLD_REGISTERED, "", "", + FALSE, + G_PARAM_READABLE | + G_PARAM_STATIC_STRINGS); /** * NMSecretAgentOld:capabilities: * * A bitfield of %NMSecretAgentCapabilities. **/ - g_object_class_install_property - (object_class, PROP_CAPABILITIES, - g_param_spec_flags (NM_SECRET_AGENT_OLD_CAPABILITIES, "", "", - NM_TYPE_SECRET_AGENT_CAPABILITIES, - NM_SECRET_AGENT_CAPABILITY_NONE, - G_PARAM_READWRITE | - G_PARAM_CONSTRUCT | - G_PARAM_STATIC_STRINGS)); + obj_properties[PROP_CAPABILITIES] = + g_param_spec_flags (NM_SECRET_AGENT_OLD_CAPABILITIES, "", "", + NM_TYPE_SECRET_AGENT_CAPABILITIES, + NM_SECRET_AGENT_CAPABILITY_NONE, + G_PARAM_READWRITE | + G_PARAM_CONSTRUCT | + G_PARAM_STATIC_STRINGS); + + g_object_class_install_properties (object_class, _PROPERTY_ENUMS_LAST, obj_properties); } static void From 72f90a8fbcc6394f9a296353310efadda33fa8c9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 14:49:22 +0100 Subject: [PATCH 21/30] clients/secret-agent: fix cancel_get_secrets() implementation The callback must be invoked, as also documented. Otherwise, the tracked info gets leaked. Let NMSecretAgentOld (the caller) be a bit resilient against bugs in the client, and avoid a crash by prematurely remove the request-info from the pending list. That does not fully workaround the bug (it leads to a leak), but at least it does not cause other "severe" issues. The leak was present earlier as well. --- clients/common/nm-secret-agent-simple.c | 15 ++++++++++++++- libnm/nm-secret-agent-old.c | 14 ++++++-------- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index 184f89e274..5a36dffa7a 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -1035,10 +1035,23 @@ cancel_get_secrets (NMSecretAgentOld *agent, { NMSecretAgentSimple *self = NM_SECRET_AGENT_SIMPLE (agent); NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (self); + gs_free_error GError *error = NULL; gs_free char *request_id = NULL; + RequestData *request; request_id = g_strdup_printf ("%s/%s", connection_path, setting_name); - g_hash_table_remove (priv->requests, &request_id); + request = g_hash_table_lookup (priv->requests, &request_id); + if (!request) { + /* this is really a bug of the caller (or us?). We cannot invoke a callback, + * hence the caller cannot cleanup the request. */ + g_return_if_reached (); + } + + g_set_error (&error, + NM_SECRET_AGENT_ERROR, + NM_SECRET_AGENT_ERROR_AGENT_CANCELED, + "The secret agent is going away"); + _request_data_complete (request, NULL, error, NULL); } static void diff --git a/libnm/nm-secret-agent-old.c b/libnm/nm-secret-agent-old.c index 148d7c34cd..de80a46c0c 100644 --- a/libnm/nm-secret-agent-old.c +++ b/libnm/nm-secret-agent-old.c @@ -125,7 +125,6 @@ name_owner_changed (GObject *proxy, { NMSecretAgentOld *self = NM_SECRET_AGENT_OLD (user_data); NMSecretAgentOldPrivate *priv = NM_SECRET_AGENT_OLD_GET_PRIVATE (self); - GSList *iter; char *owner; owner = g_dbus_proxy_get_name_owner (G_DBUS_PROXY (proxy)); @@ -134,16 +133,14 @@ name_owner_changed (GObject *proxy, nm_secret_agent_old_register_async (self, NULL, NULL, NULL); g_free (owner); } else { - /* Cancel any pending secrets requests */ - for (iter = priv->pending_gets; iter; iter = g_slist_next (iter)) { - GetSecretsInfo *info = iter->data; + while (priv->pending_gets) { + GetSecretsInfo *info = priv->pending_gets->data; + priv->pending_gets = g_slist_remove (priv->pending_gets, info); NM_SECRET_AGENT_OLD_GET_CLASS (self)->cancel_get_secrets (self, info->path, info->setting_name); } - g_slist_free (priv->pending_gets); - priv->pending_gets = NULL; _internal_unregister (self); } @@ -307,7 +304,6 @@ get_secrets_cb (NMSecretAgentOld *self, g_variant_new ("(@a{sa{sv}})", secrets)); } - /* Remove the request from internal tracking */ get_secrets_info_finalize (self, info); } @@ -390,10 +386,12 @@ impl_secret_agent_old_cancel_get_secrets (NMSecretAgentOld *self, return; } - /* Send the cancel request up to the subclass and finalize it */ + priv->pending_gets = g_slist_remove (priv->pending_gets, info); + NM_SECRET_AGENT_OLD_GET_CLASS (self)->cancel_get_secrets (self, info->path, info->setting_name); + g_dbus_method_invocation_return_value (context, NULL); } From b9a7f1974bb67dc0edf2b8571c174b68456c6978 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 13:44:01 +0100 Subject: [PATCH 22/30] libnm/secret-agent: use CList instead of GSList for tracking requests Always always when we want a linked list, CList is a better choice than GSList. It's more convenient to use and is more efficient. Also, use GSlice allocator for GetSecretRequest data. --- libnm/nm-secret-agent-old.c | 67 +++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/libnm/nm-secret-agent-old.c b/libnm/nm-secret-agent-old.c index de80a46c0c..7b1501170d 100644 --- a/libnm/nm-secret-agent-old.c +++ b/libnm/nm-secret-agent-old.c @@ -29,6 +29,7 @@ #include "nm-dbus-helpers.h" #include "nm-simple-connection.h" #include "nm-core-internal.h" +#include "c-list/src/c-list.h" #include "introspection/org.freedesktop.NetworkManager.SecretAgent.h" #include "introspection/org.freedesktop.NetworkManager.AgentManager.h" @@ -39,6 +40,7 @@ typedef struct { char *path; char *setting_name; GDBusMethodInvocation *context; + CList gsi_lst; } GetSecretsInfo; NM_GOBJECT_PROPERTIES_DEFINE (NMSecretAgentOld, @@ -60,7 +62,7 @@ typedef struct { NMDBusSecretAgent *dbus_secret_agent; /* GetSecretsInfo structs of in-flight GetSecrets requests */ - GSList *pending_gets; + CList gsi_lst_head; char *identifier; gboolean auto_register; @@ -93,18 +95,15 @@ _internal_unregister (NMSecretAgentOld *self) } static void -get_secrets_info_finalize (NMSecretAgentOld *self, GetSecretsInfo *info) +get_secrets_info_free (GetSecretsInfo *info) { - NMSecretAgentOldPrivate *priv = NM_SECRET_AGENT_OLD_GET_PRIVATE (self); + nm_assert (info); - g_return_if_fail (info != NULL); - - priv->pending_gets = g_slist_remove (priv->pending_gets, info); + c_list_unlink_stale (&info->gsi_lst); g_free (info->path); g_free (info->setting_name); - memset (info, 0, sizeof (*info)); - g_free (info); + g_slice_free (GetSecretsInfo, info); } static inline gboolean @@ -125,18 +124,16 @@ name_owner_changed (GObject *proxy, { NMSecretAgentOld *self = NM_SECRET_AGENT_OLD (user_data); NMSecretAgentOldPrivate *priv = NM_SECRET_AGENT_OLD_GET_PRIVATE (self); - char *owner; + gs_free char *owner = NULL; + GetSecretsInfo *info; owner = g_dbus_proxy_get_name_owner (G_DBUS_PROXY (proxy)); - if (owner != NULL) { + if (owner) { if (should_auto_register (self)) nm_secret_agent_old_register_async (self, NULL, NULL, NULL); - g_free (owner); } else { - while (priv->pending_gets) { - GetSecretsInfo *info = priv->pending_gets->data; - - priv->pending_gets = g_slist_remove (priv->pending_gets, info); + while ((info = c_list_first_entry (&priv->gsi_lst_head, GetSecretsInfo, gsi_lst))) { + c_list_unlink (&info->gsi_lst); NM_SECRET_AGENT_OLD_GET_CLASS (self)->cancel_get_secrets (self, info->path, info->setting_name); @@ -304,7 +301,7 @@ get_secrets_cb (NMSecretAgentOld *self, g_variant_new ("(@a{sa{sv}})", secrets)); } - get_secrets_info_finalize (self, info); + get_secrets_info_free (info); } static void @@ -328,11 +325,13 @@ impl_secret_agent_old_get_secrets (NMSecretAgentOld *self, return; } - info = g_malloc0 (sizeof (GetSecretsInfo)); - info->path = g_strdup (connection_path); - info->setting_name = g_strdup (setting_name); - info->context = context; - priv->pending_gets = g_slist_append (priv->pending_gets, info); + info = g_slice_new (GetSecretsInfo); + *info = (GetSecretsInfo) { + .path = g_strdup (connection_path), + .setting_name = g_strdup (setting_name), + .context = context, + }; + c_list_link_tail (&priv->gsi_lst_head, &info->gsi_lst); NM_SECRET_AGENT_OLD_GET_CLASS (self)->get_secrets (self, connection, @@ -346,16 +345,16 @@ impl_secret_agent_old_get_secrets (NMSecretAgentOld *self, } static GetSecretsInfo * -find_get_secrets_info (GSList *list, const char *path, const char *setting_name) +find_get_secrets_info (NMSecretAgentOldPrivate *priv, + const char *path, + const char *setting_name) { - GSList *iter; + GetSecretsInfo *info; - for (iter = list; iter; iter = g_slist_next (iter)) { - GetSecretsInfo *candidate = iter->data; - - if ( g_strcmp0 (path, candidate->path) == 0 - && g_strcmp0 (setting_name, candidate->setting_name) == 0) - return candidate; + c_list_for_each_entry (info, &priv->gsi_lst_head, gsi_lst) { + if ( nm_streq0 (path, info->path) + && nm_streq0 (setting_name, info->setting_name)) + return info; } return NULL; } @@ -377,7 +376,7 @@ impl_secret_agent_old_cancel_get_secrets (NMSecretAgentOld *self, return; } - info = find_get_secrets_info (priv->pending_gets, connection_path, setting_name); + info = find_get_secrets_info (priv, connection_path, setting_name); if (!info) { g_dbus_method_invocation_return_error (context, NM_SECRET_AGENT_ERROR, @@ -386,7 +385,7 @@ impl_secret_agent_old_cancel_get_secrets (NMSecretAgentOld *self, return; } - priv->pending_gets = g_slist_remove (priv->pending_gets, info); + c_list_unlink (&info->gsi_lst); NM_SECRET_AGENT_OLD_GET_CLASS (self)->cancel_get_secrets (self, info->path, @@ -1165,6 +1164,7 @@ nm_secret_agent_old_init (NMSecretAgentOld *self) { NMSecretAgentOldPrivate *priv = NM_SECRET_AGENT_OLD_GET_PRIVATE (self); + c_list_init (&priv->gsi_lst_head); priv->dbus_secret_agent = nmdbus_secret_agent_skeleton_new (); _nm_dbus_bind_properties (self, priv->dbus_secret_agent); _nm_dbus_bind_methods (self, priv->dbus_secret_agent, @@ -1239,14 +1239,15 @@ dispose (GObject *object) { NMSecretAgentOld *self = NM_SECRET_AGENT_OLD (object); NMSecretAgentOldPrivate *priv = NM_SECRET_AGENT_OLD_GET_PRIVATE (self); + GetSecretsInfo *info; if (priv->registered) nm_secret_agent_old_unregister_async (self, NULL, NULL, NULL); g_clear_pointer (&priv->identifier, g_free); - while (priv->pending_gets) - get_secrets_info_finalize (self, priv->pending_gets->data); + while ((info = c_list_first_entry (&priv->gsi_lst_head, GetSecretsInfo, gsi_lst))) + get_secrets_info_free (info); g_signal_handlers_disconnect_matched (priv->dbus_secret_agent, G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, self); From ee9e9808142ea3ccc3c21ab8d0973e66bc31a57f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 13:44:01 +0100 Subject: [PATCH 23/30] libnm/secret-agent: reorder fields in NMSecretAgentOldPrivate And don't waste 4 bytes per boolean. --- libnm/nm-secret-agent-old.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/libnm/nm-secret-agent-old.c b/libnm/nm-secret-agent-old.c index 7b1501170d..d67d570607 100644 --- a/libnm/nm-secret-agent-old.c +++ b/libnm/nm-secret-agent-old.c @@ -51,13 +51,7 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMSecretAgentOld, ); typedef struct { - gboolean registered; - gboolean registering; - NMSecretAgentCapabilities capabilities; - GDBusConnection *bus; - gboolean private_bus; - gboolean session_bus; NMDBusAgentManager *manager_proxy; NMDBusSecretAgent *dbus_secret_agent; @@ -65,8 +59,15 @@ typedef struct { CList gsi_lst_head; char *identifier; - gboolean auto_register; - gboolean suppress_auto; + + NMSecretAgentCapabilities capabilities; + + bool registered:1; + bool registering:1; + bool private_bus:1; + bool session_bus:1; + bool auto_register:1; + bool suppress_auto:1; } NMSecretAgentOldPrivate; static void nm_secret_agent_old_initable_iface_init (GInitableIface *iface); From 43b3e19c885a640f93ffd6533e5e0dd82d0201b4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 13:44:01 +0100 Subject: [PATCH 24/30] libnm/secret-agent: use cleanup attribute in NMSecretAgentOldPrivate Refactor memory handling to use cleanup attribute. --- libnm/nm-secret-agent-old.c | 105 +++++++++++++++++------------------- 1 file changed, 49 insertions(+), 56 deletions(-) diff --git a/libnm/nm-secret-agent-old.c b/libnm/nm-secret-agent-old.c index d67d570607..9fc185de19 100644 --- a/libnm/nm-secret-agent-old.c +++ b/libnm/nm-secret-agent-old.c @@ -150,11 +150,11 @@ verify_sender (NMSecretAgentOld *self, GError **error) { NMSecretAgentOldPrivate *priv = NM_SECRET_AGENT_OLD_GET_PRIVATE (self); - char *nm_owner; + gs_free char *owner = NULL; const char *sender; guint32 sender_uid; - GVariant *ret; - GError *local = NULL; + gs_unref_variant GVariant *ret = NULL; + gs_free_error GError *local = NULL; g_return_val_if_fail (context != NULL, FALSE); @@ -166,8 +166,8 @@ verify_sender (NMSecretAgentOld *self, /* Verify that the sender is the same as NetworkManager's bus name owner. */ - nm_owner = g_dbus_proxy_get_name_owner (G_DBUS_PROXY (priv->manager_proxy)); - if (!nm_owner) { + owner = g_dbus_proxy_get_name_owner (G_DBUS_PROXY (priv->manager_proxy)); + if (!owner) { g_set_error_literal (error, NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_PERMISSION_DENIED, @@ -181,20 +181,16 @@ verify_sender (NMSecretAgentOld *self, NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_PERMISSION_DENIED, "Failed to get request sender."); - g_free (nm_owner); return FALSE; } - /* Check that the sender matches the current NM bus name owner */ - if (strcmp (sender, nm_owner) != 0) { + if (!nm_streq (sender, owner)) { g_set_error_literal (error, NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_PERMISSION_DENIED, "Request sender does not match NetworkManager bus name owner."); - g_free (nm_owner); return FALSE; } - g_free (nm_owner); /* If we're connected to the session bus, then this must be a test program, * so skip the UID check. @@ -213,8 +209,9 @@ verify_sender (NMSecretAgentOld *self, G_DBUS_CALL_FLAGS_NONE, -1, NULL, &local); if (!ret) { - char *remote_error = g_dbus_error_get_remote_error (local); + gs_free char *remote_error = NULL; + remote_error = g_dbus_error_get_remote_error (local); g_dbus_error_strip_remote_error (local); g_set_error (error, NM_SECRET_AGENT_ERROR, @@ -222,15 +219,12 @@ verify_sender (NMSecretAgentOld *self, "Failed to request unix user: (%s) %s.", remote_error ?: "", local->message); - g_free (remote_error); - g_error_free (local); return FALSE; } g_variant_get (ret, "(u)", &sender_uid); - g_variant_unref (ret); /* We only accept requests from NM, which always runs as root */ - if (0 != sender_uid) { + if (sender_uid != 0) { g_set_error_literal (error, NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_PERMISSION_DENIED, @@ -249,8 +243,8 @@ verify_request (NMSecretAgentOld *self, NMConnection **out_connection, GError **error) { - NMConnection *connection = NULL; - GError *local = NULL; + gs_unref_object NMConnection *connection = NULL; + gs_free_error GError *local = NULL; if (!verify_sender (self, context, error)) return FALSE; @@ -269,20 +263,18 @@ verify_request (NMSecretAgentOld *self, } /* Make sure the given connection is valid */ - g_assert (out_connection); connection = _nm_simple_connection_new_from_dbus (connection_dict, NM_SETTING_PARSE_FLAGS_BEST_EFFORT, &local); - if (connection) { - nm_connection_set_path (connection, connection_path); - *out_connection = connection; - } else { + if (!connection) { g_set_error (error, NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_INVALID_CONNECTION, "Invalid connection: %s", local->message); - g_clear_error (&local); + return FALSE; } - return !!connection; + nm_connection_set_path (connection, connection_path); + NM_SET_OUT (out_connection, g_steal_pointer (&connection)); + return TRUE; } static void @@ -317,7 +309,7 @@ impl_secret_agent_old_get_secrets (NMSecretAgentOld *self, { NMSecretAgentOldPrivate *priv = NM_SECRET_AGENT_OLD_GET_PRIVATE (self); GError *error = NULL; - NMConnection *connection = NULL; + gs_unref_object NMConnection *connection = NULL; GetSecretsInfo *info; /* Make sure the request comes from NetworkManager and is valid */ @@ -342,7 +334,6 @@ impl_secret_agent_old_get_secrets (NMSecretAgentOld *self, flags, get_secrets_cb, info); - g_object_unref (connection); } static GetSecretsInfo * @@ -416,8 +407,8 @@ impl_secret_agent_old_save_secrets (NMSecretAgentOld *self, const char *connection_path, gpointer user_data) { + gs_unref_object NMConnection *connection = NULL; GError *error = NULL; - NMConnection *connection = NULL; /* Make sure the request comes from NetworkManager and is valid */ if (!verify_request (self, context, connection_dict, connection_path, &connection, &error)) { @@ -430,7 +421,6 @@ impl_secret_agent_old_save_secrets (NMSecretAgentOld *self, connection_path, save_secrets_cb, context); - g_object_unref (connection); } static void @@ -454,8 +444,8 @@ impl_secret_agent_old_delete_secrets (NMSecretAgentOld *self, const char *connection_path, gpointer user_data) { + gs_unref_object NMConnection *connection = NULL; GError *error = NULL; - NMConnection *connection = NULL; /* Make sure the request comes from NetworkManager and is valid */ if (!verify_request (self, context, connection_dict, connection_path, &connection, &error)) { @@ -468,7 +458,6 @@ impl_secret_agent_old_delete_secrets (NMSecretAgentOld *self, connection_path, delete_secrets_cb, context); - g_object_unref (connection); } /*****************************************************************************/ @@ -477,15 +466,13 @@ static gboolean check_nm_running (NMSecretAgentOld *self, GError **error) { NMSecretAgentOldPrivate *priv = NM_SECRET_AGENT_OLD_GET_PRIVATE (self); - char *owner; + gs_free char *owner = NULL; if (priv->private_bus) return TRUE; owner = g_dbus_proxy_get_name_owner (G_DBUS_PROXY (priv->manager_proxy)); - if (owner) { - g_free (owner); + if (owner) return TRUE; - } g_set_error (error, NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_FAILED, "NetworkManager is not running"); @@ -575,6 +562,7 @@ static void reg_result (NMSecretAgentOld *self, GSimpleAsyncResult *simple, GError *error) { NMSecretAgentOldPrivate *priv = NM_SECRET_AGENT_OLD_GET_PRIVATE (self); + _nm_unused gs_unref_object GSimpleAsyncResult *simple_free = simple; priv->registering = FALSE; @@ -591,8 +579,6 @@ reg_result (NMSecretAgentOld *self, GSimpleAsyncResult *simple, GError *error) g_simple_async_result_set_op_res_gboolean (simple, TRUE); g_simple_async_result_complete (simple); } - - g_object_unref (simple); } static void @@ -623,6 +609,7 @@ reg_with_caps_cb (GObject *proxy, self = NM_SECRET_AGENT_OLD (g_async_result_get_source_object (G_ASYNC_RESULT (simple))); g_object_unref (self); /* drop extra ref added by get_source_object() */ + priv = NM_SECRET_AGENT_OLD_GET_PRIVATE (self); if (nmdbus_agent_manager_call_register_with_capabilities_finish (NMDBUS_AGENT_MANAGER (proxy), result, NULL)) { @@ -660,7 +647,7 @@ nm_secret_agent_old_register_async (NMSecretAgentOld *self, { NMSecretAgentOldPrivate *priv; NMSecretAgentOldClass *class; - GSimpleAsyncResult *simple; + gs_unref_object GSimpleAsyncResult *simple = NULL; GError *error = NULL; g_return_if_fail (NM_IS_SECRET_AGENT_OLD (self)); @@ -686,7 +673,6 @@ nm_secret_agent_old_register_async (NMSecretAgentOld *self, if (!check_nm_running (self, &error)) { g_simple_async_result_take_error (simple, error); g_simple_async_result_complete_in_idle (simple); - g_object_unref (simple); return; } @@ -697,7 +683,6 @@ nm_secret_agent_old_register_async (NMSecretAgentOld *self, &error)) { g_simple_async_result_take_error (simple, error); g_simple_async_result_complete_in_idle (simple); - g_object_unref (simple); return; } @@ -708,7 +693,8 @@ nm_secret_agent_old_register_async (NMSecretAgentOld *self, priv->identifier, priv->capabilities, NULL, - reg_with_caps_cb, simple); + reg_with_caps_cb, + g_steal_pointer (&simple)); } /** @@ -774,7 +760,7 @@ nm_secret_agent_old_unregister (NMSecretAgentOld *self, static void unregister_cb (GObject *proxy, GAsyncResult *result, gpointer user_data) { - GSimpleAsyncResult *simple = user_data; + gs_unref_object GSimpleAsyncResult *simple = user_data; NMSecretAgentOld *self; GError *error = NULL; @@ -792,7 +778,6 @@ unregister_cb (GObject *proxy, GAsyncResult *result, gpointer user_data) } g_simple_async_result_complete (simple); - g_object_unref (simple); } /** @@ -813,7 +798,7 @@ nm_secret_agent_old_unregister_async (NMSecretAgentOld *self, gpointer user_data) { NMSecretAgentOldPrivate *priv; - GSimpleAsyncResult *simple; + gs_unref_object GSimpleAsyncResult *simple = NULL; GError *error = NULL; g_return_if_fail (NM_IS_SECRET_AGENT_OLD (self)); @@ -831,14 +816,15 @@ nm_secret_agent_old_unregister_async (NMSecretAgentOld *self, if (!check_nm_running (self, &error)) { g_simple_async_result_take_error (simple, error); g_simple_async_result_complete_in_idle (simple); - g_object_unref (simple); return; } priv->suppress_auto = TRUE; - nmdbus_agent_manager_call_unregister (priv->manager_proxy, cancellable, - unregister_cb, simple); + nmdbus_agent_manager_call_unregister (priv->manager_proxy, + cancellable, + unregister_cb, + g_steal_pointer (&simple)); } /** @@ -1213,11 +1199,15 @@ init_async (GAsyncInitable *initable, int io_priority, NMSecretAgentOldInitData *init_data; init_data = g_slice_new (NMSecretAgentOldInitData); - init_data->self = self; - init_data->cancellable = cancellable ? g_object_ref (cancellable) : NULL; + *init_data = (NMSecretAgentOldInitData) { + .self = self, + .cancellable = nm_g_object_ref (cancellable), + .simple = g_simple_async_result_new (G_OBJECT (initable), + callback, + user_data, + init_async), + }; - init_data->simple = g_simple_async_result_new (G_OBJECT (initable), callback, - user_data, init_async); if (cancellable) g_simple_async_result_set_check_cancellable (init_data->simple, cancellable); @@ -1242,17 +1232,21 @@ dispose (GObject *object) NMSecretAgentOldPrivate *priv = NM_SECRET_AGENT_OLD_GET_PRIVATE (self); GetSecretsInfo *info; - if (priv->registered) + if (priv->registered) { + priv->registered = FALSE; nm_secret_agent_old_unregister_async (self, NULL, NULL, NULL); + } - g_clear_pointer (&priv->identifier, g_free); + nm_clear_g_free (&priv->identifier); while ((info = c_list_first_entry (&priv->gsi_lst_head, GetSecretsInfo, gsi_lst))) get_secrets_info_free (info); - g_signal_handlers_disconnect_matched (priv->dbus_secret_agent, G_SIGNAL_MATCH_DATA, - 0, 0, NULL, NULL, self); - g_object_unref (priv->dbus_secret_agent); + if (priv->dbus_secret_agent) { + g_signal_handlers_disconnect_matched (priv->dbus_secret_agent, G_SIGNAL_MATCH_DATA, + 0, 0, NULL, NULL, self); + g_clear_object (&priv->dbus_secret_agent); + } g_clear_object (&priv->manager_proxy); g_clear_object (&priv->bus); @@ -1267,7 +1261,6 @@ nm_secret_agent_old_class_init (NMSecretAgentOldClass *class) g_type_class_add_private (class, sizeof (NMSecretAgentOldPrivate)); - /* Virtual methods */ object_class->dispose = dispose; object_class->get_property = get_property; object_class->set_property = set_property; From d52fd81b919917d43504385dcfcb04d8c15049ec Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 13:44:01 +0100 Subject: [PATCH 25/30] libnm/secret-agent/trivial: rename internal init-data structure No "NM" prefix for internal structure. --- libnm/nm-secret-agent-old.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/libnm/nm-secret-agent-old.c b/libnm/nm-secret-agent-old.c index 9fc185de19..2f2482d99d 100644 --- a/libnm/nm-secret-agent-old.c +++ b/libnm/nm-secret-agent-old.c @@ -1013,10 +1013,10 @@ typedef struct { NMSecretAgentOld *self; GCancellable *cancellable; GSimpleAsyncResult *simple; -} NMSecretAgentOldInitData; +} InitData; static void -init_async_complete (NMSecretAgentOldInitData *init_data, GError *error) +init_async_complete (InitData *init_data, GError *error) { if (!error) g_simple_async_result_set_op_res_gboolean (init_data->simple, TRUE); @@ -1027,14 +1027,14 @@ init_async_complete (NMSecretAgentOldInitData *init_data, GError *error) g_object_unref (init_data->simple); g_clear_object (&init_data->cancellable); - g_slice_free (NMSecretAgentOldInitData, init_data); + g_slice_free (InitData, init_data); } static void init_async_registered (GObject *object, GAsyncResult *result, gpointer user_data) { NMSecretAgentOld *self = NM_SECRET_AGENT_OLD (object); - NMSecretAgentOldInitData *init_data = user_data; + InitData *init_data = user_data; GError *error = NULL; nm_secret_agent_old_register_finish (self, result, &error); @@ -1044,7 +1044,7 @@ init_async_registered (GObject *object, GAsyncResult *result, gpointer user_data static void init_async_got_proxy (GObject *object, GAsyncResult *result, gpointer user_data) { - NMSecretAgentOldInitData *init_data = user_data; + InitData *init_data = user_data; NMSecretAgentOldPrivate *priv = NM_SECRET_AGENT_OLD_GET_PRIVATE (init_data->self); GError *error = NULL; @@ -1066,7 +1066,7 @@ init_async_got_proxy (GObject *object, GAsyncResult *result, gpointer user_data) static void init_async_got_bus (GObject *initable, GAsyncResult *result, gpointer user_data) { - NMSecretAgentOldInitData *init_data = user_data; + InitData *init_data = user_data; NMSecretAgentOldPrivate *priv = NM_SECRET_AGENT_OLD_GET_PRIVATE (init_data->self); GError *error = NULL; @@ -1196,10 +1196,10 @@ init_async (GAsyncInitable *initable, int io_priority, gpointer user_data) { NMSecretAgentOld *self = NM_SECRET_AGENT_OLD (initable); - NMSecretAgentOldInitData *init_data; + InitData *init_data; - init_data = g_slice_new (NMSecretAgentOldInitData); - *init_data = (NMSecretAgentOldInitData) { + init_data = g_slice_new (InitData); + *init_data = (InitData) { .self = self, .cancellable = nm_g_object_ref (cancellable), .simple = g_simple_async_result_new (G_OBJECT (initable), From 883978ec99d18b35d85dc6964da1bf74692c55a5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 23:17:50 +0100 Subject: [PATCH 26/30] clients/secret-agent: use g_hash_table_get_keys_as_array() in nm_secret_agent_simple_enable() --- clients/common/nm-secret-agent-simple.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index 5a36dffa7a..eab11841a6 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -1090,7 +1090,8 @@ void nm_secret_agent_simple_enable (NMSecretAgentSimple *self, const char *path) { NMSecretAgentSimplePrivate *priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (self); - GList *requests, *iter; + gs_free RequestData **requests = NULL; + gsize i; gs_free char *path_full = NULL; /* The path is only used to match a request_id with the current @@ -1109,11 +1110,9 @@ nm_secret_agent_simple_enable (NMSecretAgentSimple *self, const char *path) priv->enabled = TRUE; /* Service pending secret requests. */ - requests = g_hash_table_get_values (priv->requests); - for (iter = requests; iter; iter = g_list_next (iter)) - request_secrets_from_ui (iter->data); - - g_list_free (requests); + requests = (RequestData **) g_hash_table_get_keys_as_array (priv->requests, NULL); + for (i = 0; requests[i]; i++) + request_secrets_from_ui (requests[i]); } /*****************************************************************************/ From 1a0fc8d437b04641b41c86cd1e9a35c48b5b2c67 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 23:34:56 +0100 Subject: [PATCH 27/30] clients/secret-agent: fix leaks in request_secrets_from_ui() Fixes: 3bda3fb60c104114192e4e3c9c4ba0bef84d3a00 --- clients/common/nm-secret-agent-simple.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index eab11841a6..cd2b71cfd3 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -728,11 +728,11 @@ try_spawn_vpn_auth_helper (RequestData *request, static void request_secrets_from_ui (RequestData *request) { - GPtrArray *secrets; + gs_unref_ptrarray GPtrArray *secrets = NULL; NMSecretAgentSimplePrivate *priv; NMSecretAgentSimpleSecret *secret; const char *title; - char *msg; + gs_free char *msg = NULL; gboolean ok = TRUE; priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (request->self); @@ -881,7 +881,6 @@ request_secrets_from_ui (RequestData *request) request->request_id, nm_connection_get_connection_type (request->connection)); _request_data_complete (request, NULL, error, NULL); - g_ptr_array_unref (secrets); return; } From 787f5f7a4645e4dd5331c56b44ec58e66c2a1ba1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 23 Jan 2019 13:54:38 +0100 Subject: [PATCH 28/30] clients/secret-agent: refactor code in request_secrets_from_ui() to return early --- clients/common/nm-secret-agent-simple.c | 79 +++++++++++++------------ 1 file changed, 40 insertions(+), 39 deletions(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index cd2b71cfd3..3eee3fe263 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -729,24 +729,21 @@ static void request_secrets_from_ui (RequestData *request) { gs_unref_ptrarray GPtrArray *secrets = NULL; + gs_free_error GError *error = NULL; NMSecretAgentSimplePrivate *priv; NMSecretAgentSimpleSecret *secret; const char *title; gs_free char *msg = NULL; - gboolean ok = TRUE; priv = NM_SECRET_AGENT_SIMPLE_GET_PRIVATE (request->self); g_return_if_fail (priv->enabled); /* We only handle requests for connection with @path if set. */ if (priv->path && !g_str_has_prefix (request->request_id, priv->path)) { - gs_free_error GError *error = NULL; - - error = g_error_new (NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_FAILED, - "Request for %s secrets doesn't match path %s", - request->request_id, priv->path); - _request_data_complete (request, NULL, error, NULL); - return; + g_set_error (&error, NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_FAILED, + "Request for %s secrets doesn't match path %s", + request->request_id, priv->path); + goto out_fail_error; } secrets = g_ptr_array_new_with_free_func ((GDestroyNotify) _secret_real_free); @@ -764,19 +761,22 @@ request_secrets_from_ui (RequestData *request) title = _("Authentication required by wireless network"); msg = g_strdup_printf (_("Passwords or encryption keys are required to access the wireless network '%s'."), ssid_utf8); - ok = add_wireless_secrets (request, secrets); + if (!add_wireless_secrets (request, secrets)) + goto out_fail; } else if (nm_connection_is_type (request->connection, NM_SETTING_WIRED_SETTING_NAME)) { title = _("Wired 802.1X authentication"); msg = g_strdup_printf (_("Secrets are required to access the wired network '%s'"), nm_connection_get_id (request->connection)); - ok = add_8021x_secrets (request, secrets); + if (!add_8021x_secrets (request, secrets)) + goto out_fail; } else if (nm_connection_is_type (request->connection, NM_SETTING_PPPOE_SETTING_NAME)) { title = _("DSL authentication"); msg = g_strdup_printf (_("Secrets are required for the DSL connection '%s'"), nm_connection_get_id (request->connection)); - ok = add_pppoe_secrets (request, secrets); + if (!add_pppoe_secrets (request, secrets)) + goto out_fail; } else if (nm_connection_is_type (request->connection, NM_SETTING_GSM_SETTING_NAME)) { NMSettingGsm *s_gsm = nm_connection_get_setting_gsm (request->connection); @@ -818,7 +818,8 @@ request_secrets_from_ui (RequestData *request) g_ptr_array_add (secrets, secret); } else { title = _("MACsec EAP authentication"); - ok = add_8021x_secrets (request, secrets); + if (!add_8021x_secrets (request, secrets)) + goto out_fail; } } else if (nm_connection_is_type (request->connection, NM_SETTING_CDMA_SETTING_NAME)) { NMSettingCdma *s_cdma = nm_connection_get_setting_cdma (request->connection); @@ -844,48 +845,48 @@ request_secrets_from_ui (RequestData *request) setting = nm_connection_get_setting_by_name (request->connection, NM_SETTING_CDMA_SETTING_NAME); } - if (setting) { - title = _("Mobile broadband network password"); - msg = g_strdup_printf (_("A password is required to connect to '%s'."), - nm_connection_get_id (request->connection)); + if (!setting) + goto out_fail; - secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, - _("Password"), - setting, - "password", - NULL); - g_ptr_array_add (secrets, secret); - } else - ok = FALSE; + title = _("Mobile broadband network password"); + msg = g_strdup_printf (_("A password is required to connect to '%s'."), + nm_connection_get_id (request->connection)); + + secret = _secret_real_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET, + _("Password"), + setting, + "password", + NULL); + g_ptr_array_add (secrets, secret); } else if (nm_connection_is_type (request->connection, NM_SETTING_VPN_SETTING_NAME)) { title = _("VPN password required"); - msg = NULL; if (try_spawn_vpn_auth_helper (request, secrets)) { /* This will emit REQUEST_SECRETS when ready */ return; } - ok = add_vpn_secrets (request, secrets, &msg); - if (!msg) + if (!add_vpn_secrets (request, secrets, &msg)) + goto out_fail; + if (!msg) { msg = g_strdup_printf (_("A password is required to connect to '%s'."), nm_connection_get_id (request->connection)); + } } else - ok = FALSE; - - if (!ok) { - gs_free_error GError *error = NULL; - - error = g_error_new (NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_FAILED, - "Cannot service a secrets request %s for a %s connection", - request->request_id, - nm_connection_get_connection_type (request->connection)); - _request_data_complete (request, NULL, error, NULL); - return; - } + goto out_fail; g_signal_emit (request->self, signals[REQUEST_SECRETS], 0, request->request_id, title, msg, secrets); + return; + +out_fail: + g_set_error (&error, NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_FAILED, + "Cannot service a secrets request %s for a %s connection", + request->request_id, + nm_connection_get_connection_type (request->connection)); +out_fail_error: + _request_data_complete (request, NULL, error, NULL); + } static void From bd590579dabd1940049822026357f45b4ab4d4af Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 24 Jan 2019 16:18:56 +0100 Subject: [PATCH 29/30] core: pass hints as strv to nm_act_request_get_secrets() Extend nm_act_request_get_secrets() API to allow for the underlying flexibility (of the API that it calls) to accept a strv list of hints. --- src/devices/wifi/nm-device-iwd.c | 4 ++-- src/devices/wwan/nm-modem.c | 10 ++++++---- src/nm-act-request.c | 3 +-- src/nm-act-request.h | 2 +- src/ppp/nm-ppp-manager.c | 10 +++++----- 5 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index 71b8d15153..88422eb150 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -1380,7 +1380,7 @@ wifi_secrets_get_one (NMDeviceIwd *self, TRUE, setting_name, flags, - setting_key, + NM_MAKE_STRV (setting_key), wifi_secrets_cb, nm_utils_user_data_pack (self, invocation)); } @@ -1894,7 +1894,7 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) TRUE, NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, NM_SECRET_AGENT_GET_SECRETS_FLAG_ALLOW_INTERACTION, - "psk", + NM_MAKE_STRV ("psk"), act_psk_cb, self); nm_device_state_changed (device, NM_DEVICE_STATE_NEED_AUTH, NM_DEVICE_STATE_REASON_NONE); diff --git a/src/devices/wwan/nm-modem.c b/src/devices/wwan/nm-modem.c index 11ada549da..1ad12b9d8e 100644 --- a/src/devices/wwan/nm-modem.c +++ b/src/devices/wwan/nm-modem.c @@ -948,7 +948,7 @@ nm_modem_get_secrets (NMModem *self, FALSE, setting_name, flags, - hint, + NM_MAKE_STRV (hint), modem_secrets_cb, self); g_return_if_fail (priv->secrets_id); @@ -986,8 +986,7 @@ nm_modem_act_stage1_prepare (NMModem *self, setting_name = nm_connection_need_secrets (connection, &hints); if (!setting_name) { - /* Ready to connect */ - g_assert (!hints); + nm_assert (!hints); return NM_MODEM_GET_CLASS (self)->act_stage1_prepare (self, connection, out_failure_reason); } @@ -995,11 +994,14 @@ nm_modem_act_stage1_prepare (NMModem *self, if (priv->secrets_tries++) flags |= NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW; + if (hints) + g_ptr_array_add (hints, NULL); + priv->secrets_id = nm_act_request_get_secrets (req, FALSE, setting_name, flags, - hints ? g_ptr_array_index (hints, 0) : NULL, + hints ? (const char *const*) hints->pdata : NULL, modem_secrets_cb, self); g_return_val_if_fail (priv->secrets_id, NM_ACT_STAGE_RETURN_FAILURE); diff --git a/src/nm-act-request.c b/src/nm-act-request.c index cd81696490..584eceab12 100644 --- a/src/nm-act-request.c +++ b/src/nm-act-request.c @@ -166,7 +166,7 @@ nm_act_request_get_secrets (NMActRequest *self, gboolean ref_self, const char *setting_name, NMSecretAgentGetSecretsFlags flags, - const char *hint, + const char *const*hints, NMActRequestSecretsFunc callback, gpointer callback_data) { @@ -175,7 +175,6 @@ nm_act_request_get_secrets (NMActRequest *self, NMSettingsConnectionCallId *call_id_s; NMSettingsConnection *settings_connection; NMConnection *applied_connection; - const char *hints[2] = { hint, NULL }; g_return_val_if_fail (NM_IS_ACT_REQUEST (self), NULL); diff --git a/src/nm-act-request.h b/src/nm-act-request.h index af2b749055..e16e1ecfdc 100644 --- a/src/nm-act-request.h +++ b/src/nm-act-request.h @@ -69,7 +69,7 @@ NMActRequestGetSecretsCallId *nm_act_request_get_secrets (NMActRequest *req, gboolean take_ref, const char *setting_name, NMSecretAgentGetSecretsFlags flags, - const char *hint, + const char *const*hints, NMActRequestSecretsFunc callback, gpointer callback_data); diff --git a/src/ppp/nm-ppp-manager.c b/src/ppp/nm-ppp-manager.c index 122901fcd8..548f46606f 100644 --- a/src/ppp/nm-ppp-manager.c +++ b/src/ppp/nm-ppp-manager.c @@ -363,7 +363,7 @@ impl_ppp_manager_need_secrets (NMDBusObject *obj, const char *username = NULL; const char *password = NULL; guint32 tries; - GPtrArray *hints = NULL; + gs_unref_ptrarray GPtrArray *hints = NULL; GError *error = NULL; NMSecretAgentGetSecretsFlags flags = NM_SECRET_AGENT_GET_SECRETS_FLAG_ALLOW_INTERACTION; @@ -393,18 +393,18 @@ impl_ppp_manager_need_secrets (NMDBusObject *obj, if (tries > 1) flags |= NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW; + if (hints) + g_ptr_array_add (hints, NULL); + priv->secrets_id = nm_act_request_get_secrets (priv->act_req, FALSE, priv->secrets_setting_name, flags, - hints ? g_ptr_array_index (hints, 0) : NULL, + hints ? (const char *const*) hints->pdata : NULL, ppp_secrets_cb, self); g_object_set_qdata (G_OBJECT (applied_connection), ppp_manager_secret_tries_quark (), GUINT_TO_POINTER (++tries)); priv->pending_secrets_context = invocation; - - if (hints) - g_ptr_array_free (hints, TRUE); } static void From 472f89da6b5dbd88ffd0815966dfaeb98dd8e024 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 22 Jan 2019 23:17:50 +0100 Subject: [PATCH 30/30] wifi,clients/secret-agent: use defines for property names in secret hints --- clients/common/nm-secret-agent-simple.c | 2 +- src/devices/wifi/nm-device-iwd.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c index 3eee3fe263..f649316fe3 100644 --- a/clients/common/nm-secret-agent-simple.c +++ b/clients/common/nm-secret-agent-simple.c @@ -780,7 +780,7 @@ request_secrets_from_ui (RequestData *request) } else if (nm_connection_is_type (request->connection, NM_SETTING_GSM_SETTING_NAME)) { NMSettingGsm *s_gsm = nm_connection_get_setting_gsm (request->connection); - if (g_strv_contains (NM_CAST_STRV_CC (request->hints), "pin")) { + if (g_strv_contains (NM_CAST_STRV_CC (request->hints), NM_SETTING_GSM_PIN)) { title = _("PIN code required"); msg = g_strdup (_("PIN code is needed for the mobile broadband device")); diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index 88422eb150..5fcf6dff23 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -1894,7 +1894,7 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) TRUE, NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, NM_SECRET_AGENT_GET_SECRETS_FLAG_ALLOW_INTERACTION, - NM_MAKE_STRV ("psk"), + NM_MAKE_STRV (NM_SETTING_WIRELESS_SECURITY_PSK), act_psk_cb, self); nm_device_state_changed (device, NM_DEVICE_STATE_NEED_AUTH, NM_DEVICE_STATE_REASON_NONE);