From 9e3d3083d22f2a52a2cad2165b778651d4a4b7db Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 31 Aug 2015 16:37:55 +0200 Subject: auth-utils: some refactoring in nm-auth-utils.c - move nm_auth_chain_check_done() and nm_auth_chain_remove_call() into the only caller auth_call_complete(). - take a ref of the "context" argument. - in nm_auth_chain_add_call(), assert that we didn't yet invoke the done-callback. The auth-chain should not be reusued. - use slice allocator for ChainData, AuthCall and NMAuthChain --- src/nm-auth-utils.c | 124 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 74 insertions(+), 50 deletions(-) diff --git a/src/nm-auth-utils.c b/src/nm-auth-utils.c index 1ed01b6c8d..8c1c4dac4a 100644 --- a/src/nm-auth-utils.c +++ b/src/nm-auth-utils.c @@ -39,6 +39,7 @@ struct NMAuthChain { GError *error; guint idle_id; + gboolean done; NMAuthChainResultFunc done_func; gpointer user_data; @@ -56,15 +57,26 @@ typedef struct { GDestroyNotify destroy; } ChainData; +static ChainData * +chain_data_new (gpointer data, GDestroyNotify destroy) +{ + ChainData *tmp; + + tmp = g_slice_new (ChainData); + tmp->data = data; + tmp->destroy = destroy; + return tmp; +} + static void -free_data (gpointer data) +chain_data_free (gpointer data) { ChainData *tmp = data; if (tmp->destroy) tmp->destroy (tmp->data); memset (tmp, 0, sizeof (ChainData)); - g_free (tmp); + g_slice_free (ChainData, tmp); } static gboolean @@ -73,8 +85,9 @@ auth_chain_finish (gpointer user_data) NMAuthChain *self = user_data; self->idle_id = 0; + self->done = TRUE; - /* Ensure we say alive across the callback */ + /* Ensure we stay alive across the callback */ self->refcount++; self->done_func (self, self->error, self->context, self->user_data); nm_auth_chain_unref (self); @@ -116,27 +129,33 @@ nm_auth_chain_new_subject (NMAuthSubject *subject, g_return_val_if_fail (NM_IS_AUTH_SUBJECT (subject), NULL); g_return_val_if_fail (nm_auth_subject_is_unix_process (subject) || nm_auth_subject_is_internal (subject), NULL); - self = g_malloc0 (sizeof (NMAuthChain)); + self = g_slice_new0 (NMAuthChain); self->refcount = 1; - self->data = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, free_data); + self->data = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, chain_data_free); self->done_func = done_func; self->user_data = user_data; - self->context = context; + self->context = context ? g_object_ref (context) : NULL; self->subject = g_object_ref (subject); return self; } -gpointer -nm_auth_chain_get_data (NMAuthChain *self, const char *tag) +static gpointer +_get_data (NMAuthChain *self, const char *tag) { ChainData *tmp; + tmp = g_hash_table_lookup (self->data, tag); + return tmp ? tmp->data : NULL; +} + +gpointer +nm_auth_chain_get_data (NMAuthChain *self, const char *tag) +{ g_return_val_if_fail (self != NULL, NULL); g_return_val_if_fail (tag != NULL, NULL); - tmp = g_hash_table_lookup (self->data, tag); - return tmp ? tmp->data : NULL; + return _get_data (self, tag); } /** @@ -165,7 +184,7 @@ nm_auth_chain_steal_data (NMAuthChain *self, const char *tag) value = tmp->data; /* Make sure the destroy handler isn't called when freeing */ tmp->destroy = NULL; - free_data (tmp); + chain_data_free (tmp); g_free (orig_key); } return value; @@ -177,35 +196,30 @@ nm_auth_chain_set_data (NMAuthChain *self, gpointer data, GDestroyNotify data_destroy) { - ChainData *tmp; - g_return_if_fail (self != NULL); g_return_if_fail (tag != NULL); if (data == NULL) g_hash_table_remove (self->data, tag); else { - tmp = g_malloc0 (sizeof (ChainData)); - tmp->data = data; - tmp->destroy = data_destroy; - - g_hash_table_insert (self->data, g_strdup (tag), tmp); + g_hash_table_insert (self->data, + g_strdup (tag), + chain_data_new (data, data_destroy)); } } gulong nm_auth_chain_get_data_ulong (NMAuthChain *self, const char *tag) { - gulong *ptr; + gulong *data; g_return_val_if_fail (self != NULL, 0); g_return_val_if_fail (tag != NULL, 0); - ptr = nm_auth_chain_get_data (self, tag); - return *ptr; + data = _get_data (self, tag); + return data ? *data : 0ul; } - void nm_auth_chain_set_data_ulong (NMAuthChain *self, const char *tag, @@ -232,30 +246,13 @@ nm_auth_chain_get_subject (NMAuthChain *self) NMAuthCallResult nm_auth_chain_get_result (NMAuthChain *self, const char *permission) { + gpointer data; + g_return_val_if_fail (self != NULL, NM_AUTH_CALL_RESULT_UNKNOWN); g_return_val_if_fail (permission != NULL, NM_AUTH_CALL_RESULT_UNKNOWN); - return GPOINTER_TO_UINT (nm_auth_chain_get_data (self, permission)); -} - -static void -nm_auth_chain_check_done (NMAuthChain *self) -{ - g_return_if_fail (self != NULL); - - if (g_slist_length (self->calls) == 0) { - g_assert (self->idle_id == 0); - self->idle_id = g_idle_add (auth_chain_finish, self); - } -} - -static void -nm_auth_chain_remove_call (NMAuthChain *self, AuthCall *call) -{ - g_return_if_fail (self != NULL); - g_return_if_fail (call != NULL); - - self->calls = g_slist_remove (self->calls, call); + data = _get_data (self, permission); + return data ? GPOINTER_TO_UINT (data) : NM_AUTH_CALL_RESULT_UNKNOWN; } static AuthCall * @@ -263,10 +260,9 @@ auth_call_new (NMAuthChain *chain, const char *permission) { AuthCall *call; - call = g_malloc0 (sizeof (AuthCall)); + call = g_slice_new0 (AuthCall); call->chain = chain; call->permission = g_strdup (permission); - chain->calls = g_slist_append (chain->calls, call); return call; } @@ -275,15 +271,27 @@ auth_call_free (AuthCall *call) { g_free (call->permission); g_clear_object (&call->cancellable); - g_free (call); + g_slice_free (AuthCall, call); } -/* This can get used from scheduled idles, hence the boolean return */ static gboolean auth_call_complete (AuthCall *call) { - nm_auth_chain_remove_call (call->chain, call); - nm_auth_chain_check_done (call->chain); + NMAuthChain *self; + + g_return_val_if_fail (call, G_SOURCE_REMOVE); + + self = call->chain; + + g_return_val_if_fail (self, G_SOURCE_REMOVE); + g_return_val_if_fail (g_slist_find (self->calls, call), G_SOURCE_REMOVE); + + self->calls = g_slist_remove (self->calls, call); + + if (!self->calls) { + g_assert (!self->idle_id && !self->done); + self->idle_id = g_idle_add (auth_chain_finish, self); + } auth_call_free (call); return FALSE; } @@ -367,8 +375,10 @@ nm_auth_chain_add_call (NMAuthChain *self, g_return_if_fail (permission && *permission); g_return_if_fail (self->subject); g_return_if_fail (nm_auth_subject_is_unix_process (self->subject) || nm_auth_subject_is_internal (self->subject)); + g_return_if_fail (!self->idle_id && !self->done); call = auth_call_new (self, permission); + self->calls = g_slist_append (self->calls, call); if ( nm_auth_subject_is_internal (self->subject) || nm_auth_subject_get_unix_process_uid (self->subject) == 0 @@ -398,10 +408,21 @@ nm_auth_chain_add_call (NMAuthChain *self, } } +/** + * nm_auth_chain_unref: + * @self: the auth-chain + * + * Unrefs the auth-chain. By unrefing the auth-chain, you also cancel + * the receipt of the done-callback. IOW, the callback will not be invoked. + * + * The only exception is, if you call nm_auth_chain_unref() from inside + * the callback. In this case, @self stays alive until the callback returns. + */ void nm_auth_chain_unref (NMAuthChain *self) { g_return_if_fail (self != NULL); + g_return_if_fail (self->refcount > 0); self->refcount--; if (self->refcount > 0) @@ -412,13 +433,16 @@ nm_auth_chain_unref (NMAuthChain *self) g_object_unref (self->subject); + if (self->context) + g_object_unref (self->context); + g_slist_free_full (self->calls, auth_call_cancel); g_clear_error (&self->error); g_hash_table_destroy (self->data); memset (self, 0, sizeof (NMAuthChain)); - g_free (self); + g_slice_free (NMAuthChain, self); } /************ utils **************/ -- cgit v1.2.1 From 55d672347fe771b715d6644dff72cde5870da4e3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 31 Aug 2015 15:59:38 +0200 Subject: core/trivial: add code comment to nm_utils_get_shared_wifi_permission() --- src/NetworkManagerUtils.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c index 2e732c52a5..d78313df50 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -1540,6 +1540,12 @@ nm_match_spec_join (GSList *specs) return g_string_free (str, FALSE); } +/** + * nm_utils_get_shared_wifi_permission: + * @connection: the NMConnection to lookup the permission. + * + * Returns: a static string of the wifi-permission (if any) or %NULL. + */ const char * nm_utils_get_shared_wifi_permission (NMConnection *connection) { -- cgit v1.2.1 From 745d50185937d150a132bfd8d64d43f347e78444 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 2 Sep 2015 14:23:29 +0200 Subject: agent-manager/refact: merge the subclasses into Request Merge ConnectionRequest structure into Request. --- src/settings/nm-agent-manager.c | 609 +++++++++++++++++++--------------------- 1 file changed, 285 insertions(+), 324 deletions(-) diff --git a/src/settings/nm-agent-manager.c b/src/settings/nm-agent-manager.c index be59b18358..b27c5a0f5f 100644 --- a/src/settings/nm-agent-manager.c +++ b/src/settings/nm-agent-manager.c @@ -70,11 +70,31 @@ NM_DEFINE_SINGLETON_INSTANCE (NMAgentManager); } \ } G_STMT_END -#define LOG_REQ_FMT "[%p/%s%s%s]" -#define LOG_REQ_ARG(req) (req), NM_PRINT_FMT_QUOTE_STRING ((req)->detail) - -#define LOG_REQ2_FMT "[%p/%s%s%s/%s%s%s]" -#define LOG_REQ2_ARG(req) (req), NM_PRINT_FMT_QUOTE_STRING ((req)->parent.detail), NM_PRINT_FMT_QUOTE_STRING ((req)->setting_name) +#define LOG_REQ_FMT "[%p/%s%s%s%s%s%s]" +#define LOG_REQ_ARG(req) \ + (req), \ + NM_PRINT_FMT_QUOTE_STRING ((req)->detail), \ + NM_PRINT_FMT_QUOTED (((req)->request_type == REQUEST_TYPE_CON_GET) && (req)->con.get.setting_name, \ + "/\"", (req)->con.get.setting_name, "\"", \ + ((req)->request_type == REQUEST_TYPE_CON_GET ? "/(none)" : _request_type_to_string ((req)->request_type, FALSE))) + +typedef enum { + REQUEST_TYPE_INVALID, + REQUEST_TYPE_CON_GET, + REQUEST_TYPE_CON_SAVE, + REQUEST_TYPE_CON_DEL, +} RequestType; + +static const char * +_request_type_to_string (RequestType request_type, gboolean verbose) +{ + switch (request_type) { + case REQUEST_TYPE_CON_GET: return verbose ? "getting" : "get"; + case REQUEST_TYPE_CON_SAVE: return verbose ? "saving" : "sav"; + case REQUEST_TYPE_CON_DEL: return verbose ? "deleting" : "del"; + default: return "??"; + } +} G_DEFINE_TYPE (NMAgentManager, nm_agent_manager, NM_TYPE_EXPORTED_OBJECT) @@ -112,6 +132,8 @@ static void request_remove_agent (Request *req, NMSecretAgent *agent, GSList **p static void request_next_agent (Request *req); +static gboolean connection_request_add_agent (Request *req, NMSecretAgent *agent); + /*************************************************************/ static gboolean @@ -423,6 +445,8 @@ typedef void (*RequestCancelFunc) (Request *req); struct _Request { NMAgentManager *self; + RequestType request_type; + guint32 reqid; char *detail; char *verb; @@ -438,53 +462,90 @@ struct _Request { guint32 idle_id; - RequestAddAgentFunc add_agent_callback; RequestCancelFunc cancel_callback; RequestNextFunc next_callback; RequestCompleteFunc complete_callback; gpointer complete_callback_data; gboolean completed; - GDestroyNotify free_func; + union { + struct { + NMConnection *connection; + + NMAuthChain *chain; + + /* Whether the agent currently being asked for secrets + * has the system.modify privilege. + */ + gboolean current_has_modify; + + union { + struct { + NMSecretAgentGetSecretsFlags flags; + char *setting_name; + char **hints; + + GVariant *existing_secrets; + + NMAgentSecretsResultFunc callback; + gpointer callback_data; + gpointer other_data2; + gpointer other_data3; + } get; + }; + } con; + }; }; static guint32 next_req_id = 1; static Request * -request_new (gsize struct_size, - NMAgentManager *self, +request_new (NMAgentManager *self, + RequestType request_type, const char *detail, const char *verb, NMAuthSubject *subject, RequestCompleteFunc complete_callback, gpointer complete_callback_data, - RequestAddAgentFunc add_agent_callback, RequestNextFunc next_callback, - RequestCancelFunc cancel_callback, - GDestroyNotify free_func) + RequestCancelFunc cancel_callback) { Request *req; - req = g_malloc0 (struct_size); + req = g_slice_new0 (Request); req->self = g_object_ref (self); + req->request_type = request_type; req->reqid = next_req_id++; req->detail = g_strdup (detail); req->verb = g_strdup (verb); req->subject = g_object_ref (subject); req->complete_callback = complete_callback; req->complete_callback_data = complete_callback_data; - req->add_agent_callback = add_agent_callback, req->next_callback = next_callback; req->cancel_callback = cancel_callback; - req->free_func = free_func; return req; } static void request_free (Request *req) { - if (req->free_func) - req->free_func ((gpointer) req); + switch (req->request_type) { + case REQUEST_TYPE_CON_GET: + case REQUEST_TYPE_CON_SAVE: + case REQUEST_TYPE_CON_DEL: + g_object_unref (req->con.connection); + if (req->con.chain) + nm_auth_chain_unref (req->con.chain); + if (req->request_type == REQUEST_TYPE_CON_GET) { + g_free (req->con.get.setting_name); + g_strfreev (req->con.get.hints); + if (req->con.get.existing_secrets) + g_variant_unref (req->con.get.existing_secrets); + } + break; + default: + g_assert_not_reached (); + } if (req->idle_id) g_source_remove (req->idle_id); @@ -504,7 +565,7 @@ request_free (Request *req) g_object_unref (req->current); memset (req, 0, sizeof (Request)); - g_free (req); + g_slice_free (Request, req); } static void @@ -575,8 +636,10 @@ request_add_agent (Request *req, NMSecretAgent *agent) self = req->self; - if (req->add_agent_callback && !req->add_agent_callback (req, agent)) - return; + if (req->request_type == REQUEST_TYPE_CON_GET) { + if (!connection_request_add_agent (req, agent)) + return; + } /* If the request should filter agents by UID, do that now */ if (nm_auth_subject_is_unix_process (req->subject)) { @@ -687,59 +750,22 @@ request_start (gpointer user_data) /*************************************************************/ -/* Request subclass for connection secrets */ -typedef struct { - Request parent; - - NMSecretAgentGetSecretsFlags flags; - NMConnection *connection; - char *setting_name; - char **hints; - - GVariant *existing_secrets; - - NMAgentSecretsResultFunc callback; - gpointer callback_data; - gpointer other_data2; - gpointer other_data3; - - NMAuthChain *chain; - - /* Whether the agent currently being asked for secrets - * has the system.modify privilege. - */ - gboolean current_has_modify; -} ConnectionRequest; - -static void -connection_request_free (gpointer data) -{ - ConnectionRequest *req = data; - - g_object_unref (req->connection); - g_free (req->setting_name); - g_strfreev (req->hints); - if (req->existing_secrets) - g_variant_unref (req->existing_secrets); - if (req->chain) - nm_auth_chain_unref (req->chain); -} - static gboolean -connection_request_add_agent (Request *parent, NMSecretAgent *agent) +connection_request_add_agent (Request *req, NMSecretAgent *agent) { NMAgentManager *self; - ConnectionRequest *req = (ConnectionRequest *) parent; NMAuthSubject *subject = nm_secret_agent_get_subject(agent); - self = parent->self; + g_return_val_if_fail (req->request_type == REQUEST_TYPE_CON_GET, FALSE); + + self = req->self; /* Ensure the caller's username exists in the connection's permissions, * or that the permissions is empty (ie, visible by everyone). */ - if (!nm_auth_is_subject_in_acl (req->connection, subject, NULL)) { + if (!nm_auth_is_subject_in_acl (req->con.connection, subject, NULL)) { _LOGD (agent, "agent ignored for secrets request "LOG_REQ_FMT" (not in ACL)", - LOG_REQ_ARG (parent)); + LOG_REQ_ARG (req)); /* Connection not visible to this agent's user */ return FALSE; } @@ -747,79 +773,6 @@ connection_request_add_agent (Request *parent, NMSecretAgent *agent) return TRUE; } -static ConnectionRequest * -connection_request_new_get (NMAgentManager *self, - NMConnection *connection, - NMAuthSubject *subject, - GVariant *existing_secrets, - const char *setting_name, - const char *verb, - NMSecretAgentGetSecretsFlags flags, - const char **hints, - NMAgentSecretsResultFunc callback, - gpointer callback_data, - gpointer other_data2, - gpointer other_data3, - RequestCompleteFunc complete_callback, - gpointer complete_callback_data, - RequestNextFunc next_callback, - RequestCancelFunc cancel_callback) -{ - ConnectionRequest *req; - - req = (ConnectionRequest *) request_new (sizeof (ConnectionRequest), - self, - nm_connection_get_id (connection), - verb, - subject, - complete_callback, - complete_callback_data, - connection_request_add_agent, - next_callback, - cancel_callback, - connection_request_free); - g_assert (req); - - req->connection = g_object_ref (connection); - if (existing_secrets) - req->existing_secrets = g_variant_ref (existing_secrets); - req->setting_name = g_strdup (setting_name); - req->hints = g_strdupv ((char **) hints); - req->flags = flags; - req->callback = callback; - req->callback_data = callback_data; - req->other_data2 = other_data2; - req->other_data3 = other_data3; - return req; -} - -static ConnectionRequest * -connection_request_new_other (NMAgentManager *self, - NMConnection *connection, - NMAuthSubject *subject, - const char *verb, - RequestCompleteFunc complete_callback, - gpointer complete_callback_data, - RequestNextFunc next_callback) -{ - ConnectionRequest *req; - - req = (ConnectionRequest *) request_new (sizeof (ConnectionRequest), - self, - nm_connection_get_id (connection), - verb, - subject, - complete_callback, - complete_callback_data, - NULL, - next_callback, - NULL, - connection_request_free); - g_assert (req); - req->connection = g_object_ref (connection); - return req; -} - static void get_done_cb (NMSecretAgent *agent, NMSecretAgentCallId call_id, @@ -828,57 +781,57 @@ get_done_cb (NMSecretAgent *agent, gpointer user_data) { NMAgentManager *self; - Request *parent = user_data; - ConnectionRequest *req = user_data; + Request *req = user_data; GVariant *setting_secrets; const char *agent_dbus_owner; struct passwd *pw; char *agent_uname = NULL; - g_return_if_fail (call_id == parent->current_call_id); - g_return_if_fail (agent == parent->current); + g_return_if_fail (call_id == req->current_call_id); + g_return_if_fail (agent == req->current); + g_return_if_fail (req->request_type == REQUEST_TYPE_CON_GET); - self = parent->self; + self = req->self; - parent->current_call_id = NULL; + req->current_call_id = NULL; if (error) { if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) { - _LOGD (agent, "get secrets request cancelled: "LOG_REQ2_FMT, - LOG_REQ2_ARG (req)); + _LOGD (agent, "get secrets request cancelled: "LOG_REQ_FMT, + LOG_REQ_ARG (req)); return; } - _LOGD (agent, "agent failed secrets request "LOG_REQ2_FMT": %s", - LOG_REQ2_ARG (req), + _LOGD (agent, "agent failed secrets request "LOG_REQ_FMT": %s", + LOG_REQ_ARG (req), error->message); if (g_error_matches (error, NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_USER_CANCELED)) { error = g_error_new_literal (NM_AGENT_MANAGER_ERROR, NM_AGENT_MANAGER_ERROR_USER_CANCELED, "User canceled the secrets request."); - req_complete_error (parent, error); + req_complete_error (req, error); g_error_free (error); } else { /* Try the next agent */ - request_next_agent (parent); + request_next_agent (req); maybe_remove_agent_on_error (agent, error); } return; } /* Ensure the setting we wanted secrets for got returned and has something in it */ - setting_secrets = g_variant_lookup_value (secrets, req->setting_name, NM_VARIANT_TYPE_SETTING); + setting_secrets = g_variant_lookup_value (secrets, req->con.get.setting_name, NM_VARIANT_TYPE_SETTING); if (!setting_secrets || !g_variant_n_children (setting_secrets)) { - _LOGD (agent, "agent returned no secrets for request "LOG_REQ2_FMT, - LOG_REQ2_ARG (req)); + _LOGD (agent, "agent returned no secrets for request "LOG_REQ_FMT, + LOG_REQ_ARG (req)); /* Try the next agent */ - request_next_agent (parent); + request_next_agent (req); return; } - _LOGD (agent, "agent returned secrets for request "LOG_REQ2_FMT, - LOG_REQ2_ARG (req)); + _LOGD (agent, "agent returned secrets for request "LOG_REQ_FMT, + LOG_REQ_ARG (req)); /* Get the agent's username */ pw = getpwuid (nm_secret_agent_get_owner_uid (agent)); @@ -889,7 +842,7 @@ get_done_cb (NMSecretAgent *agent, } agent_dbus_owner = nm_secret_agent_get_dbus_owner (agent); - req_complete_success (parent, secrets, agent_dbus_owner, agent_uname); + req_complete_success (req, secrets, agent_dbus_owner, agent_uname); g_free (agent_uname); } @@ -934,35 +887,36 @@ set_secrets_not_required (NMConnection *connection, GVariant *dict) } static void -get_agent_request_secrets (ConnectionRequest *req, gboolean include_system_secrets) +get_agent_request_secrets (Request *req, gboolean include_system_secrets) { - Request *parent = (Request *) req; NMConnection *tmp; - tmp = nm_simple_connection_new_clone (req->connection); + g_return_if_fail (req->request_type == REQUEST_TYPE_CON_GET); + + tmp = nm_simple_connection_new_clone (req->con.connection); nm_connection_clear_secrets (tmp); if (include_system_secrets) { - if (req->existing_secrets) - (void) nm_connection_update_secrets (tmp, req->setting_name, req->existing_secrets, NULL); + if (req->con.get.existing_secrets) + (void) nm_connection_update_secrets (tmp, req->con.get.setting_name, req->con.get.existing_secrets, NULL); } else { /* Update secret flags in the temporary connection to indicate that * the system secrets we're not sending to the agent aren't required, * so the agent can properly validate UI controls and such. */ - if (req->existing_secrets) - set_secrets_not_required (tmp, req->existing_secrets); + if (req->con.get.existing_secrets) + set_secrets_not_required (tmp, req->con.get.existing_secrets); } - parent->current_call_id = nm_secret_agent_get_secrets (parent->current, - tmp, - req->setting_name, - (const char **) req->hints, - req->flags, - get_done_cb, - req); - if (!parent->current_call_id) { + req->current_call_id = nm_secret_agent_get_secrets (req->current, + tmp, + req->con.get.setting_name, + (const char **) req->con.get.hints, + req->con.get.flags, + get_done_cb, + req); + if (!req->current_call_id) { g_warn_if_reached (); - request_next_agent (parent); + request_next_agent (req); } g_object_unref (tmp); @@ -975,20 +929,21 @@ get_agent_modify_auth_cb (NMAuthChain *chain, gpointer user_data) { NMAgentManager *self; - Request *parent = user_data; - ConnectionRequest *req = user_data; + Request *req = user_data; const char *perm; - self = parent->self; + g_return_if_fail (req->request_type == REQUEST_TYPE_CON_GET); - req->chain = NULL; + self = req->self; + + req->con.chain = NULL; if (error) { - _LOGD (parent->current, "agent "LOG_REQ2_FMT" MODIFY check error: (%d) %s", - LOG_REQ2_ARG (req), + _LOGD (req->current, "agent "LOG_REQ_FMT" MODIFY check error: (%d) %s", + LOG_REQ_ARG (req), error->code, error->message ? error->message : "(unknown)"); /* Try the next agent */ - request_next_agent (parent); + request_next_agent (req); } else { /* If the agent obtained the 'modify' permission, we send all system secrets * to it. If it didn't, we still ask it for secrets, but we don't send @@ -997,13 +952,13 @@ get_agent_modify_auth_cb (NMAuthChain *chain, perm = nm_auth_chain_get_data (chain, "perm"); g_assert (perm); if (nm_auth_chain_get_result (chain, perm) == NM_AUTH_CALL_RESULT_YES) - req->current_has_modify = TRUE; + req->con.current_has_modify = TRUE; - _LOGD (parent->current, "agent "LOG_REQ2_FMT" MODIFY check result %s", - LOG_REQ2_ARG (req), - req->current_has_modify ? "YES" : "NO"); + _LOGD (req->current, "agent "LOG_REQ_FMT" MODIFY check result %s", + LOG_REQ_ARG (req), + req->con.current_has_modify ? "YES" : "NO"); - get_agent_request_secrets (req, req->current_has_modify); + get_agent_request_secrets (req, req->con.current_has_modify); } nm_auth_chain_unref (chain); @@ -1053,18 +1008,19 @@ has_system_secrets (NMConnection *connection) } static void -get_next_cb (Request *parent) +get_next_cb (Request *req) { NMAgentManager *self; - ConnectionRequest *req = (ConnectionRequest *) parent; NMSettingConnection *s_con; const char *agent_dbus_owner, *perm; - self = parent->self; + g_return_if_fail (req->request_type == REQUEST_TYPE_CON_GET); + + self = req->self; - req->current_has_modify = FALSE; + req->con.current_has_modify = FALSE; - agent_dbus_owner = nm_secret_agent_get_dbus_owner (parent->current); + agent_dbus_owner = nm_secret_agent_get_dbus_owner (req->current); /* If the request flags allow user interaction, and there are existing * system secrets (or blank secrets that are supposed to be system-owned), @@ -1072,33 +1028,33 @@ get_next_cb (Request *parent) * secrets to the agent. We shouldn't leak system-owned secrets to * unprivileged users. */ - if ( (req->flags != NM_SECRET_AGENT_GET_SECRETS_FLAG_NONE) - && (req->existing_secrets || has_system_secrets (req->connection))) { - _LOGD (NULL, "("LOG_REQ2_FMT") request has system secrets; checking agent %s for MODIFY", - LOG_REQ2_ARG (req), agent_dbus_owner); + if ( (req->con.get.flags != NM_SECRET_AGENT_GET_SECRETS_FLAG_NONE) + && (req->con.get.existing_secrets || has_system_secrets (req->con.connection))) { + _LOGD (NULL, "("LOG_REQ_FMT") request has system secrets; checking agent %s for MODIFY", + LOG_REQ_ARG (req), agent_dbus_owner); - req->chain = nm_auth_chain_new_subject (nm_secret_agent_get_subject (parent->current), - NULL, - get_agent_modify_auth_cb, - req); - g_assert (req->chain); + req->con.chain = nm_auth_chain_new_subject (nm_secret_agent_get_subject (req->current), + NULL, + get_agent_modify_auth_cb, + req); + g_assert (req->con.chain); /* If the caller is the only user in the connection's permissions, then * we use the 'modify.own' permission instead of 'modify.system'. If the * request affects more than just the caller, require 'modify.system'. */ - s_con = nm_connection_get_setting_connection (req->connection); + s_con = nm_connection_get_setting_connection (req->con.connection); g_assert (s_con); if (nm_setting_connection_get_num_permissions (s_con) == 1) perm = NM_AUTH_PERMISSION_SETTINGS_MODIFY_OWN; else perm = NM_AUTH_PERMISSION_SETTINGS_MODIFY_SYSTEM; - nm_auth_chain_set_data (req->chain, "perm", (gpointer) perm, NULL); + nm_auth_chain_set_data (req->con.chain, "perm", (gpointer) perm, NULL); - nm_auth_chain_add_call (req->chain, perm, TRUE); + nm_auth_chain_add_call (req->con.chain, perm, TRUE); } else { - _LOGD (NULL, "("LOG_REQ2_FMT") requesting user-owned secrets from agent %s", - LOG_REQ2_ARG (req), agent_dbus_owner); + _LOGD (NULL, "("LOG_REQ_FMT") requesting user-owned secrets from agent %s", + LOG_REQ_ARG (req), agent_dbus_owner); get_agent_request_secrets (req, FALSE); } @@ -1108,55 +1064,56 @@ static gboolean get_start (gpointer user_data) { NMAgentManager *self; - Request *parent = user_data; - ConnectionRequest *req = user_data; + Request *req = user_data; GVariant *setting_secrets = NULL; - self = parent->self; + g_return_val_if_fail (req->request_type == REQUEST_TYPE_CON_GET, G_SOURCE_REMOVE); - parent->idle_id = 0; + self = req->self; + + req->idle_id = 0; /* Check if there are any existing secrets */ - if (req->existing_secrets) - setting_secrets = g_variant_lookup_value (req->existing_secrets, req->setting_name, NM_VARIANT_TYPE_SETTING); + if (req->con.get.existing_secrets) + setting_secrets = g_variant_lookup_value (req->con.get.existing_secrets, req->con.get.setting_name, NM_VARIANT_TYPE_SETTING); if (setting_secrets && g_variant_n_children (setting_secrets)) { NMConnection *tmp; GError *error = NULL; - gboolean new_secrets = (req->flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW); + gboolean new_secrets = (req->con.get.flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW); /* The connection already had secrets; check if any more are required. * If no more are required, we're done. If secrets are still needed, * ask a secret agent for more. This allows admins to provide generic * secrets but allow additional user-specific ones as well. */ - tmp = nm_simple_connection_new_clone (req->connection); + tmp = nm_simple_connection_new_clone (req->con.connection); g_assert (tmp); - if (!nm_connection_update_secrets (tmp, req->setting_name, req->existing_secrets, &error)) { - req_complete_error (parent, error); + if (!nm_connection_update_secrets (tmp, req->con.get.setting_name, req->con.get.existing_secrets, &error)) { + req_complete_error (req, error); g_clear_error (&error); } else { /* Do we have everything we need? */ - if ( (req->flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_ONLY_SYSTEM) + if ( (req->con.get.flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_ONLY_SYSTEM) || ((nm_connection_need_secrets (tmp, NULL) == NULL) && (new_secrets == FALSE))) { - _LOGD (NULL, "("LOG_REQ2_FMT") system settings secrets sufficient", - LOG_REQ2_ARG (req)); + _LOGD (NULL, "("LOG_REQ_FMT") system settings secrets sufficient", + LOG_REQ_ARG (req)); /* Got everything, we're done */ - req_complete_success (parent, req->existing_secrets, NULL, NULL); + req_complete_success (req, req->con.get.existing_secrets, NULL, NULL); } else { - _LOGD (NULL, "("LOG_REQ2_FMT") system settings secrets insufficient, asking agents", - LOG_REQ2_ARG (req)); + _LOGD (NULL, "("LOG_REQ_FMT") system settings secrets insufficient, asking agents", + LOG_REQ_ARG (req)); /* We don't, so ask some agents for additional secrets */ - if ( req->flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_NO_ERRORS - && !parent->pending) { + if ( req->con.get.flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_NO_ERRORS + && !req->pending) { /* The request initiated from GetSecrets() via DBus, * don't error out if any secrets are missing. */ - req_complete_success (parent, req->existing_secrets, NULL, NULL); + req_complete_success (req, req->con.get.existing_secrets, NULL, NULL); } else - request_next_agent (parent); + request_next_agent (req); } } g_object_unref (tmp); @@ -1165,7 +1122,7 @@ get_start (gpointer user_data) * agents for secrets. Let the Agent Manager handle which agents * we'll ask and in which order. */ - request_next_agent (parent); + request_next_agent (req); } if (setting_secrets) @@ -1175,7 +1132,7 @@ get_start (gpointer user_data) } static void -get_complete_cb (Request *parent, +get_complete_cb (Request *req, GVariant *secrets, const char *agent_dbus_owner, const char *agent_username, @@ -1184,33 +1141,32 @@ get_complete_cb (Request *parent, { NMAgentManager *self = NM_AGENT_MANAGER (user_data); NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self); - ConnectionRequest *req = (ConnectionRequest *) parent; + + g_return_if_fail (req->request_type == REQUEST_TYPE_CON_GET); /* Send secrets back to the requesting object */ - req->callback (self, - parent->reqid, - agent_dbus_owner, - agent_username, - req->current_has_modify, - req->setting_name, - req->flags, - error ? NULL : secrets, - error, - req->callback_data, - req->other_data2, - req->other_data3); - - g_hash_table_remove (priv->requests, GUINT_TO_POINTER (parent->reqid)); + req->con.get.callback (self, + req->reqid, + agent_dbus_owner, + agent_username, + req->con.current_has_modify, + req->con.get.setting_name, + req->con.get.flags, + error ? NULL : secrets, + error, + req->con.get.callback_data, + req->con.get.other_data2, + req->con.get.other_data3); + + g_hash_table_remove (priv->requests, GUINT_TO_POINTER (req->reqid)); } static void -get_cancel_cb (Request *parent) +get_cancel_cb (Request *req) { - ConnectionRequest *req = (ConnectionRequest *) parent; - - req->current_has_modify = FALSE; - if (parent->current && parent->current_call_id) - nm_secret_agent_cancel_secrets (parent->current, parent->current_call_id); + req->con.current_has_modify = FALSE; + if (req->current && req->current_call_id) + nm_secret_agent_cancel_secrets (req->current, req->current_call_id); } guint32 @@ -1227,8 +1183,7 @@ nm_agent_manager_get_secrets (NMAgentManager *self, gpointer other_data3) { NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self); - Request *parent; - ConnectionRequest *req; + Request *req; g_return_val_if_fail (self != NULL, 0); g_return_val_if_fail (NM_IS_CONNECTION (connection), 0); @@ -1245,31 +1200,34 @@ nm_agent_manager_get_secrets (NMAgentManager *self, * This in turn depends on nm_connection_to_dbus() and nm_setting_to_hash() * both returning NULL if they didn't hash anything. */ + req = request_new (self, + REQUEST_TYPE_CON_GET, + nm_connection_get_id (connection), + "getting", + subject, + get_complete_cb, + self, + get_next_cb, + get_cancel_cb); + + req->con.connection = g_object_ref (connection); + if (existing_secrets) + req->con.get.existing_secrets = g_variant_ref (existing_secrets); + req->con.get.setting_name = g_strdup (setting_name); + req->con.get.hints = g_strdupv ((char **) hints); + req->con.get.flags = flags; + req->con.get.callback = callback; + req->con.get.callback_data = callback_data; + req->con.get.other_data2 = other_data2; + req->con.get.other_data3 = other_data3; - req = connection_request_new_get (self, - connection, - subject, - existing_secrets, - setting_name, - "getting", - flags, - hints, - callback, - callback_data, - other_data2, - other_data3, - get_complete_cb, - self, - get_next_cb, - get_cancel_cb); - parent = (Request *) req; - g_hash_table_insert (priv->requests, GUINT_TO_POINTER (parent->reqid), req); + g_hash_table_insert (priv->requests, GUINT_TO_POINTER (req->reqid), req); /* Kick off the request */ - if (!(req->flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_ONLY_SYSTEM)) - request_add_agents (self, parent); - parent->idle_id = g_idle_add (get_start, req); - return parent->reqid; + if (!(req->con.get.flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_ONLY_SYSTEM)) + request_add_agents (self, req); + req->idle_id = g_idle_add (get_start, req); + return req->reqid; } void @@ -1293,51 +1251,51 @@ save_done_cb (NMSecretAgent *agent, gpointer user_data) { NMAgentManager *self; - ConnectionRequest *req = user_data; - Request *parent = (Request *) req; + Request *req = user_data; const char *agent_dbus_owner; - g_return_if_fail (call_id == parent->current_call_id); - g_return_if_fail (agent == parent->current); + g_return_if_fail (call_id == req->current_call_id); + g_return_if_fail (agent == req->current); + g_return_if_fail (req->request_type == REQUEST_TYPE_CON_SAVE); - self = parent->self; + self = req->self; - parent->current_call_id = NULL; + req->current_call_id = NULL; if (error) { if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) { _LOGD (agent, "save secrets request cancelled: "LOG_REQ_FMT, - LOG_REQ_ARG (parent)); + LOG_REQ_ARG (req)); return; } _LOGD (agent, "agent failed save secrets request "LOG_REQ_FMT": %s", - LOG_REQ_ARG (parent), error->message); + LOG_REQ_ARG (req), error->message); /* Try the next agent */ - request_next_agent (parent); + request_next_agent (req); maybe_remove_agent_on_error (agent, error); return; } _LOGD (agent, "agent saved secrets for request "LOG_REQ_FMT, - LOG_REQ_ARG (parent)); + LOG_REQ_ARG (req)); agent_dbus_owner = nm_secret_agent_get_dbus_owner (agent); - req_complete_success (parent, NULL, NULL, agent_dbus_owner); + req_complete_success (req, NULL, NULL, agent_dbus_owner); } static void -save_next_cb (Request *parent) +save_next_cb (Request *req) { - ConnectionRequest *req = (ConnectionRequest *) parent; + g_return_if_fail (req->request_type == REQUEST_TYPE_CON_SAVE); - parent->current_call_id = nm_secret_agent_save_secrets (parent->current, - req->connection, - save_done_cb, - req); - if (!parent->current_call_id) { + req->current_call_id = nm_secret_agent_save_secrets (req->current, + req->con.connection, + save_done_cb, + req); + if (!req->current_call_id) { g_warn_if_reached (); - request_next_agent (parent); + request_next_agent (req); } } @@ -1359,8 +1317,7 @@ nm_agent_manager_save_secrets (NMAgentManager *self, NMAuthSubject *subject) { NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self); - ConnectionRequest *req; - Request *parent; + Request *req; g_return_val_if_fail (self != NULL, 0); g_return_val_if_fail (NM_IS_CONNECTION (connection), 0); @@ -1370,20 +1327,22 @@ nm_agent_manager_save_secrets (NMAgentManager *self, nm_connection_get_path (connection), nm_connection_get_id (connection)); - req = connection_request_new_other (self, - connection, - subject, - "saving", - save_complete_cb, - self, - save_next_cb); - parent = (Request *) req; - g_hash_table_insert (priv->requests, GUINT_TO_POINTER (parent->reqid), req); + req = request_new (self, + REQUEST_TYPE_CON_SAVE, + nm_connection_get_id (connection), + "saving", + subject, + save_complete_cb, + self, + save_next_cb, + NULL); + req->con.connection = g_object_ref (connection); + g_hash_table_insert (priv->requests, GUINT_TO_POINTER (req->reqid), req); /* Kick off the request */ - request_add_agents (self, parent); - parent->idle_id = g_idle_add (request_start, req); - return parent->reqid; + request_add_agents (self, req); + req->idle_id = g_idle_add (request_start, req); + return req->reqid; } /*************************************************************/ @@ -1400,6 +1359,7 @@ delete_done_cb (NMSecretAgent *agent, g_return_if_fail (call_id == req->current_call_id); g_return_if_fail (agent == req->current); + g_return_if_fail (req->request_type == REQUEST_TYPE_CON_DEL); self = req->self; @@ -1426,17 +1386,17 @@ delete_done_cb (NMSecretAgent *agent, } static void -delete_next_cb (Request *parent) +delete_next_cb (Request *req) { - ConnectionRequest *req = (ConnectionRequest *) parent; + g_return_if_fail (req->request_type == REQUEST_TYPE_CON_DEL); - parent->current_call_id = nm_secret_agent_delete_secrets (parent->current, - req->connection, - delete_done_cb, - req); - if (!parent->current_call_id) { + req->current_call_id = nm_secret_agent_delete_secrets (req->current, + req->con.connection, + delete_done_cb, + req); + if (!req->current_call_id) { g_warn_if_reached (); - request_next_agent (parent); + request_next_agent (req); } } @@ -1458,8 +1418,7 @@ nm_agent_manager_delete_secrets (NMAgentManager *self, { NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self); NMAuthSubject *subject; - ConnectionRequest *req; - Request *parent; + Request *req; g_return_val_if_fail (self != NULL, 0); g_return_val_if_fail (NM_IS_CONNECTION (connection), 0); @@ -1470,21 +1429,23 @@ nm_agent_manager_delete_secrets (NMAgentManager *self, nm_connection_get_id (connection)); subject = nm_auth_subject_new_internal (); - req = connection_request_new_other (self, - connection, - subject, - "deleting", - delete_complete_cb, - self, - delete_next_cb); + req = request_new (self, + REQUEST_TYPE_CON_DEL, + nm_connection_get_id (connection), + "deleting", + subject, + delete_complete_cb, + self, + delete_next_cb, + NULL); + req->con.connection = g_object_ref (connection); g_object_unref (subject); - parent = (Request *) req; - g_hash_table_insert (priv->requests, GUINT_TO_POINTER (parent->reqid), req); + g_hash_table_insert (priv->requests, GUINT_TO_POINTER (req->reqid), req); /* Kick off the request */ - request_add_agents (self, parent); - parent->idle_id = g_idle_add (request_start, req); - return parent->reqid; + request_add_agents (self, req); + req->idle_id = g_idle_add (request_start, req); + return req->reqid; } /*************************************************************/ -- cgit v1.2.1 From ea57ecc8eee0ea759457f1856868ea56024986d2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 2 Sep 2015 15:20:20 +0200 Subject: agent-manager/refact: replace function callbacks by direct calls or inline Drop the function pointers. Instead either inline them or call them explicitly (possibly after switching on the request_type). --- src/settings/nm-agent-manager.c | 240 ++++++++++++++-------------------------- 1 file changed, 81 insertions(+), 159 deletions(-) diff --git a/src/settings/nm-agent-manager.c b/src/settings/nm-agent-manager.c index b27c5a0f5f..7b9656bd82 100644 --- a/src/settings/nm-agent-manager.c +++ b/src/settings/nm-agent-manager.c @@ -132,7 +132,9 @@ static void request_remove_agent (Request *req, NMSecretAgent *agent, GSList **p static void request_next_agent (Request *req); -static gboolean connection_request_add_agent (Request *req, NMSecretAgent *agent); +static void get_next_cb (Request *req); +static void save_next_cb (Request *req); +static void delete_next_cb (Request *req); /*************************************************************/ @@ -431,17 +433,6 @@ done: /*************************************************************/ -typedef void (*RequestCompleteFunc) (Request *req, - GVariant *secrets, - const char *agent_dbus_owner, - const char *agent_username, - GError *error, - gpointer user_data); -typedef gboolean (*RequestAddAgentFunc) (Request *req, NMSecretAgent *agent); -typedef void (*RequestNextFunc) (Request *req); -typedef void (*RequestCancelFunc) (Request *req); - -/* Basic secrets request structure */ struct _Request { NMAgentManager *self; @@ -449,7 +440,6 @@ struct _Request { guint32 reqid; char *detail; - char *verb; NMAuthSubject *subject; @@ -462,10 +452,6 @@ struct _Request { guint32 idle_id; - RequestCancelFunc cancel_callback; - RequestNextFunc next_callback; - RequestCompleteFunc complete_callback; - gpointer complete_callback_data; gboolean completed; union { @@ -503,12 +489,7 @@ static Request * request_new (NMAgentManager *self, RequestType request_type, const char *detail, - const char *verb, - NMAuthSubject *subject, - RequestCompleteFunc complete_callback, - gpointer complete_callback_data, - RequestNextFunc next_callback, - RequestCancelFunc cancel_callback) + NMAuthSubject *subject) { Request *req; @@ -517,12 +498,7 @@ request_new (NMAgentManager *self, req->request_type = request_type; req->reqid = next_req_id++; req->detail = g_strdup (detail); - req->verb = g_strdup (verb); req->subject = g_object_ref (subject); - req->complete_callback = complete_callback; - req->complete_callback_data = complete_callback_data; - req->next_callback = next_callback; - req->cancel_callback = cancel_callback; return req; } @@ -550,13 +526,21 @@ request_free (Request *req) if (req->idle_id) g_source_remove (req->idle_id); - if (!req->completed && req->cancel_callback) - req->cancel_callback (req); + if (!req->completed) { + switch (req->request_type) { + case REQUEST_TYPE_CON_GET: + req->con.current_has_modify = FALSE; + if (req->current && req->current_call_id) + nm_secret_agent_cancel_secrets (req->current, req->current_call_id); + break; + default: + break; + } + } g_object_unref (req->subject); g_free (req->detail); - g_free (req->verb); g_slist_free_full (req->pending, g_object_unref); g_object_unref (req->self); @@ -569,25 +553,47 @@ request_free (Request *req) } static void -req_complete_success (Request *req, - GVariant *secrets, - const char *agent_dbus_owner, - const char *agent_uname) +req_complete (Request *req, + GVariant *secrets, + const char *agent_dbus_owner, + const char *agent_username, + GError *error) { + NMAgentManager *self = req->self; + NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self); + req->completed = TRUE; - req->complete_callback (req, - secrets, - agent_dbus_owner, - agent_uname, - NULL, - req->complete_callback_data); + + switch (req->request_type) { + case REQUEST_TYPE_CON_GET: + req->con.get.callback (self, + req->reqid, + agent_dbus_owner, + agent_username, + req->con.current_has_modify, + req->con.get.setting_name, + req->con.get.flags, + error ? NULL : secrets, + error, + req->con.get.callback_data, + req->con.get.other_data2, + req->con.get.other_data3); + + break; + case REQUEST_TYPE_CON_SAVE: + case REQUEST_TYPE_CON_DEL: + break; + default: + g_return_if_reached (); + } + + g_hash_table_remove (priv->requests, GUINT_TO_POINTER (req->reqid)); } static void req_complete_error (Request *req, GError *error) { - req->completed = TRUE; - req->complete_callback (req, NULL, NULL, NULL, error, req->complete_callback_data); + req_complete (req, NULL, NULL, NULL, error); } static gint @@ -637,8 +643,17 @@ request_add_agent (Request *req, NMSecretAgent *agent) self = req->self; if (req->request_type == REQUEST_TYPE_CON_GET) { - if (!connection_request_add_agent (req, agent)) + NMAuthSubject *subject = nm_secret_agent_get_subject (agent); + + /* Ensure the caller's username exists in the connection's permissions, + * or that the permissions is empty (ie, visible by everyone). + */ + if (!nm_auth_is_subject_in_acl (req->con.connection, subject, NULL)) { + _LOGD (agent, "agent ignored for secrets request "LOG_REQ_FMT" (not in ACL)", + LOG_REQ_ARG (req)); + /* Connection not visible to this agent's user */ return; + } } /* If the request should filter agents by UID, do that now */ @@ -699,9 +714,22 @@ request_next_agent (Request *req) req->pending = g_slist_remove (req->pending, req->current); _LOGD (req->current, "agent %s secrets for request "LOG_REQ_FMT, - req->verb, LOG_REQ_ARG (req)); + _request_type_to_string (req->request_type, TRUE), + LOG_REQ_ARG (req)); - req->next_callback (req); + switch (req->request_type) { + case REQUEST_TYPE_CON_GET: + get_next_cb (req); + break; + case REQUEST_TYPE_CON_SAVE: + save_next_cb (req); + break; + case REQUEST_TYPE_CON_DEL: + delete_next_cb (req); + break; + default: + g_assert_not_reached (); + } } else { /* No more secret agents are available to fulfill this secrets request */ error = g_error_new_literal (NM_AGENT_MANAGER_ERROR, @@ -750,29 +778,6 @@ request_start (gpointer user_data) /*************************************************************/ -static gboolean -connection_request_add_agent (Request *req, NMSecretAgent *agent) -{ - NMAgentManager *self; - NMAuthSubject *subject = nm_secret_agent_get_subject(agent); - - g_return_val_if_fail (req->request_type == REQUEST_TYPE_CON_GET, FALSE); - - self = req->self; - - /* Ensure the caller's username exists in the connection's permissions, - * or that the permissions is empty (ie, visible by everyone). - */ - if (!nm_auth_is_subject_in_acl (req->con.connection, subject, NULL)) { - _LOGD (agent, "agent ignored for secrets request "LOG_REQ_FMT" (not in ACL)", - LOG_REQ_ARG (req)); - /* Connection not visible to this agent's user */ - return FALSE; - } - - return TRUE; -} - static void get_done_cb (NMSecretAgent *agent, NMSecretAgentCallId call_id, @@ -842,7 +847,7 @@ get_done_cb (NMSecretAgent *agent, } agent_dbus_owner = nm_secret_agent_get_dbus_owner (agent); - req_complete_success (req, secrets, agent_dbus_owner, agent_uname); + req_complete (req, secrets, agent_dbus_owner, agent_uname, NULL); g_free (agent_uname); } @@ -1014,8 +1019,6 @@ get_next_cb (Request *req) NMSettingConnection *s_con; const char *agent_dbus_owner, *perm; - g_return_if_fail (req->request_type == REQUEST_TYPE_CON_GET); - self = req->self; req->con.current_has_modify = FALSE; @@ -1101,7 +1104,7 @@ get_start (gpointer user_data) LOG_REQ_ARG (req)); /* Got everything, we're done */ - req_complete_success (req, req->con.get.existing_secrets, NULL, NULL); + req_complete (req, req->con.get.existing_secrets, NULL, NULL, NULL); } else { _LOGD (NULL, "("LOG_REQ_FMT") system settings secrets insufficient, asking agents", LOG_REQ_ARG (req)); @@ -1111,7 +1114,7 @@ get_start (gpointer user_data) && !req->pending) { /* The request initiated from GetSecrets() via DBus, * don't error out if any secrets are missing. */ - req_complete_success (req, req->con.get.existing_secrets, NULL, NULL); + req_complete (req, req->con.get.existing_secrets, NULL, NULL, NULL); } else request_next_agent (req); } @@ -1131,44 +1134,6 @@ get_start (gpointer user_data) return FALSE; } -static void -get_complete_cb (Request *req, - GVariant *secrets, - const char *agent_dbus_owner, - const char *agent_username, - GError *error, - gpointer user_data) -{ - NMAgentManager *self = NM_AGENT_MANAGER (user_data); - NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self); - - g_return_if_fail (req->request_type == REQUEST_TYPE_CON_GET); - - /* Send secrets back to the requesting object */ - req->con.get.callback (self, - req->reqid, - agent_dbus_owner, - agent_username, - req->con.current_has_modify, - req->con.get.setting_name, - req->con.get.flags, - error ? NULL : secrets, - error, - req->con.get.callback_data, - req->con.get.other_data2, - req->con.get.other_data3); - - g_hash_table_remove (priv->requests, GUINT_TO_POINTER (req->reqid)); -} - -static void -get_cancel_cb (Request *req) -{ - req->con.current_has_modify = FALSE; - if (req->current && req->current_call_id) - nm_secret_agent_cancel_secrets (req->current, req->current_call_id); -} - guint32 nm_agent_manager_get_secrets (NMAgentManager *self, NMConnection *connection, @@ -1203,12 +1168,7 @@ nm_agent_manager_get_secrets (NMAgentManager *self, req = request_new (self, REQUEST_TYPE_CON_GET, nm_connection_get_id (connection), - "getting", - subject, - get_complete_cb, - self, - get_next_cb, - get_cancel_cb); + subject); req->con.connection = g_object_ref (connection); if (existing_secrets) @@ -1281,14 +1241,12 @@ save_done_cb (NMSecretAgent *agent, LOG_REQ_ARG (req)); agent_dbus_owner = nm_secret_agent_get_dbus_owner (agent); - req_complete_success (req, NULL, NULL, agent_dbus_owner); + req_complete (req, NULL, NULL, agent_dbus_owner, NULL); } static void save_next_cb (Request *req) { - g_return_if_fail (req->request_type == REQUEST_TYPE_CON_SAVE); - req->current_call_id = nm_secret_agent_save_secrets (req->current, req->con.connection, save_done_cb, @@ -1299,18 +1257,6 @@ save_next_cb (Request *req) } } -static void -save_complete_cb (Request *req, - GVariant *secrets, - const char *agent_dbus_owner, - const char *agent_username, - GError *error, - gpointer user_data) -{ - g_hash_table_remove (NM_AGENT_MANAGER_GET_PRIVATE (user_data)->requests, - GUINT_TO_POINTER (req->reqid)); -} - guint32 nm_agent_manager_save_secrets (NMAgentManager *self, NMConnection *connection, @@ -1330,12 +1276,7 @@ nm_agent_manager_save_secrets (NMAgentManager *self, req = request_new (self, REQUEST_TYPE_CON_SAVE, nm_connection_get_id (connection), - "saving", - subject, - save_complete_cb, - self, - save_next_cb, - NULL); + subject); req->con.connection = g_object_ref (connection); g_hash_table_insert (priv->requests, GUINT_TO_POINTER (req->reqid), req); @@ -1388,8 +1329,6 @@ delete_done_cb (NMSecretAgent *agent, static void delete_next_cb (Request *req) { - g_return_if_fail (req->request_type == REQUEST_TYPE_CON_DEL); - req->current_call_id = nm_secret_agent_delete_secrets (req->current, req->con.connection, delete_done_cb, @@ -1400,18 +1339,6 @@ delete_next_cb (Request *req) } } -static void -delete_complete_cb (Request *req, - GVariant *secrets, - const char *agent_dbus_owner, - const char *agent_username, - GError *error, - gpointer user_data) -{ - g_hash_table_remove (NM_AGENT_MANAGER_GET_PRIVATE (user_data)->requests, - GUINT_TO_POINTER (req->reqid)); -} - guint32 nm_agent_manager_delete_secrets (NMAgentManager *self, NMConnection *connection) @@ -1432,12 +1359,7 @@ nm_agent_manager_delete_secrets (NMAgentManager *self, req = request_new (self, REQUEST_TYPE_CON_DEL, nm_connection_get_id (connection), - "deleting", - subject, - delete_complete_cb, - self, - delete_next_cb, - NULL); + subject); req->con.connection = g_object_ref (connection); g_object_unref (subject); g_hash_table_insert (priv->requests, GUINT_TO_POINTER (req->reqid), req); -- cgit v1.2.1 From 4ccae958025b8e555a7d3f9a1b18b8dae0deee65 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 2 Sep 2015 15:56:20 +0200 Subject: agent-manager/refact: replace get_start() by request_start() Let all implementations call request_start(), instead of getting-secrets doing something special and call get_start(). --- src/settings/nm-agent-manager.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/settings/nm-agent-manager.c b/src/settings/nm-agent-manager.c index 7b9656bd82..7a81abb332 100644 --- a/src/settings/nm-agent-manager.c +++ b/src/settings/nm-agent-manager.c @@ -136,6 +136,8 @@ static void get_next_cb (Request *req); static void save_next_cb (Request *req); static void delete_next_cb (Request *req); +static gboolean _con_get_try_complete_early (Request *req); + /*************************************************************/ static gboolean @@ -772,7 +774,18 @@ request_start (gpointer user_data) Request *req = user_data; req->idle_id = 0; + + switch (req->request_type) { + case REQUEST_TYPE_CON_GET: + if (_con_get_try_complete_early (req)) + goto out; + break; + default: + break; + } request_next_agent (req); + +out: return FALSE; } @@ -1064,18 +1077,14 @@ get_next_cb (Request *req) } static gboolean -get_start (gpointer user_data) +_con_get_try_complete_early (Request *req) { NMAgentManager *self; - Request *req = user_data; GVariant *setting_secrets = NULL; - - g_return_val_if_fail (req->request_type == REQUEST_TYPE_CON_GET, G_SOURCE_REMOVE); + gboolean completed = TRUE; self = req->self; - req->idle_id = 0; - /* Check if there are any existing secrets */ if (req->con.get.existing_secrets) setting_secrets = g_variant_lookup_value (req->con.get.existing_secrets, req->con.get.setting_name, NM_VARIANT_TYPE_SETTING); @@ -1116,7 +1125,7 @@ get_start (gpointer user_data) * don't error out if any secrets are missing. */ req_complete (req, req->con.get.existing_secrets, NULL, NULL, NULL); } else - request_next_agent (req); + completed = FALSE; } } g_object_unref (tmp); @@ -1125,13 +1134,13 @@ get_start (gpointer user_data) * agents for secrets. Let the Agent Manager handle which agents * we'll ask and in which order. */ - request_next_agent (req); + completed = FALSE; } if (setting_secrets) g_variant_unref (setting_secrets); - return FALSE; + return completed; } guint32 @@ -1186,7 +1195,7 @@ nm_agent_manager_get_secrets (NMAgentManager *self, /* Kick off the request */ if (!(req->con.get.flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_ONLY_SYSTEM)) request_add_agents (self, req); - req->idle_id = g_idle_add (get_start, req); + req->idle_id = g_idle_add (request_start, req); return req->reqid; } -- cgit v1.2.1 From 0205dc9f5aef657e83f237c6e047d02e2d71f7c9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 2 Sep 2015 16:06:44 +0200 Subject: agent-manager/trivial: rename functions --- src/settings/nm-agent-manager.c | 82 ++++++++++++++++++++--------------------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/src/settings/nm-agent-manager.c b/src/settings/nm-agent-manager.c index 7a81abb332..7b08d2e09c 100644 --- a/src/settings/nm-agent-manager.c +++ b/src/settings/nm-agent-manager.c @@ -132,9 +132,9 @@ static void request_remove_agent (Request *req, NMSecretAgent *agent, GSList **p static void request_next_agent (Request *req); -static void get_next_cb (Request *req); -static void save_next_cb (Request *req); -static void delete_next_cb (Request *req); +static void _con_get_request_start (Request *req); +static void _con_save_request_start (Request *req); +static void _con_del_request_start (Request *req); static gboolean _con_get_try_complete_early (Request *req); @@ -721,13 +721,13 @@ request_next_agent (Request *req) switch (req->request_type) { case REQUEST_TYPE_CON_GET: - get_next_cb (req); + _con_get_request_start (req); break; case REQUEST_TYPE_CON_SAVE: - save_next_cb (req); + _con_save_request_start (req); break; case REQUEST_TYPE_CON_DEL: - delete_next_cb (req); + _con_del_request_start (req); break; default: g_assert_not_reached (); @@ -792,11 +792,11 @@ out: /*************************************************************/ static void -get_done_cb (NMSecretAgent *agent, - NMSecretAgentCallId call_id, - GVariant *secrets, - GError *error, - gpointer user_data) +_con_get_request_done (NMSecretAgent *agent, + NMSecretAgentCallId call_id, + GVariant *secrets, + GError *error, + gpointer user_data) { NMAgentManager *self; Request *req = user_data; @@ -905,7 +905,7 @@ set_secrets_not_required (NMConnection *connection, GVariant *dict) } static void -get_agent_request_secrets (Request *req, gboolean include_system_secrets) +_con_get_request_start_proceed (Request *req, gboolean include_system_secrets) { NMConnection *tmp; @@ -930,7 +930,7 @@ get_agent_request_secrets (Request *req, gboolean include_system_secrets) req->con.get.setting_name, (const char **) req->con.get.hints, req->con.get.flags, - get_done_cb, + _con_get_request_done, req); if (!req->current_call_id) { g_warn_if_reached (); @@ -941,10 +941,10 @@ get_agent_request_secrets (Request *req, gboolean include_system_secrets) } static void -get_agent_modify_auth_cb (NMAuthChain *chain, - GError *error, - GDBusMethodInvocation *context, - gpointer user_data) +_con_get_request_start_validated (NMAuthChain *chain, + GError *error, + GDBusMethodInvocation *context, + gpointer user_data) { NMAgentManager *self; Request *req = user_data; @@ -976,18 +976,18 @@ get_agent_modify_auth_cb (NMAuthChain *chain, LOG_REQ_ARG (req), req->con.current_has_modify ? "YES" : "NO"); - get_agent_request_secrets (req, req->con.current_has_modify); + _con_get_request_start_proceed (req, req->con.current_has_modify); } nm_auth_chain_unref (chain); } static void -check_system_secrets_cb (NMSetting *setting, - const char *key, - const GValue *value, - GParamFlags flags, - gpointer user_data) +has_system_secrets_check (NMSetting *setting, + const char *key, + const GValue *value, + GParamFlags flags, + gpointer user_data) { NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE; gboolean *has_system = user_data; @@ -1021,12 +1021,12 @@ has_system_secrets (NMConnection *connection) { gboolean has_system = FALSE; - nm_connection_for_each_setting_value (connection, check_system_secrets_cb, &has_system); + nm_connection_for_each_setting_value (connection, has_system_secrets_check, &has_system); return has_system; } static void -get_next_cb (Request *req) +_con_get_request_start (Request *req) { NMAgentManager *self; NMSettingConnection *s_con; @@ -1051,7 +1051,7 @@ get_next_cb (Request *req) req->con.chain = nm_auth_chain_new_subject (nm_secret_agent_get_subject (req->current), NULL, - get_agent_modify_auth_cb, + _con_get_request_start_validated, req); g_assert (req->con.chain); @@ -1072,7 +1072,7 @@ get_next_cb (Request *req) _LOGD (NULL, "("LOG_REQ_FMT") requesting user-owned secrets from agent %s", LOG_REQ_ARG (req), agent_dbus_owner); - get_agent_request_secrets (req, FALSE); + _con_get_request_start_proceed (req, FALSE); } } @@ -1213,11 +1213,11 @@ nm_agent_manager_cancel_secrets (NMAgentManager *self, /*************************************************************/ static void -save_done_cb (NMSecretAgent *agent, - NMSecretAgentCallId call_id, - GVariant *secrets, - GError *error, - gpointer user_data) +_con_save_request_done (NMSecretAgent *agent, + NMSecretAgentCallId call_id, + GVariant *secrets, + GError *error, + gpointer user_data) { NMAgentManager *self; Request *req = user_data; @@ -1254,11 +1254,11 @@ save_done_cb (NMSecretAgent *agent, } static void -save_next_cb (Request *req) +_con_save_request_start (Request *req) { req->current_call_id = nm_secret_agent_save_secrets (req->current, req->con.connection, - save_done_cb, + _con_save_request_done, req); if (!req->current_call_id) { g_warn_if_reached (); @@ -1298,11 +1298,11 @@ nm_agent_manager_save_secrets (NMAgentManager *self, /*************************************************************/ static void -delete_done_cb (NMSecretAgent *agent, - NMSecretAgentCallId call_id, - GVariant *secrets, - GError *error, - gpointer user_data) +_con_del_request_done (NMSecretAgent *agent, + NMSecretAgentCallId call_id, + GVariant *secrets, + GError *error, + gpointer user_data) { NMAgentManager *self; Request *req = user_data; @@ -1336,11 +1336,11 @@ delete_done_cb (NMSecretAgent *agent, } static void -delete_next_cb (Request *req) +_con_del_request_start (Request *req) { req->current_call_id = nm_secret_agent_delete_secrets (req->current, req->con.connection, - delete_done_cb, + _con_del_request_done, req); if (!req->current_call_id) { g_warn_if_reached (); -- cgit v1.2.1 From 7a8eee99ff5b942c430b243850eb54131041b0bc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 3 Sep 2015 12:43:00 +0200 Subject: agent-manager/refact: return early from _con_get_try_complete_early() instead of if-else-if --- src/settings/nm-agent-manager.c | 92 +++++++++++++++++++---------------------- 1 file changed, 43 insertions(+), 49 deletions(-) diff --git a/src/settings/nm-agent-manager.c b/src/settings/nm-agent-manager.c index 7b08d2e09c..9780ea31a7 100644 --- a/src/settings/nm-agent-manager.c +++ b/src/settings/nm-agent-manager.c @@ -1080,8 +1080,9 @@ static gboolean _con_get_try_complete_early (Request *req) { NMAgentManager *self; - GVariant *setting_secrets = NULL; - gboolean completed = TRUE; + gs_unref_variant GVariant *setting_secrets = NULL; + gs_unref_object NMConnection *tmp = NULL; + GError *error = NULL; self = req->self; @@ -1089,58 +1090,51 @@ _con_get_try_complete_early (Request *req) if (req->con.get.existing_secrets) setting_secrets = g_variant_lookup_value (req->con.get.existing_secrets, req->con.get.setting_name, NM_VARIANT_TYPE_SETTING); - if (setting_secrets && g_variant_n_children (setting_secrets)) { - NMConnection *tmp; - GError *error = NULL; - gboolean new_secrets = (req->con.get.flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW); + if (!setting_secrets || !g_variant_n_children (setting_secrets)) + return FALSE; - /* The connection already had secrets; check if any more are required. - * If no more are required, we're done. If secrets are still needed, - * ask a secret agent for more. This allows admins to provide generic - * secrets but allow additional user-specific ones as well. - */ - tmp = nm_simple_connection_new_clone (req->con.connection); - g_assert (tmp); + /* The connection already had secrets; check if any more are required. + * If no more are required, we're done. If secrets are still needed, + * ask a secret agent for more. This allows admins to provide generic + * secrets but allow additional user-specific ones as well. + */ + tmp = nm_simple_connection_new_clone (req->con.connection); + g_assert (tmp); - if (!nm_connection_update_secrets (tmp, req->con.get.setting_name, req->con.get.existing_secrets, &error)) { - req_complete_error (req, error); - g_clear_error (&error); - } else { - /* Do we have everything we need? */ - if ( (req->con.get.flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_ONLY_SYSTEM) - || ((nm_connection_need_secrets (tmp, NULL) == NULL) && (new_secrets == FALSE))) { - _LOGD (NULL, "("LOG_REQ_FMT") system settings secrets sufficient", - LOG_REQ_ARG (req)); - - /* Got everything, we're done */ - req_complete (req, req->con.get.existing_secrets, NULL, NULL, NULL); - } else { - _LOGD (NULL, "("LOG_REQ_FMT") system settings secrets insufficient, asking agents", - LOG_REQ_ARG (req)); - - /* We don't, so ask some agents for additional secrets */ - if ( req->con.get.flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_NO_ERRORS - && !req->pending) { - /* The request initiated from GetSecrets() via DBus, - * don't error out if any secrets are missing. */ - req_complete (req, req->con.get.existing_secrets, NULL, NULL, NULL); - } else - completed = FALSE; - } - } - g_object_unref (tmp); - } else { - /* Couldn't get secrets from system settings, so now we ask the - * agents for secrets. Let the Agent Manager handle which agents - * we'll ask and in which order. - */ - completed = FALSE; + if (!nm_connection_update_secrets (tmp, req->con.get.setting_name, req->con.get.existing_secrets, &error)) { + req_complete_error (req, error); + g_clear_error (&error); + return TRUE; + } + /* Do we have everything we need? */ + if ( NM_FLAGS_HAS (req->con.get.flags, NM_SECRET_AGENT_GET_SECRETS_FLAG_ONLY_SYSTEM) + || ( (nm_connection_need_secrets (tmp, NULL) == NULL) + && !NM_FLAGS_HAS(req->con.get.flags, NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW))) { + _LOGD (NULL, "("LOG_REQ_FMT") system settings secrets sufficient", + LOG_REQ_ARG (req)); + + /* Got everything, we're done */ + req_complete (req, req->con.get.existing_secrets, NULL, NULL, NULL); + return TRUE; } - if (setting_secrets) - g_variant_unref (setting_secrets); + _LOGD (NULL, "("LOG_REQ_FMT") system settings secrets insufficient, asking agents", + LOG_REQ_ARG (req)); + + /* We don't, so ask some agents for additional secrets */ + if ( req->con.get.flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_NO_ERRORS + && !req->pending) { + /* The request initiated from GetSecrets() via DBus, + * don't error out if any secrets are missing. */ + req_complete (req, req->con.get.existing_secrets, NULL, NULL, NULL); + return TRUE; + } - return completed; + /* Couldn't get secrets from system settings, so now we ask the + * agents for secrets. Let the Agent Manager handle which agents + * we'll ask and in which order. + */ + return FALSE; } guint32 -- cgit v1.2.1 From 40eda71dc655660753782c5fad56becf70f3a9df Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 2 Sep 2015 21:43:30 +0200 Subject: agent-manager: fix type of idle_id in Request structure --- src/settings/nm-agent-manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/settings/nm-agent-manager.c b/src/settings/nm-agent-manager.c index 9780ea31a7..afc7f328b4 100644 --- a/src/settings/nm-agent-manager.c +++ b/src/settings/nm-agent-manager.c @@ -452,7 +452,7 @@ struct _Request { /* Stores the sorted list of NMSecretAgents which will be asked for secrets */ GSList *pending; - guint32 idle_id; + guint idle_id; gboolean completed; -- cgit v1.2.1 From 21fd5fa0ab45ef1651c0e062e6c84c6d692161fa Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 2 Sep 2015 22:43:04 +0200 Subject: settings: refactor call_id type of async functions for NMAgentManager, NMSettingsConnection and NMActRequest Instead of having the call_id of type guint32, make it an (opaque) pointer type. This has the advantage of strong typing and avoids the possiblity of reusing an invalid integer (or overflow of the call-id counter). OTOH, it has the disadvantage, that after a call_id is disposed, it might be reused for future invocations (because malloc might reuse the memory). In fact, it is always an error to use a call_id that is already completed. This commit also adds assertions to the cancel() calls that the provided call_id is a pending call. Hence, such a bug will be uncovered by assertions (that only might not tigger in certain unlikely cases where a call-id got reused). Note that for NMAgentManager, save_secrets() and delete_secrets() both returned a call_id. But they didn't also provide a callback when the operation completes. So the user trying to cancel such a call, cannot know whether the operation is still in process and he cannot avoid triggering an assertion. Fix that by not returning a call-id for these operations. No caller cared about it anyway. For NMSettingsConnection, also track the internally scheduled requests for so that we can cancel them on dispose. --- src/devices/nm-device-ethernet.c | 2 +- src/devices/wifi/nm-device-wifi.c | 2 +- src/devices/wwan/nm-modem.c | 8 +- src/nm-activation-request.c | 55 +++++++------ src/nm-activation-request.h | 19 +++-- src/ppp-manager/nm-ppp-manager.c | 8 +- src/settings/nm-agent-manager.c | 54 ++++++------- src/settings/nm-agent-manager.h | 39 ++++----- src/settings/nm-settings-connection.c | 145 +++++++++++++++++++++------------- src/settings/nm-settings-connection.h | 23 +++--- src/vpn-manager/nm-vpn-connection.c | 10 +-- 11 files changed, 202 insertions(+), 163 deletions(-) diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index 03c1d5109a..c2bbad62de 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -480,7 +480,7 @@ supplicant_interface_release (NMDeviceEthernet *self) static void wired_secrets_cb (NMActRequest *req, - guint32 call_id, + NMActRequestGetSecretsCallId call_id, NMConnection *connection, GError *error, gpointer user_data) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 807f7b9cca..dabf62cb3b 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -1674,7 +1674,7 @@ cleanup_association_attempt (NMDeviceWifi *self, gboolean disconnect) static void wifi_secrets_cb (NMActRequest *req, - guint32 call_id, + NMActRequestGetSecretsCallId call_id, NMConnection *connection, GError *error, gpointer user_data) diff --git a/src/devices/wwan/nm-modem.c b/src/devices/wwan/nm-modem.c index d6433a085d..16ca844642 100644 --- a/src/devices/wwan/nm-modem.c +++ b/src/devices/wwan/nm-modem.c @@ -73,7 +73,7 @@ typedef struct { NMActRequest *act_request; guint32 secrets_tries; - guint32 secrets_id; + NMActRequestGetSecretsCallId secrets_id; guint32 mm_ip_timeout; @@ -724,13 +724,13 @@ cancel_get_secrets (NMModem *self) if (priv->secrets_id) { nm_act_request_cancel_secrets (priv->act_request, priv->secrets_id); - priv->secrets_id = 0; + priv->secrets_id = NULL; } } static void modem_secrets_cb (NMActRequest *req, - guint32 call_id, + NMActRequestGetSecretsCallId call_id, NMConnection *connection, GError *error, gpointer user_data) @@ -740,7 +740,7 @@ modem_secrets_cb (NMActRequest *req, g_return_if_fail (call_id == priv->secrets_id); - priv->secrets_id = 0; + priv->secrets_id = NULL; if (error) nm_log_warn (LOGD_MB, "(%s): %s", nm_modem_get_uid (self), error->message); diff --git a/src/nm-activation-request.c b/src/nm-activation-request.c index 3ced2a3dcf..2d12b760e3 100644 --- a/src/nm-activation-request.c +++ b/src/nm-activation-request.c @@ -75,16 +75,18 @@ nm_act_request_get_connection (NMActRequest *req) /*******************************************************************/ -typedef struct { +struct _NMActRequestGetSecretsCallId { NMActRequest *self; - guint32 call_id; + NMSettingsConnectionCallId call_id_s; NMActRequestSecretsFunc callback; gpointer callback_data; -} GetSecretsInfo; +}; + +typedef struct _NMActRequestGetSecretsCallId GetSecretsInfo; static void get_secrets_cb (NMSettingsConnection *connection, - guint32 call_id, + NMSettingsConnectionCallId call_id_s, const char *agent_username, const char *setting_name, GError *error, @@ -93,14 +95,14 @@ get_secrets_cb (NMSettingsConnection *connection, GetSecretsInfo *info = user_data; NMActRequestPrivate *priv = NM_ACT_REQUEST_GET_PRIVATE (info->self); - g_return_if_fail (info->call_id == call_id); + g_return_if_fail (info->call_id_s == call_id_s); priv->secrets_calls = g_slist_remove (priv->secrets_calls, info); - info->callback (info->self, call_id, NM_CONNECTION (connection), error, info->callback_data); + info->callback (info->self, info, NM_CONNECTION (connection), error, info->callback_data); g_free (info); } -guint32 +NMActRequestGetSecretsCallId nm_act_request_get_secrets (NMActRequest *self, const char *setting_name, NMSecretAgentGetSecretsFlags flags, @@ -110,7 +112,7 @@ nm_act_request_get_secrets (NMActRequest *self, { NMActRequestPrivate *priv; GetSecretsInfo *info; - guint32 call_id; + NMSettingsConnectionCallId call_id_s; NMConnection *connection; const char *hints[2] = { hint, NULL }; @@ -128,25 +130,25 @@ nm_act_request_get_secrets (NMActRequest *self, flags |= NM_SECRET_AGENT_GET_SECRETS_FLAG_USER_REQUESTED; connection = nm_active_connection_get_connection (NM_ACTIVE_CONNECTION (self)); - call_id = nm_settings_connection_get_secrets (NM_SETTINGS_CONNECTION (connection), - nm_active_connection_get_subject (NM_ACTIVE_CONNECTION (self)), - setting_name, - flags, - hints, - get_secrets_cb, - info, - NULL); - if (call_id > 0) { - info->call_id = call_id; + call_id_s = nm_settings_connection_get_secrets (NM_SETTINGS_CONNECTION (connection), + nm_active_connection_get_subject (NM_ACTIVE_CONNECTION (self)), + setting_name, + flags, + hints, + get_secrets_cb, + info, + NULL); + if (call_id_s) { + info->call_id_s = call_id_s; priv->secrets_calls = g_slist_append (priv->secrets_calls, info); } else g_free (info); - return call_id; + return info; } void -nm_act_request_cancel_secrets (NMActRequest *self, guint32 call_id) +nm_act_request_cancel_secrets (NMActRequest *self, NMActRequestGetSecretsCallId call_id) { NMActRequestPrivate *priv; NMConnection *connection; @@ -154,24 +156,25 @@ nm_act_request_cancel_secrets (NMActRequest *self, guint32 call_id) g_return_if_fail (self); g_return_if_fail (NM_IS_ACT_REQUEST (self)); - g_return_if_fail (call_id > 0); + g_return_if_fail (call_id); priv = NM_ACT_REQUEST_GET_PRIVATE (self); - connection = nm_active_connection_get_connection (NM_ACTIVE_CONNECTION (self)); for (iter = priv->secrets_calls; iter; iter = g_slist_next (iter)) { GetSecretsInfo *info = iter->data; /* Remove the matching info */ - if (info->call_id == call_id) { + if (info == call_id) { priv->secrets_calls = g_slist_remove_link (priv->secrets_calls, iter); g_slist_free (iter); - nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (connection), call_id); + connection = nm_active_connection_get_connection (NM_ACTIVE_CONNECTION (self)); + nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (connection), info->call_id_s); g_free (info); - break; + return; } } + g_return_if_reached (); } /********************************************************************/ @@ -444,7 +447,7 @@ dispose (GObject *object) for (iter = priv->secrets_calls; connection && iter; iter = g_slist_next (iter)) { GetSecretsInfo *info = iter->data; - nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (connection), info->call_id); + nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (connection), info->call_id_s); g_free (info); } g_slist_free (priv->secrets_calls); diff --git a/src/nm-activation-request.h b/src/nm-activation-request.h index b93fff5b72..8c990d520f 100644 --- a/src/nm-activation-request.h +++ b/src/nm-activation-request.h @@ -33,6 +33,9 @@ #define NM_IS_ACT_REQUEST_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), NM_TYPE_ACT_REQUEST)) #define NM_ACT_REQUEST_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_ACT_REQUEST, NMActRequestClass)) +struct _NMActRequestGetSecretsCallId; +typedef struct _NMActRequestGetSecretsCallId *NMActRequestGetSecretsCallId; + struct _NMActRequest { NMActiveConnection parent; }; @@ -62,19 +65,19 @@ void nm_act_request_add_share_rule (NMActRequest *req, /* Secrets handling */ typedef void (*NMActRequestSecretsFunc) (NMActRequest *req, - guint32 call_id, + NMActRequestGetSecretsCallId call_id, NMConnection *connection, GError *error, gpointer user_data); -guint32 nm_act_request_get_secrets (NMActRequest *req, - const char *setting_name, - NMSecretAgentGetSecretsFlags flags, - const char *hint, - NMActRequestSecretsFunc callback, - gpointer callback_data); +NMActRequestGetSecretsCallId nm_act_request_get_secrets (NMActRequest *req, + const char *setting_name, + NMSecretAgentGetSecretsFlags flags, + const char *hint, + NMActRequestSecretsFunc callback, + gpointer callback_data); -void nm_act_request_cancel_secrets (NMActRequest *req, guint32 call_id); +void nm_act_request_cancel_secrets (NMActRequest *req, NMActRequestGetSecretsCallId call_id); #endif /* __NETWORKMANAGER_ACTIVATION_REQUEST_H__ */ diff --git a/src/ppp-manager/nm-ppp-manager.c b/src/ppp-manager/nm-ppp-manager.c index dcc916902a..a37d42ff61 100644 --- a/src/ppp-manager/nm-ppp-manager.c +++ b/src/ppp-manager/nm-ppp-manager.c @@ -63,7 +63,7 @@ typedef struct { NMActRequest *act_req; GDBusMethodInvocation *pending_secrets_context; - guint32 secrets_id; + NMActRequestGetSecretsCallId secrets_id; const char *secrets_setting_name; guint32 ppp_watch_id; @@ -243,7 +243,7 @@ cancel_get_secrets (NMPPPManager *self) if (priv->secrets_id) { nm_act_request_cancel_secrets (priv->act_req, priv->secrets_id); - priv->secrets_id = 0; + priv->secrets_id = NULL; } priv->secrets_setting_name = NULL; } @@ -309,7 +309,7 @@ extract_details_from_connection (NMConnection *connection, static void ppp_secrets_cb (NMActRequest *req, - guint32 call_id, + NMActRequestGetSecretsCallId call_id, NMConnection *connection, GError *error, gpointer user_data) @@ -348,7 +348,7 @@ ppp_secrets_cb (NMActRequest *req, out: priv->pending_secrets_context = NULL; - priv->secrets_id = 0; + priv->secrets_id = NULL; priv->secrets_setting_name = NULL; } diff --git a/src/settings/nm-agent-manager.c b/src/settings/nm-agent-manager.c index afc7f328b4..f4d25f2647 100644 --- a/src/settings/nm-agent-manager.c +++ b/src/settings/nm-agent-manager.c @@ -124,7 +124,7 @@ enum { static guint signals[LAST_SIGNAL] = { 0 }; -typedef struct _Request Request; +typedef struct _NMAgentManagerCallId Request; static void request_add_agent (Request *req, NMSecretAgent *agent); @@ -160,7 +160,7 @@ remove_agent (NMAgentManager *self, const char *owner) /* Remove this agent from any in-progress secrets requests */ g_hash_table_iter_init (&iter, priv->requests); - while (g_hash_table_iter_next (&iter, NULL, &data)) + while (g_hash_table_iter_next (&iter, &data, NULL)) request_remove_agent ((Request *) data, agent, &pending_reqs); /* We cannot call request_next_agent() from from within hash iterating loop, @@ -290,7 +290,7 @@ agent_register_permissions_done (NMAuthChain *chain, /* Add this agent to any in-progress secrets requests */ g_hash_table_iter_init (&iter, priv->requests); - while (g_hash_table_iter_next (&iter, NULL, (gpointer) &req)) + while (g_hash_table_iter_next (&iter, (gpointer) &req, NULL)) request_add_agent (req, agent); } @@ -435,12 +435,11 @@ done: /*************************************************************/ -struct _Request { +struct _NMAgentManagerCallId { NMAgentManager *self; RequestType request_type; - guint32 reqid; char *detail; NMAuthSubject *subject; @@ -485,8 +484,6 @@ struct _Request { }; }; -static guint32 next_req_id = 1; - static Request * request_new (NMAgentManager *self, RequestType request_type, @@ -498,7 +495,6 @@ request_new (NMAgentManager *self, req = g_slice_new0 (Request); req->self = g_object_ref (self); req->request_type = request_type; - req->reqid = next_req_id++; req->detail = g_strdup (detail); req->subject = g_object_ref (subject); return req; @@ -569,7 +565,7 @@ req_complete (Request *req, switch (req->request_type) { case REQUEST_TYPE_CON_GET: req->con.get.callback (self, - req->reqid, + req, agent_dbus_owner, agent_username, req->con.current_has_modify, @@ -589,7 +585,7 @@ req_complete (Request *req, g_return_if_reached (); } - g_hash_table_remove (priv->requests, GUINT_TO_POINTER (req->reqid)); + g_hash_table_remove (priv->requests, req); } static void @@ -1137,7 +1133,7 @@ _con_get_try_complete_early (Request *req) return FALSE; } -guint32 +NMAgentManagerCallId nm_agent_manager_get_secrets (NMAgentManager *self, NMConnection *connection, NMAuthSubject *subject, @@ -1184,24 +1180,26 @@ nm_agent_manager_get_secrets (NMAgentManager *self, req->con.get.other_data2 = other_data2; req->con.get.other_data3 = other_data3; - g_hash_table_insert (priv->requests, GUINT_TO_POINTER (req->reqid), req); + g_hash_table_add (priv->requests, req); /* Kick off the request */ if (!(req->con.get.flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_ONLY_SYSTEM)) request_add_agents (self, req); req->idle_id = g_idle_add (request_start, req); - return req->reqid; + return req; } void nm_agent_manager_cancel_secrets (NMAgentManager *self, - guint32 request_id) + NMAgentManagerCallId request_id) { g_return_if_fail (self != NULL); - g_return_if_fail (request_id > 0); + g_return_if_fail (request_id); + g_return_if_fail (request_id->request_type == REQUEST_TYPE_CON_GET); - g_hash_table_remove (NM_AGENT_MANAGER_GET_PRIVATE (self)->requests, - GUINT_TO_POINTER (request_id)); + if (!g_hash_table_remove (NM_AGENT_MANAGER_GET_PRIVATE (self)->requests, + request_id)) + g_return_if_reached (); } /*************************************************************/ @@ -1260,7 +1258,7 @@ _con_save_request_start (Request *req) } } -guint32 +void nm_agent_manager_save_secrets (NMAgentManager *self, NMConnection *connection, NMAuthSubject *subject) @@ -1268,8 +1266,8 @@ nm_agent_manager_save_secrets (NMAgentManager *self, NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self); Request *req; - g_return_val_if_fail (self != NULL, 0); - g_return_val_if_fail (NM_IS_CONNECTION (connection), 0); + g_return_if_fail (self); + g_return_if_fail (NM_IS_CONNECTION (connection)); nm_log_dbg (LOGD_SETTINGS, "Saving secrets for connection %s (%s)", @@ -1281,12 +1279,11 @@ nm_agent_manager_save_secrets (NMAgentManager *self, nm_connection_get_id (connection), subject); req->con.connection = g_object_ref (connection); - g_hash_table_insert (priv->requests, GUINT_TO_POINTER (req->reqid), req); + g_hash_table_add (priv->requests, req); /* Kick off the request */ request_add_agents (self, req); req->idle_id = g_idle_add (request_start, req); - return req->reqid; } /*************************************************************/ @@ -1342,7 +1339,7 @@ _con_del_request_start (Request *req) } } -guint32 +void nm_agent_manager_delete_secrets (NMAgentManager *self, NMConnection *connection) { @@ -1350,8 +1347,8 @@ nm_agent_manager_delete_secrets (NMAgentManager *self, NMAuthSubject *subject; Request *req; - g_return_val_if_fail (self != NULL, 0); - g_return_val_if_fail (NM_IS_CONNECTION (connection), 0); + g_return_if_fail (self != NULL); + g_return_if_fail (NM_IS_CONNECTION (connection)); nm_log_dbg (LOGD_SETTINGS, "Deleting secrets for connection %s (%s)", @@ -1365,12 +1362,11 @@ nm_agent_manager_delete_secrets (NMAgentManager *self, subject); req->con.connection = g_object_ref (connection); g_object_unref (subject); - g_hash_table_insert (priv->requests, GUINT_TO_POINTER (req->reqid), req); + g_hash_table_add (priv->requests, req); /* Kick off the request */ request_add_agents (self, req); req->idle_id = g_idle_add (request_start, req); - return req->reqid; } /*************************************************************/ @@ -1493,8 +1489,8 @@ nm_agent_manager_init (NMAgentManager *self) priv->agents = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_object_unref); priv->requests = g_hash_table_new_full (g_direct_hash, g_direct_equal, - NULL, - (GDestroyNotify) request_free); + (GDestroyNotify) request_free, + NULL); } static void diff --git a/src/settings/nm-agent-manager.h b/src/settings/nm-agent-manager.h index 792ee572d3..35564aa103 100644 --- a/src/settings/nm-agent-manager.h +++ b/src/settings/nm-agent-manager.h @@ -33,6 +33,9 @@ #define NM_IS_AGENT_MANAGER_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), NM_TYPE_AGENT_MANAGER)) #define NM_AGENT_MANAGER_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_AGENT_MANAGER, NMAgentManagerClass)) +struct _NMAgentManagerCallId; +typedef struct _NMAgentManagerCallId *NMAgentManagerCallId; + struct _NMAgentManager { NMExportedObject parent; }; @@ -50,7 +53,7 @@ NMAgentManager *nm_agent_manager_get (void); /* If no agent fulfilled the secrets request, agent_dbus_owner will be NULL */ typedef void (*NMAgentSecretsResultFunc) (NMAgentManager *manager, - guint32 call_id, + NMAgentManagerCallId call_id, const char *agent_dbus_owner, const char *agent_uname, gboolean agent_has_modify, @@ -62,27 +65,27 @@ typedef void (*NMAgentSecretsResultFunc) (NMAgentManager *manager, gpointer other_data2, gpointer other_data3); -guint32 nm_agent_manager_get_secrets (NMAgentManager *manager, - NMConnection *connection, - NMAuthSubject *subject, - GVariant *existing_secrets, - const char *setting_name, - NMSecretAgentGetSecretsFlags flags, - const char **hints, - NMAgentSecretsResultFunc callback, - gpointer callback_data, - gpointer other_data2, - gpointer other_data3); +NMAgentManagerCallId nm_agent_manager_get_secrets (NMAgentManager *manager, + NMConnection *connection, + NMAuthSubject *subject, + GVariant *existing_secrets, + const char *setting_name, + NMSecretAgentGetSecretsFlags flags, + const char **hints, + NMAgentSecretsResultFunc callback, + gpointer callback_data, + gpointer other_data2, + gpointer other_data3); void nm_agent_manager_cancel_secrets (NMAgentManager *manager, - guint32 request_id); + NMAgentManagerCallId request_id); -guint32 nm_agent_manager_save_secrets (NMAgentManager *manager, - NMConnection *connection, - NMAuthSubject *subject); +void nm_agent_manager_save_secrets (NMAgentManager *manager, + NMConnection *connection, + NMAuthSubject *subject); -guint32 nm_agent_manager_delete_secrets (NMAgentManager *manager, - NMConnection *connection); +void nm_agent_manager_delete_secrets (NMAgentManager *manager, + NMConnection *connection); NMSecretAgent *nm_agent_manager_get_agent_by_user (NMAgentManager *manager, const char *username); diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 8e0c2a7276..d9e9653dec 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -72,6 +72,18 @@ G_DEFINE_TYPE_WITH_CODE (NMSettingsConnection, nm_settings_connection, NM_TYPE_E NM_TYPE_SETTINGS_CONNECTION, \ NMSettingsConnectionPrivate)) +static inline NMAgentManagerCallId +NM_AGENT_MANAGER_CALL_ID (NMSettingsConnectionCallId call_id) +{ + return (NMAgentManagerCallId) call_id; +} + +static inline NMSettingsConnectionCallId +NM_SETTINGS_CONNECTION_CALL_ID (NMAgentManagerCallId call_id_a) +{ + return (NMSettingsConnectionCallId) call_id_a; +} + enum { PROP_0 = 0, PROP_VISIBLE, @@ -103,7 +115,9 @@ typedef struct { GSList *pending_auths; /* List of pending authentication requests */ gboolean visible; /* Is this connection is visible by some session? */ - GSList *reqs; /* in-progress secrets requests */ + + GSList *reqs_int; /* in-progress internal secrets requests */ + GSList *reqs_ext; /* in-progress external secrets requests (D-Bus) */ /* Caches secrets from on-disk connections; were they not cached any * call to nm_connection_clear_secrets() wipes them out and we'd have @@ -794,7 +808,7 @@ new_secrets_commit_cb (NMSettingsConnection *self, static void agent_secrets_done_cb (NMAgentManager *manager, - guint32 call_id, + NMAgentManagerCallId call_id_a, const char *agent_dbus_owner, const char *agent_username, gboolean agent_has_modify, @@ -806,6 +820,7 @@ agent_secrets_done_cb (NMAgentManager *manager, gpointer other_data2, gpointer other_data3) { + NMSettingsConnectionCallId call_id = NM_SETTINGS_CONNECTION_CALL_ID (call_id_a); NMSettingsConnection *self = NM_SETTINGS_CONNECTION (user_data); NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); NMSettingsConnectionSecretsFunc callback = other_data2; @@ -815,8 +830,10 @@ agent_secrets_done_cb (NMAgentManager *manager, gboolean agent_had_system = FALSE; ForEachSecretFlags cmp_flags = { NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_SECRET_FLAG_NONE }; + priv->reqs_int = g_slist_remove (priv->reqs_int, call_id_a); + if (error) { - _LOGD ("(%s:%u) secrets request error: (%d) %s", + _LOGD ("(%s:%p) secrets request error: (%d) %s", setting_name, call_id, error->code, @@ -837,7 +854,7 @@ agent_secrets_done_cb (NMAgentManager *manager, g_assert (secrets); if (agent_dbus_owner) { - _LOGD ("(%s:%u) secrets returned from agent %s", + _LOGD ("(%s:%p) secrets returned from agent %s", setting_name, call_id, agent_dbus_owner); @@ -854,7 +871,7 @@ agent_secrets_done_cb (NMAgentManager *manager, /* No user interaction was allowed when requesting secrets; the * agent is being bad. Remove system-owned secrets. */ - _LOGD ("(%s:%u) interaction forbidden but agent %s returned system secrets", + _LOGD ("(%s:%p) interaction forbidden but agent %s returned system secrets", setting_name, call_id, agent_dbus_owner); @@ -864,7 +881,7 @@ agent_secrets_done_cb (NMAgentManager *manager, /* Agent didn't successfully authenticate; clear system-owned secrets * from the secrets the agent returned. */ - _LOGD ("(%s:%u) agent failed to authenticate but provided system secrets", + _LOGD ("(%s:%p) agent failed to authenticate but provided system secrets", setting_name, call_id); @@ -872,12 +889,12 @@ agent_secrets_done_cb (NMAgentManager *manager, } } } else { - _LOGD ("(%s:%u) existing secrets returned", + _LOGD ("(%s:%p) existing secrets returned", setting_name, call_id); } - _LOGD ("(%s:%u) secrets request completed", + _LOGD ("(%s:%p) secrets request completed", setting_name, call_id); @@ -914,19 +931,19 @@ agent_secrets_done_cb (NMAgentManager *manager, * nothing has changed, since agent-owned secrets don't get saved here. */ if (agent_had_system) { - _LOGD ("(%s:%u) saving new secrets to backing storage", + _LOGD ("(%s:%p) saving new secrets to backing storage", setting_name, call_id); nm_settings_connection_commit_changes (self, NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE, new_secrets_commit_cb, NULL); } else { - _LOGD ("(%s:%u) new agent secrets processed", + _LOGD ("(%s:%p) new agent secrets processed", setting_name, call_id); } } else { - _LOGD ("(%s:%u) failed to update with agent secrets: (%d) %s", + _LOGD ("(%s:%p) failed to update with agent secrets: (%d) %s", setting_name, call_id, local ? local->code : -1, @@ -934,7 +951,7 @@ agent_secrets_done_cb (NMAgentManager *manager, } g_variant_unref (filtered_secrets); } else { - _LOGD ("(%s:%u) failed to update with existing secrets: (%d) %s", + _LOGD ("(%s:%p) failed to update with existing secrets: (%d) %s", setting_name, call_id, local ? local->code : -1, @@ -963,7 +980,7 @@ agent_secrets_done_cb (NMAgentManager *manager, * * Returns: a call ID which may be used to cancel the ongoing secrets request **/ -guint32 +NMSettingsConnectionCallId nm_settings_connection_get_secrets (NMSettingsConnection *self, NMAuthSubject *subject, const char *setting_name, @@ -975,7 +992,7 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self, { NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); GVariant *existing_secrets; - guint32 call_id = 0; + NMAgentManagerCallId call_id_a; gs_free char *joined_hints = NULL; /* Use priv->secrets to work around the fact that nm_connection_clear_secrets() @@ -985,7 +1002,7 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self, g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, "%s.%d - Internal error; secrets cache invalid.", __FILE__, __LINE__); - return 0; + return NULL; } /* Make sure the request actually requests something we can return */ @@ -993,46 +1010,50 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self, g_set_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_SETTING_NOT_FOUND, "%s.%d - Connection didn't have requested setting '%s'.", __FILE__, __LINE__, setting_name); - return 0; + return NULL; } existing_secrets = nm_connection_to_dbus (priv->system_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS); if (existing_secrets) g_variant_ref_sink (existing_secrets); - call_id = nm_agent_manager_get_secrets (priv->agent_mgr, - NM_CONNECTION (self), - subject, - existing_secrets, - setting_name, - flags, - hints, - agent_secrets_done_cb, - self, - callback, - callback_data); + call_id_a = nm_agent_manager_get_secrets (priv->agent_mgr, + NM_CONNECTION (self), + subject, + existing_secrets, + setting_name, + flags, + hints, + agent_secrets_done_cb, + self, + callback, + callback_data); if (existing_secrets) g_variant_unref (existing_secrets); - _LOGD ("(%s:%u) secrets requested flags 0x%X hints '%s'", + _LOGD ("(%s:%p) secrets requested flags 0x%X hints '%s'", setting_name, - call_id, + call_id_a, flags, (hints && hints[0]) ? (joined_hints = g_strjoinv (",", (char **) hints)) : "(none)"); - return call_id; + priv->reqs_int = g_slist_append (priv->reqs_int, call_id_a); + + return NM_SETTINGS_CONNECTION_CALL_ID (call_id_a); } void nm_settings_connection_cancel_secrets (NMSettingsConnection *self, - guint32 call_id) + NMSettingsConnectionCallId call_id) { NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); - _LOGD ("(%u) secrets canceled", - call_id); + if (!g_slist_find (priv->reqs_int, call_id)) + g_return_if_reached (); + + _LOGD ("(%p) secrets canceled", call_id); - priv->reqs = g_slist_remove (priv->reqs, GUINT_TO_POINTER (call_id)); - nm_agent_manager_cancel_secrets (priv->agent_mgr, call_id); + priv->reqs_int = g_slist_remove (priv->reqs_int, call_id); + nm_agent_manager_cancel_secrets (priv->agent_mgr, NM_AGENT_MANAGER_CALL_ID (call_id)); } /**** User authorization **************************************/ @@ -1606,7 +1627,7 @@ impl_settings_connection_delete (NMSettingsConnection *self, { NMAuthSubject *subject = NULL; GError *error = NULL; - + if (!check_writable (NM_CONNECTION (self), &error)) goto out_err; @@ -1627,7 +1648,7 @@ out_err: static void dbus_get_agent_secrets_cb (NMSettingsConnection *self, - guint32 call_id, + NMSettingsConnectionCallId call_id, const char *agent_username, const char *setting_name, GError *error, @@ -1637,7 +1658,7 @@ dbus_get_agent_secrets_cb (NMSettingsConnection *self, GDBusMethodInvocation *context = user_data; GVariant *dict; - priv->reqs = g_slist_remove (priv->reqs, GUINT_TO_POINTER (call_id)); + priv->reqs_ext = g_slist_remove (priv->reqs_ext, call_id); if (error) g_dbus_method_invocation_return_gerror (context, error); @@ -1655,7 +1676,7 @@ dbus_get_agent_secrets_cb (NMSettingsConnection *self, } static void -dbus_get_secrets_auth_cb (NMSettingsConnection *self, +dbus_get_secrets_auth_cb (NMSettingsConnection *self, GDBusMethodInvocation *context, NMAuthSubject *subject, GError *error, @@ -1663,22 +1684,22 @@ dbus_get_secrets_auth_cb (NMSettingsConnection *self, { NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); char *setting_name = user_data; - guint32 call_id = 0; + NMSettingsConnectionCallId call_id; GError *local = NULL; if (!error) { call_id = nm_settings_connection_get_secrets (self, - subject, - setting_name, - NM_SECRET_AGENT_GET_SECRETS_FLAG_USER_REQUESTED - | NM_SECRET_AGENT_GET_SECRETS_FLAG_NO_ERRORS, - NULL, - dbus_get_agent_secrets_cb, - context, - &local); - if (call_id > 0) { + subject, + setting_name, + NM_SECRET_AGENT_GET_SECRETS_FLAG_USER_REQUESTED + | NM_SECRET_AGENT_GET_SECRETS_FLAG_NO_ERRORS, + NULL, + dbus_get_agent_secrets_cb, + context, + &local); + if (call_id) { /* track the request and wait for the callback */ - priv->reqs = g_slist_append (priv->reqs, GUINT_TO_POINTER (call_id)); + priv->reqs_ext = g_slist_append (priv->reqs_ext, call_id); } } @@ -2334,15 +2355,31 @@ constructed (GObject *object) G_OBJECT_CLASS (nm_settings_connection_parent_class)->constructed (object); } +static void +_cancel_all (NMAgentManager *agent_mgr, GSList **preqs) +{ + while (*preqs) { + NMAgentManagerCallId call_id_a = (*preqs)->data; + + *preqs = g_slist_delete_link (*preqs, *preqs); + nm_agent_manager_cancel_secrets (agent_mgr, call_id_a); + } +} + static void dispose (GObject *object) { NMSettingsConnection *self = NM_SETTINGS_CONNECTION (object); NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); - GSList *iter; _LOGD ("disposing"); + /* Cancel in-progress secrets requests */ + if (priv->agent_mgr) { + _cancel_all (priv->agent_mgr, &priv->reqs_ext); + _cancel_all (priv->agent_mgr, &priv->reqs_int); + } + if (priv->updated_idle_id) { g_source_remove (priv->updated_idle_id); priv->updated_idle_id = 0; @@ -2363,12 +2400,6 @@ dispose (GObject *object) g_slist_free_full (priv->pending_auths, (GDestroyNotify) nm_auth_chain_unref); priv->pending_auths = NULL; - /* Cancel in-progress secrets requests */ - for (iter = priv->reqs; iter; iter = g_slist_next (iter)) - nm_agent_manager_cancel_secrets (priv->agent_mgr, GPOINTER_TO_UINT (iter->data)); - g_slist_free (priv->reqs); - priv->reqs = NULL; - g_clear_pointer (&priv->seen_bssids, (GDestroyNotify) g_hash_table_destroy); set_visible (self, FALSE); diff --git a/src/settings/nm-settings-connection.h b/src/settings/nm-settings-connection.h index b4b48271ba..588a99af53 100644 --- a/src/settings/nm-settings-connection.h +++ b/src/settings/nm-settings-connection.h @@ -85,6 +85,9 @@ typedef enum { /*< skip >*/ NM_SETTINGS_CONNECTION_COMMIT_REASON_ID_CHANGED = (1LL << 1), } NMSettingsConnectionCommitReason; +struct _NMSettingsConnectionCallId; +typedef struct _NMSettingsConnectionCallId *NMSettingsConnectionCallId; + typedef struct _NMSettingsConnectionClass NMSettingsConnectionClass; typedef void (*NMSettingsConnectionCommitFunc) (NMSettingsConnection *self, @@ -144,23 +147,23 @@ void nm_settings_connection_delete (NMSettingsConnection *self, gpointer user_data); typedef void (*NMSettingsConnectionSecretsFunc) (NMSettingsConnection *self, - guint32 call_id, + NMSettingsConnectionCallId call_id, const char *agent_username, const char *setting_name, GError *error, gpointer user_data); -guint32 nm_settings_connection_get_secrets (NMSettingsConnection *self, - NMAuthSubject *subject, - const char *setting_name, - NMSecretAgentGetSecretsFlags flags, - const char **hints, - NMSettingsConnectionSecretsFunc callback, - gpointer callback_data, - GError **error); +NMSettingsConnectionCallId nm_settings_connection_get_secrets (NMSettingsConnection *self, + NMAuthSubject *subject, + const char *setting_name, + NMSecretAgentGetSecretsFlags flags, + const char **hints, + NMSettingsConnectionSecretsFunc callback, + gpointer callback_data, + GError **error); void nm_settings_connection_cancel_secrets (NMSettingsConnection *self, - guint32 call_id); + NMSettingsConnectionCallId call_id); gboolean nm_settings_connection_is_visible (NMSettingsConnection *self); diff --git a/src/vpn-manager/nm-vpn-connection.c b/src/vpn-manager/nm-vpn-connection.c index 4f84e18531..04d27e1244 100644 --- a/src/vpn-manager/nm-vpn-connection.c +++ b/src/vpn-manager/nm-vpn-connection.c @@ -80,7 +80,7 @@ typedef struct { gboolean service_can_persist; gboolean connection_can_persist; - guint32 secrets_id; + NMSettingsConnectionCallId secrets_id; SecretsReq secrets_idx; char *username; @@ -334,7 +334,7 @@ _set_vpn_state (NMVpnConnection *connection, /* Clear any in-progress secrets request */ if (priv->secrets_id) { nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (priv->connection), priv->secrets_id); - priv->secrets_id = 0; + priv->secrets_id = NULL; } dispatcher_cleanup (connection); @@ -2009,7 +2009,7 @@ plugin_new_secrets_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_da static void get_secrets_cb (NMSettingsConnection *connection, - guint32 call_id, + NMSettingsConnectionCallId call_id, const char *agent_username, const char *setting_name, GError *error, @@ -2022,7 +2022,7 @@ get_secrets_cb (NMSettingsConnection *connection, g_return_if_fail (NM_CONNECTION (connection) == priv->connection); g_return_if_fail (call_id == priv->secrets_id); - priv->secrets_id = 0; + priv->secrets_id = NULL; if (error && priv->secrets_idx >= SECRETS_REQ_NEW) { nm_log_err (LOGD_VPN, "Failed to request VPN secrets #%d: (%d) %s", @@ -2227,7 +2227,7 @@ dispose (GObject *object) if (priv->secrets_id) { nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (priv->connection), priv->secrets_id); - priv->secrets_id = 0; + priv->secrets_id = NULL; } if (priv->cancellable) { -- cgit v1.2.1 From afb37d706f2cf63ebc9b5b3f92dc7a0163f2dc1b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 3 Sep 2015 10:22:29 +0200 Subject: secret-agent/trivial: add code comment --- src/settings/nm-secret-agent.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/settings/nm-secret-agent.c b/src/settings/nm-secret-agent.c index d77f5e0e29..ca30d46211 100644 --- a/src/settings/nm-secret-agent.c +++ b/src/settings/nm-secret-agent.c @@ -446,6 +446,17 @@ do_cancel_secrets (NMSecretAgent *self, Request *r, gboolean disposing) } } +/** + * nm_secret_agent_cancel_secrets: + * @self: #NMSecretAgent instance + * @call_id: the call id to cancel + * + * It is an error to pass an invalid @call_id or a @call_id for an operation + * that already completed. NMSecretAgent will always invoke the callback, + * also for cancel() and dispose(). + * In case of nm_secret_agent_cancel_secrets() this will synchronously invoke the + * callback before nm_secret_agent_cancel_secrets() returns. + */ void nm_secret_agent_cancel_secrets (NMSecretAgent *self, NMSecretAgentCallId call_id) { -- cgit v1.2.1 From 1b5664fed4ee40f9b7da7da88fd6b58c6b8a9bd0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 3 Sep 2015 10:24:09 +0200 Subject: agent-manager: always invoke complete function for asynchronous nm_agent_manager_get_secrets() Refactor agent-manager to always invoke the complete function for nm_agent_manager_get_secrets(). In general, the complete function is always invoked asnychronously when starting the operation. On the other hand, when cancelling the operation or disposing the manager with pending operations, we now (always) synchronously invoke the callback. This makes it simpler for the user to reliably cancel the request and perform potential cleanup. This behavior bubbles up through NMSettingsConnection and NMActRequest, and other callers that make directly or indicrectly make use of nm_agent_manager_get_secrets(). --- src/devices/nm-device-ethernet.c | 4 +- src/devices/wifi/nm-device-wifi.c | 4 +- src/devices/wwan/nm-modem.c | 34 ++++------ src/devices/wwan/nm-modem.h | 8 +-- src/nm-activation-request.c | 53 ++++++++------- src/ppp-manager/nm-ppp-manager.c | 12 ++-- src/settings/nm-agent-manager.c | 119 +++++++++++++++++++++++++--------- src/settings/nm-settings-connection.c | 50 +++++++++++--- src/vpn-manager/nm-vpn-connection.c | 8 +-- 9 files changed, 190 insertions(+), 102 deletions(-) diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index c2bbad62de..3de26b48c1 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -488,7 +488,9 @@ wired_secrets_cb (NMActRequest *req, NMDeviceEthernet *self = NM_DEVICE_ETHERNET (user_data); NMDevice *dev = NM_DEVICE (self); - g_return_if_fail (req == nm_device_get_act_request (dev)); + if (req != nm_device_get_act_request (dev)) + return; + g_return_if_fail (nm_device_get_state (dev) == NM_DEVICE_STATE_NEED_AUTH); g_return_if_fail (nm_act_request_get_connection (req) == connection); diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index dabf62cb3b..c668e1c70f 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -1682,7 +1682,9 @@ wifi_secrets_cb (NMActRequest *req, NMDevice *device = NM_DEVICE (user_data); NMDeviceWifi *self = NM_DEVICE_WIFI (device); - g_return_if_fail (req == nm_device_get_act_request (device)); + if (req != nm_device_get_act_request (device)) + return; + g_return_if_fail (nm_device_get_state (device) == NM_DEVICE_STATE_NEED_AUTH); g_return_if_fail (nm_act_request_get_connection (req) == connection); diff --git a/src/devices/wwan/nm-modem.c b/src/devices/wwan/nm-modem.c index 16ca844642..71c2323983 100644 --- a/src/devices/wwan/nm-modem.c +++ b/src/devices/wwan/nm-modem.c @@ -722,10 +722,7 @@ cancel_get_secrets (NMModem *self) { NMModemPrivate *priv = NM_MODEM_GET_PRIVATE (self); - if (priv->secrets_id) { - nm_act_request_cancel_secrets (priv->act_request, priv->secrets_id); - priv->secrets_id = NULL; - } + nm_act_request_cancel_secrets (priv->act_request, priv->secrets_id); } static void @@ -742,13 +739,16 @@ modem_secrets_cb (NMActRequest *req, priv->secrets_id = NULL; + if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + if (error) nm_log_warn (LOGD_MB, "(%s): %s", nm_modem_get_uid (self), error->message); g_signal_emit (self, signals[AUTH_RESULT], 0, error); } -gboolean +void nm_modem_get_secrets (NMModem *self, const char *setting_name, gboolean request_new, @@ -767,10 +767,8 @@ nm_modem_get_secrets (NMModem *self, hint, modem_secrets_cb, self); - if (priv->secrets_id) - g_signal_emit (self, signals[AUTH_REQUESTED], 0); - - return !!(priv->secrets_id); + g_return_if_fail (priv->secrets_id); + g_signal_emit (self, signals[AUTH_REQUESTED], 0); } /*****************************************************************************/ @@ -790,8 +788,7 @@ nm_modem_act_stage1_prepare (NMModem *self, NMDeviceStateReason *reason) { NMModemPrivate *priv = NM_MODEM_GET_PRIVATE (self); - NMActStageReturn ret; - GPtrArray *hints = NULL; + gs_unref_ptrarray GPtrArray *hints = NULL; const char *setting_name = NULL; NMSecretAgentGetSecretsFlags flags = NM_SECRET_AGENT_GET_SECRETS_FLAG_ALLOW_INTERACTION; NMConnection *connection; @@ -820,18 +817,9 @@ nm_modem_act_stage1_prepare (NMModem *self, hints ? g_ptr_array_index (hints, 0) : NULL, modem_secrets_cb, self); - if (priv->secrets_id) { - g_signal_emit (self, signals[AUTH_REQUESTED], 0); - ret = NM_ACT_STAGE_RETURN_POSTPONE; - } else { - *reason = NM_DEVICE_STATE_REASON_NO_SECRETS; - ret = NM_ACT_STAGE_RETURN_FAILURE; - } - - if (hints) - g_ptr_array_free (hints, TRUE); - - return ret; + g_return_val_if_fail (priv->secrets_id, NM_ACT_STAGE_RETURN_FAILURE); + g_signal_emit (self, signals[AUTH_REQUESTED], 0); + return NM_ACT_STAGE_RETURN_POSTPONE; } /*****************************************************************************/ diff --git a/src/devices/wwan/nm-modem.h b/src/devices/wwan/nm-modem.h index 35b80ba15b..fffbeb3213 100644 --- a/src/devices/wwan/nm-modem.h +++ b/src/devices/wwan/nm-modem.h @@ -218,10 +218,10 @@ NMActStageReturn nm_modem_stage3_ip6_config_start (NMModem *modem, void nm_modem_ip4_pre_commit (NMModem *modem, NMDevice *device, NMIP4Config *config); -gboolean nm_modem_get_secrets (NMModem *modem, - const char *setting_name, - gboolean request_new, - const char *hint); +void nm_modem_get_secrets (NMModem *modem, + const char *setting_name, + gboolean request_new, + const char *hint); void nm_modem_deactivate (NMModem *modem, NMDevice *device); diff --git a/src/nm-activation-request.c b/src/nm-activation-request.c index 2d12b760e3..90590838c0 100644 --- a/src/nm-activation-request.c +++ b/src/nm-activation-request.c @@ -96,12 +96,34 @@ get_secrets_cb (NMSettingsConnection *connection, NMActRequestPrivate *priv = NM_ACT_REQUEST_GET_PRIVATE (info->self); g_return_if_fail (info->call_id_s == call_id_s); + g_return_if_fail (g_slist_find (priv->secrets_calls, info)); + priv->secrets_calls = g_slist_remove (priv->secrets_calls, info); - info->callback (info->self, info, NM_CONNECTION (connection), error, info->callback_data); + if (info->callback) + info->callback (info->self, info, NM_CONNECTION (connection), error, info->callback_data); g_free (info); } +/** + * nm_act_request_get_secrets: + * @self: + * @setting_name: + * @flags: + * @hint: + * @callback: + * @callback_data: + * + * Asnychronously starts the request for secrets. This function cannot + * fail. + * + * The return call-id can be used to cancel the request. You are + * only allowed to cancel a still pending operation (once). + * The callback will always be invoked once, even for canceling + * or disposing of NMActRequest. + * + * Returns: a call-id. + */ NMActRequestGetSecretsCallId nm_act_request_get_secrets (NMActRequest *self, const char *setting_name, @@ -152,7 +174,6 @@ nm_act_request_cancel_secrets (NMActRequest *self, NMActRequestGetSecretsCallId { NMActRequestPrivate *priv; NMConnection *connection; - GSList *iter; g_return_if_fail (self); g_return_if_fail (NM_IS_ACT_REQUEST (self)); @@ -160,21 +181,11 @@ nm_act_request_cancel_secrets (NMActRequest *self, NMActRequestGetSecretsCallId priv = NM_ACT_REQUEST_GET_PRIVATE (self); - for (iter = priv->secrets_calls; iter; iter = g_slist_next (iter)) { - GetSecretsInfo *info = iter->data; - - /* Remove the matching info */ - if (info == call_id) { - priv->secrets_calls = g_slist_remove_link (priv->secrets_calls, iter); - g_slist_free (iter); + if (g_slist_find (priv->secrets_calls, call_id)) + g_return_if_reached (); - connection = nm_active_connection_get_connection (NM_ACTIVE_CONNECTION (self)); - nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (connection), info->call_id_s); - g_free (info); - return; - } - } - g_return_if_reached (); + connection = nm_active_connection_get_connection (NM_ACTIVE_CONNECTION (self)); + nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (connection), call_id->call_id_s); } /********************************************************************/ @@ -434,7 +445,6 @@ dispose (GObject *object) { NMActRequestPrivate *priv = NM_ACT_REQUEST_GET_PRIVATE (object); NMConnection *connection; - GSList *iter; /* Clear any share rules */ if (priv->share_rules) { @@ -444,14 +454,13 @@ dispose (GObject *object) /* Kill any in-progress secrets requests */ connection = nm_active_connection_get_connection (NM_ACTIVE_CONNECTION (object)); - for (iter = priv->secrets_calls; connection && iter; iter = g_slist_next (iter)) { - GetSecretsInfo *info = iter->data; + while (priv->secrets_calls) { + GetSecretsInfo *info = priv->secrets_calls->data; nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (connection), info->call_id_s); - g_free (info); + + g_return_if_fail (!priv->secrets_calls || info != priv->secrets_calls->data); } - g_slist_free (priv->secrets_calls); - priv->secrets_calls = NULL; G_OBJECT_CLASS (nm_act_request_parent_class)->dispose (object); } diff --git a/src/ppp-manager/nm-ppp-manager.c b/src/ppp-manager/nm-ppp-manager.c index a37d42ff61..b5bb22c11d 100644 --- a/src/ppp-manager/nm-ppp-manager.c +++ b/src/ppp-manager/nm-ppp-manager.c @@ -229,7 +229,7 @@ static void remove_timeout_handler (NMPPPManager *manager) { NMPPPManagerPrivate *priv = NM_PPP_MANAGER_GET_PRIVATE (manager); - + if (priv->ppp_timeout_handler) { g_source_remove (priv->ppp_timeout_handler); priv->ppp_timeout_handler = 0; @@ -241,11 +241,10 @@ cancel_get_secrets (NMPPPManager *self) { NMPPPManagerPrivate *priv = NM_PPP_MANAGER_GET_PRIVATE (self); - if (priv->secrets_id) { + if (priv->secrets_id) nm_act_request_cancel_secrets (priv->act_req, priv->secrets_id); - priv->secrets_id = NULL; - } - priv->secrets_setting_name = NULL; + + g_return_if_fail (!priv->secrets_id && !priv->secrets_setting_name); } static gboolean @@ -324,6 +323,9 @@ ppp_secrets_cb (NMActRequest *req, g_return_if_fail (req == priv->act_req); g_return_if_fail (call_id == priv->secrets_id); + if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + goto out; + if (error) { nm_log_warn (LOGD_PPP, "%s", error->message); g_dbus_method_invocation_return_gerror (priv->pending_secrets_context, error); diff --git a/src/settings/nm-agent-manager.c b/src/settings/nm-agent-manager.c index f4d25f2647..8685f3b21f 100644 --- a/src/settings/nm-agent-manager.c +++ b/src/settings/nm-agent-manager.c @@ -453,8 +453,6 @@ struct _NMAgentManagerCallId { guint idle_id; - gboolean completed; - union { struct { NMConnection *connection; @@ -524,16 +522,11 @@ request_free (Request *req) if (req->idle_id) g_source_remove (req->idle_id); - if (!req->completed) { - switch (req->request_type) { - case REQUEST_TYPE_CON_GET: - req->con.current_has_modify = FALSE; - if (req->current && req->current_call_id) - nm_secret_agent_cancel_secrets (req->current, req->current_call_id); - break; - default: - break; - } + if (req->current && req->current_call_id) { + /* cancel-secrets invokes the done-callback synchronously -- in which case + * the handler just return. + * Hence, we can proceed to free @req... */ + nm_secret_agent_cancel_secrets (req->current, req->current_call_id); } g_object_unref (req->subject); @@ -551,16 +544,13 @@ request_free (Request *req) } static void -req_complete (Request *req, - GVariant *secrets, - const char *agent_dbus_owner, - const char *agent_username, - GError *error) +req_complete_release (Request *req, + GVariant *secrets, + const char *agent_dbus_owner, + const char *agent_username, + GError *error) { NMAgentManager *self = req->self; - NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self); - - req->completed = TRUE; switch (req->request_type) { case REQUEST_TYPE_CON_GET: @@ -585,7 +575,38 @@ req_complete (Request *req, g_return_if_reached (); } - g_hash_table_remove (priv->requests, req); + request_free (req); +} + +static void +req_complete_cancel (Request *req, + GQuark domain, + guint code, + const char *message) +{ + GError *error = NULL; + + nm_assert (req && req->self); + nm_assert (!g_hash_table_contains (NM_AGENT_MANAGER_GET_PRIVATE (req->self)->requests, req)); + + g_set_error_literal (&error, domain, code, message); + req_complete_release (req, NULL, NULL, NULL, error); + g_error_free (error); +} + +static void +req_complete (Request *req, + GVariant *secrets, + const char *agent_dbus_owner, + const char *agent_username, + GError *error) +{ + NMAgentManager *self = req->self; + NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self); + + if (!g_hash_table_remove (priv->requests, req)) + g_return_if_reached (); + req_complete_release (req, secrets, agent_dbus_owner, agent_username, error); } static void @@ -1133,6 +1154,29 @@ _con_get_try_complete_early (Request *req) return FALSE; } +/** + * nm_agent_manager_get_secrets: + * @self: + * @connection: + * @subject: + * @existing_secrets: + * @flags: + * @hints: + * @callback: + * @callback_data: + * @other_data2: + * @other_data3: + * + * Requests secrets for a connection. + * + * This function cannot fail. The callback will be invoked + * asynchrnously, but it will always be invoked exactly once. + * Even for cancellation and disposing of @self. In those latter + * cases, the callback is invoked synchrnously during the cancellation/ + * disposal. + * + * Returns: a call-id to cancel the call. + */ NMAgentManagerCallId nm_agent_manager_get_secrets (NMAgentManager *self, NMConnection *connection, @@ -1180,7 +1224,8 @@ nm_agent_manager_get_secrets (NMAgentManager *self, req->con.get.other_data2 = other_data2; req->con.get.other_data3 = other_data3; - g_hash_table_add (priv->requests, req); + if (!g_hash_table_add (priv->requests, req)) + g_assert_not_reached (); /* Kick off the request */ if (!(req->con.get.flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_ONLY_SYSTEM)) @@ -1200,6 +1245,8 @@ nm_agent_manager_cancel_secrets (NMAgentManager *self, if (!g_hash_table_remove (NM_AGENT_MANAGER_GET_PRIVATE (self)->requests, request_id)) g_return_if_reached (); + + req_complete_cancel (request_id, G_IO_ERROR, G_IO_ERROR_CANCELLED, "Request cancelled"); } /*************************************************************/ @@ -1279,7 +1326,8 @@ nm_agent_manager_save_secrets (NMAgentManager *self, nm_connection_get_id (connection), subject); req->con.connection = g_object_ref (connection); - g_hash_table_add (priv->requests, req); + if (!g_hash_table_add (priv->requests, req)) + g_assert_not_reached (); /* Kick off the request */ request_add_agents (self, req); @@ -1362,7 +1410,8 @@ nm_agent_manager_delete_secrets (NMAgentManager *self, subject); req->con.connection = g_object_ref (connection); g_object_unref (subject); - g_hash_table_add (priv->requests, req); + if (!g_hash_table_add (priv->requests, req)) + g_assert_not_reached (); /* Kick off the request */ request_add_agents (self, req); @@ -1487,10 +1536,7 @@ nm_agent_manager_init (NMAgentManager *self) NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self); priv->agents = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_object_unref); - priv->requests = g_hash_table_new_full (g_direct_hash, - g_direct_equal, - (GDestroyNotify) request_free, - NULL); + priv->requests = g_hash_table_new (g_direct_hash, g_direct_equal); } static void @@ -1517,6 +1563,19 @@ dispose (GObject *object) { NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (object); + if (priv->requests) { + GHashTableIter iter; + Request *req; + + g_hash_table_iter_init (&iter, priv->requests); + while (g_hash_table_iter_next (&iter, (gpointer *) &req, NULL)) { + g_hash_table_iter_remove (&iter); + req_complete_cancel (req, G_IO_ERROR, G_IO_ERROR_FAILED, "Disposing NMAgentManagerClass instance"); + } + g_hash_table_destroy (priv->requests); + priv->requests = NULL; + } + g_slist_free_full (priv->chains, (GDestroyNotify) nm_auth_chain_unref); priv->chains = NULL; @@ -1524,10 +1583,6 @@ dispose (GObject *object) g_hash_table_destroy (priv->agents); priv->agents = NULL; } - if (priv->requests) { - g_hash_table_destroy (priv->requests); - priv->requests = NULL; - } if (priv->auth_mgr) { g_signal_handlers_disconnect_by_func (priv->auth_mgr, diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index d9e9653dec..918b03ecfa 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -117,7 +117,12 @@ typedef struct { gboolean visible; /* Is this connection is visible by some session? */ GSList *reqs_int; /* in-progress internal secrets requests */ - GSList *reqs_ext; /* in-progress external secrets requests (D-Bus) */ + + GSList *reqs_ext; /* in-progress external secrets requests (D-Bus). Note that + this list contains NMSettingsConnectionCallId, vs. NMAgentManagerCallId. + So, we should for example cancel the reqs_ext with nm_settings_connection_cancel_secrets() + instead of nm_agent_manager_cancel_secrets(). + Actually, the difference doesn't matter, because both are indeed equivalent. */ /* Caches secrets from on-disk connections; were they not cached any * call to nm_connection_clear_secrets() wipes them out and we'd have @@ -806,6 +811,16 @@ new_secrets_commit_cb (NMSettingsConnection *self, } } +static void +_callback_nop (NMSettingsConnection *self, + NMSettingsConnectionCallId call_id, + const char *agent_username, + const char *setting_name, + GError *error, + gpointer user_data) +{ +} + static void agent_secrets_done_cb (NMAgentManager *manager, NMAgentManagerCallId call_id_a, @@ -830,14 +845,16 @@ agent_secrets_done_cb (NMAgentManager *manager, gboolean agent_had_system = FALSE; ForEachSecretFlags cmp_flags = { NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_SECRET_FLAG_NONE }; - priv->reqs_int = g_slist_remove (priv->reqs_int, call_id_a); + if (!callback) + callback = _callback_nop; + + g_return_if_fail (g_slist_find (priv->reqs_int, call_id)); + + priv->reqs_int = g_slist_remove (priv->reqs_int, call_id); if (error) { - _LOGD ("(%s:%p) secrets request error: (%d) %s", - setting_name, - call_id, - error->code, - error->message ? error->message : "(unknown)"); + _LOGD ("(%s:%p) secrets request error: %s", + setting_name, call_id, error->message); callback (self, call_id, NULL, setting_name, error, callback_data); return; @@ -978,7 +995,13 @@ agent_secrets_done_cb (NMAgentManager *manager, * Retrieves secrets from persistent storage and queries any secret agents for * additional secrets. * - * Returns: a call ID which may be used to cancel the ongoing secrets request + * With the returned call-id, the call can be cancelled. It is an error + * to cancel a call more then once or a call that already completed. + * The callback will always be invoked exactly once, also for cancellation + * and disposing of @self. In those latter cases, the callback will be invoked + * synchronously during cancellation/disposing. + * + * Returns: a call ID which may be used to cancel the ongoing secrets request. **/ NMSettingsConnectionCallId nm_settings_connection_get_secrets (NMSettingsConnection *self, @@ -1052,7 +1075,6 @@ nm_settings_connection_cancel_secrets (NMSettingsConnection *self, _LOGD ("(%p) secrets canceled", call_id); - priv->reqs_int = g_slist_remove (priv->reqs_int, call_id); nm_agent_manager_cancel_secrets (priv->agent_mgr, NM_AGENT_MANAGER_CALL_ID (call_id)); } @@ -1658,6 +1680,8 @@ dbus_get_agent_secrets_cb (NMSettingsConnection *self, GDBusMethodInvocation *context = user_data; GVariant *dict; + g_return_if_fail (g_slist_find (priv->reqs_ext, call_id)); + priv->reqs_ext = g_slist_remove (priv->reqs_ext, call_id); if (error) @@ -2361,8 +2385,11 @@ _cancel_all (NMAgentManager *agent_mgr, GSList **preqs) while (*preqs) { NMAgentManagerCallId call_id_a = (*preqs)->data; - *preqs = g_slist_delete_link (*preqs, *preqs); + /* Cancel will invoke the complete callback, which in + * turn deletes the current call-id from the list. */ nm_agent_manager_cancel_secrets (agent_mgr, call_id_a); + + g_return_if_fail (!*preqs || (call_id_a != (*preqs)->data)); } } @@ -2376,6 +2403,9 @@ dispose (GObject *object) /* Cancel in-progress secrets requests */ if (priv->agent_mgr) { + /* although @reqs_ext theoretically contains NMSettingsConnectionCallId, + * we still cancel it with nm_agent_manager_cancel_secrets() -- it is + * actually correct. */ _cancel_all (priv->agent_mgr, &priv->reqs_ext); _cancel_all (priv->agent_mgr, &priv->reqs_int); } diff --git a/src/vpn-manager/nm-vpn-connection.c b/src/vpn-manager/nm-vpn-connection.c index 04d27e1244..2bb4fa4678 100644 --- a/src/vpn-manager/nm-vpn-connection.c +++ b/src/vpn-manager/nm-vpn-connection.c @@ -332,10 +332,8 @@ _set_vpn_state (NMVpnConnection *connection, _state_to_ac_state (vpn_state)); /* Clear any in-progress secrets request */ - if (priv->secrets_id) { + if (priv->secrets_id) nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (priv->connection), priv->secrets_id); - priv->secrets_id = NULL; - } dispatcher_cleanup (connection); @@ -2024,6 +2022,9 @@ get_secrets_cb (NMSettingsConnection *connection, priv->secrets_id = NULL; + if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + if (error && priv->secrets_idx >= SECRETS_REQ_NEW) { nm_log_err (LOGD_VPN, "Failed to request VPN secrets #%d: (%d) %s", priv->secrets_idx + 1, error->code, error->message); @@ -2227,7 +2228,6 @@ dispose (GObject *object) if (priv->secrets_id) { nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (priv->connection), priv->secrets_id); - priv->secrets_id = NULL; } if (priv->cancellable) { -- cgit v1.2.1