degesch: remove cyclic dependency

Between users and channels.

1 files changed, 35 insertions, 39 deletions

diff --git a/degesch.c b/degesch.c
index 12850ed..d624731 100644
--- a/degesch.c
+++ b/degesch.c
@@ -101,12 +101,6 @@ static struct config_item g_config_table[] =
 		return false;                                                          \
 	BLOCK_END
 
-struct user;
-static void user_unref (struct user *self);
-
-struct channel;
-static void channel_unref (struct channel *self);
-
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 
 // We need a few reference countable objects with support
@@ -144,8 +138,6 @@ struct user_channel
 {
 	LIST_HEADER (struct user_channel)
 
-	// FIXME: this should be a weak reference to destroy a cycle
-
 	struct channel *channel;            ///< Reference to channel
 };
 
@@ -159,7 +151,8 @@ user_channel_new (void)
 static void
 user_channel_destroy (struct user_channel *self)
 {
-	channel_unref (self->channel);
+	// The "channel" reference is weak and this object should get
+	// destroyed whenever the user stops being in the channel.
 	free (self);
 }
 
@@ -227,8 +220,10 @@ channel_user_destroy (struct channel_user *self)
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 
-// We keep references to channels in their users and buffers,
-// and weak references in the name lookup table.
+// We keep references to channels in their buffers,
+// and weak references in their users and the name lookup table.
+
+// XXX: this doesn't really have to be reference countable
 
 struct channel
 {
@@ -259,8 +254,8 @@ channel_destroy (struct channel *self)
 	free (self->name);
 	free (self->mode);
 	free (self->topic);
-	LIST_FOR_EACH (struct channel_user, iter, self->users)
-		channel_user_destroy (iter);
+	// Owner has to make sure we have no users by now
+	hard_assert (!self->users);
 	str_vector_free (&self->names_buf);
 	free (self);
 }
@@ -1328,10 +1323,30 @@ irc_get_or_make_user_buffer (struct app_context *ctx, const char *nickname)
 }
 
 static void
+irc_channel_unlink_user
+	(struct channel *channel, struct channel_user *channel_user)
+{
+	// First destroy the user's weak references to the channel
+	struct user *user = channel_user->user;
+	LIST_FOR_EACH (struct user_channel, iter, user->channels)
+		if (iter->channel == channel)
+		{
+			LIST_UNLINK (user->channels, iter);
+			user_channel_destroy (iter);
+		}
+
+	// Then just unlink the user from the channel
+	LIST_UNLINK (channel->users, channel_user);
+	channel_user_destroy (channel_user);
+}
+
+static void
 irc_channel_on_destroy (void *object, void *user_data)
 {
 	struct channel *channel = object;
 	struct app_context *ctx = user_data;
+	LIST_FOR_EACH (struct channel_user, iter, channel->users)
+		irc_channel_unlink_user (channel, iter);
 	str_map_set (&ctx->irc_channels, channel->name, NULL);
 }
 
@@ -1351,21 +1366,11 @@ irc_make_channel (struct app_context *ctx, char *name)
 }
 
 static void
-irc_unlink_user_from_channel (struct user *user, struct channel *channel)
+irc_remove_user_from_channel (struct user *user, struct channel *channel)
 {
-	// The order is important here as the channel can hold the last reference
-	LIST_FOR_EACH (struct user_channel, iter, user->channels)
-		if (iter->channel == channel)
-		{
-			LIST_UNLINK (user->channels, iter);
-			user_channel_destroy (iter);
-		}
 	LIST_FOR_EACH (struct channel_user, iter, channel->users)
 		if (iter->user == user)
-		{
-			LIST_UNLINK (channel->users, iter);
-			channel_user_destroy (iter);
-		}
+			irc_channel_unlink_user (channel, iter);
 }
 
 // --- Supporting code ---------------------------------------------------------
@@ -1945,7 +1950,7 @@ irc_handle_join (struct app_context *ctx, const struct irc_message *msg)
 
 	// Link the user with the channel
 	struct user_channel *user_channel = user_channel_new ();
-	user_channel->channel = channel_ref (channel);
+	user_channel->channel = channel;
 	LIST_PREPEND (user->channels, user_channel);
 
 	struct channel_user *channel_user = channel_user_new ();
@@ -1985,7 +1990,7 @@ irc_handle_kick (struct app_context *ctx, const struct irc_message *msg)
 
 	// It would be is weird for this to be false
 	if (user && channel)
-		irc_unlink_user_from_channel (user, channel);
+		irc_remove_user_from_channel (user, channel);
 
 	if (buffer)
 	{
@@ -2106,7 +2111,7 @@ irc_handle_part (struct app_context *ctx, const struct irc_message *msg)
 
 	// It would be is weird for this to be false
 	if (user && channel)
-		irc_unlink_user_from_channel (user, channel);
+		irc_remove_user_from_channel (user, channel);
 
 	if (buffer)
 	{
@@ -2202,17 +2207,8 @@ irc_handle_quit (struct app_context *ctx, const struct irc_message *msg)
 			buffer_send (ctx, buffer, BUFFER_LINE_QUIT, 0,
 				msg->prefix, message, "");
 
-		// Unlink the user from the channel
-		struct channel *channel = iter->channel;
-		LIST_FOR_EACH (struct channel_user, iter, channel->users)
-			if (iter->user == user)
-			{
-				LIST_UNLINK (channel->users, iter);
-				channel_user_destroy (iter);
-			}
-
-		LIST_UNLINK (user->channels, iter);
-		user_channel_destroy (iter);
+		// This destroys "iter" which doesn't matter to us
+		irc_remove_user_from_channel (user, iter->channel);
 	}
 }