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

1 files changed, 63 insertions, 44 deletions

diff --git a/src/nm-auth-utils.c b/src/nm-auth-utils.c
index 1ed01b6c8d..e9bee59508 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,12 +129,12 @@ 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;
@@ -165,7 +178,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,19 +190,15 @@ 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));
 	}
 }
 
@@ -201,8 +210,8 @@ nm_auth_chain_get_data_ulong (NMAuthChain *self, const char *tag)
 	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;
+	ptr = g_hash_table_lookup (self->data, tag);
+	return ptr ? *ptr : 0ul;
 }
 
 
@@ -232,30 +241,13 @@ nm_auth_chain_get_subject (NMAuthChain *self)
 NMAuthCallResult
 nm_auth_chain_get_result (NMAuthChain *self, const char *permission)
 {
+	gpointer result;
+
 	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);
+	result = g_hash_table_lookup (self->data, permission);
+	return result ? GPOINTER_TO_UINT (result) : NM_AUTH_CALL_RESULT_UNKNOWN;
 }
 
 static AuthCall *
@@ -263,10 +255,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 +266,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 +370,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 +403,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 +428,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 **************/