Cleanup sub1_from_channel. Don't kick insecure users on +z channels when we

are merging (equal TS), only kick our users when we are on the losing side
(=if there is a different TS, IOTW: the channel was recreated later).

include/h.h

@@ -169,7 +169,7 @@ extern aCommand *find_Command_simple(char *cmd);
 extern aChannel *find_channel(char *, aChannel *);
 extern Membership *find_membership_link(Membership *lp, aChannel *ptr);
 extern Member *find_member_link(Member *, aClient *);
-extern void remove_user_from_channel(aClient *, aChannel *);
+extern int remove_user_from_channel(aClient *, aChannel *);
 extern void add_server_to_table(aClient *);
 extern void remove_server_from_table(aClient *);
 extern void iNAH_host(aClient *sptr, char *host);
@@ -601,7 +601,7 @@ extern MODVAR long sajoinmode;
 extern void add_user_to_channel(aChannel *chptr, aClient *who, int flags);
 extern int add_banid(aClient *, aChannel *, char *);
 extern int add_exbanid(aClient *cptr, aChannel *chptr, char *banid);
-extern void sub1_from_channel(aChannel *);
+extern int sub1_from_channel(aChannel *);
 extern MODVAR aCtab cFlagTab[];
 extern char *unreal_encodespace(char *s);
 extern char *unreal_decodespace(char *s);

include/modules.h

@@ -1016,7 +1016,7 @@ int hooktype_view_topic_outside_channel(aClient *sptr, aChannel *chptr);
 int hooktype_chan_permit_nick_change(aClient *sptr, aChannel *chptr);
 int hooktype_is_channel_secure(aChannel *chptr);
 int hooktype_can_send_secure(aClient *sptr, aChannel *chptr);
-void hooktype_channel_synced(aChannel *chptr, int merge, int removetheirs, int nomode);
+int hooktype_channel_synced(aChannel *chptr, int merge, int removetheirs, int nomode);
 int hooktype_can_sajoin(aClient *target, aChannel *chptr, aClient *sptr);
 int hooktype_check_init(aClient *cptr, char *sockname, size_t size);
 int hooktype_mode_deop(aClient *sptr, aClient *victim, aChannel *chptr, u_int what, int modechar, long my_access, char **badmode);

src/channel.c

@@ -54,7 +54,7 @@ char cmodestring[512];
 /* Some forward declarations */
 char *clean_ban_mask(char *, int, aClient *);
 void channel_modes(aClient *cptr, char *mbuf, char *pbuf, size_t mbuf_size, size_t pbuf_size, aChannel *chptr);
-void sub1_from_channel(aChannel *);
+int sub1_from_channel(aChannel *);
 void clean_channelname(char *);
 void del_invite(aClient *, aChannel *);
 
@@ -560,40 +560,48 @@ void add_user_to_channel(aChannel *chptr, aClient *who, int flags)
 	}
 }
 
-void remove_user_from_channel(aClient *sptr, aChannel *chptr)
+/** Remove the user from the channel.
+ * This doesn't send any PART etc. It does the free'ing of
+ * membership etc. It will also DESTROY the channel if the
+ * user was the last user (and the channel is not +P),
+ * via sub1_from_channel(), that is.
+ */
+int remove_user_from_channel(aClient *sptr, aChannel *chptr)
 {
-	Member **curr; Membership **curr2;
-	Member *tmp; Membership *tmp2;
-	Member *lp = chptr->members;
+	Member **curr;
+	Membership **curr2;
+	Member *tmp;
+	Membership *tmp2;
 
-	/* find 1st entry in list that is not user */
-	for (; lp && (lp->cptr == sptr); lp = lp->next);
-	for (;;)
+	/* Update chptr->members list */
+	for (curr = &chptr->members; (tmp = *curr); curr = &tmp->next)
 	{
-		for (curr = &chptr->members; (tmp = *curr); curr = &tmp->next)
-			if (tmp->cptr == sptr)
-			{
-				*curr = tmp->next;
-				free_member(tmp);
-				break;
-			}
-		for (curr2 = &sptr->user->channel; (tmp2 = *curr2); curr2 = &tmp2->next)
-			if (tmp2->chptr == chptr)
-			{
-				*curr2 = tmp2->next;
-				free_membership(tmp2, MyClient(sptr));
-				break;
-			}
-		sptr->user->joined--;
-		if (lp)
+		if (tmp->cptr == sptr)
+		{
+			*curr = tmp->next;
+			free_member(tmp);
 			break;
-		if (chptr->members)
-			sptr = chptr->members->cptr;
-		else
-			break;
-		sub1_from_channel(chptr);
+		}
 	}
-	sub1_from_channel(chptr);
+
+	/* Update sptr->user->channel list */
+	for (curr2 = &sptr->user->channel; (tmp2 = *curr2); curr2 = &tmp2->next)
+	{
+		if (tmp2->chptr == chptr)
+		{
+			*curr2 = tmp2->next;
+			free_membership(tmp2, MyClient(sptr));
+			break;
+		}
+	}
+
+	/* Update user record to reflect 1 less joined */
+	sptr->user->joined--;
+
+	/* Now sub1_from_channel() will deal with the channel record
+	 * and destroy the channel if needed.
+	 */
+	return sub1_from_channel(chptr);
 }
 
 long get_access(aClient *cptr, aChannel *chptr)
@@ -1047,75 +1055,79 @@ void del_invite(aClient *cptr, aChannel *chptr)
 		}
 }
 
-/*
-**  Subtract one user from channel i (and free channel
-**  block, if channel became empty).
-*/
-void sub1_from_channel(aChannel *chptr)
+/** Subtract one user from channel i. Free the channel if it became empty.
+ * @param chptr The channel
+ * @returns 1 if the channel was freed, 0 if the channel still exists.
+ */
+int sub1_from_channel(aChannel *chptr)
 {
 	Ban *ban;
 	Link *lp;
 	int should_destroy = 1;
 
 	--chptr->users;
-	if (chptr->users <= 0)
+	if (chptr->users > 0)
+		return 0;
+
+	/* No users in the channel anymore */
+	chptr->users = 0; /* to be sure */
+
+	/* If the channel is +P then this hook will actually stop destruction. */
+	RunHook2(HOOKTYPE_CHANNEL_DESTROY, chptr, &should_destroy);
+	if (!should_destroy)
+		return 0;
+
+	/* We are now going to destroy the channel.
+	 * But first we will destroy all kinds of references and lists...
+	 */
+
+	while ((lp = chptr->invites))
+		del_invite(lp->value.cptr, chptr);
+
+	while (chptr->banlist)
 	{
-		chptr->users = 0;
-
-		/*
-		 * Now, find all invite links from channel structure
-		 */
-		RunHook2(HOOKTYPE_CHANNEL_DESTROY, chptr, &should_destroy);
-		if (!should_destroy)
-			return;
-
-		while ((lp = chptr->invites))
-			del_invite(lp->value.cptr, chptr);
-
-		while (chptr->banlist)
-		{
-			ban = chptr->banlist;
-			chptr->banlist = ban->next;
-			MyFree(ban->banstr);
-			MyFree(ban->who);
-			free_ban(ban);
-		}
-		while (chptr->exlist)
-		{
-			ban = chptr->exlist;
-			chptr->exlist = ban->next;
-			MyFree(ban->banstr);
-			MyFree(ban->who);
-			free_ban(ban);
-		}
-		while (chptr->invexlist)
-		{
-			ban = chptr->invexlist;
-			chptr->invexlist = ban->next;
-			MyFree(ban->banstr);
-			MyFree(ban->who);
-			free_ban(ban);
-		}
-
-		/* free extcmode params */
-		extcmode_free_paramlist(chptr->mode.extmodeparams);
-
-		if (chptr->mode_lock)
-			MyFree(chptr->mode_lock);
-		if (chptr->topic)
-			MyFree(chptr->topic);
-		if (chptr->topic_nick)
-			MyFree(chptr->topic_nick);
-		if (chptr->prevch)
-			chptr->prevch->nextch = chptr->nextch;
-		else
-			channel = chptr->nextch;
-		if (chptr->nextch)
-			chptr->nextch->prevch = chptr->prevch;
-		(void)del_from_channel_hash_table(chptr->chname, chptr);
-		IRCstats.channels--;
-		MyFree(chptr);
+		ban = chptr->banlist;
+		chptr->banlist = ban->next;
+		MyFree(ban->banstr);
+		MyFree(ban->who);
+		free_ban(ban);
 	}
+	while (chptr->exlist)
+	{
+		ban = chptr->exlist;
+		chptr->exlist = ban->next;
+		MyFree(ban->banstr);
+		MyFree(ban->who);
+		free_ban(ban);
+	}
+	while (chptr->invexlist)
+	{
+		ban = chptr->invexlist;
+		chptr->invexlist = ban->next;
+		MyFree(ban->banstr);
+		MyFree(ban->who);
+		free_ban(ban);
+	}
+
+	/* free extcmode params */
+	extcmode_free_paramlist(chptr->mode.extmodeparams);
+
+	if (chptr->mode_lock)
+		MyFree(chptr->mode_lock);
+	if (chptr->topic)
+		MyFree(chptr->topic);
+	if (chptr->topic_nick)
+		MyFree(chptr->topic_nick);
+	if (chptr->prevch)
+		chptr->prevch->nextch = chptr->nextch;
+	else
+		channel = chptr->nextch;
+	if (chptr->nextch)
+		chptr->nextch->prevch = chptr->prevch;
+	(void)del_from_channel_hash_table(chptr->chname, chptr);
+	IRCstats.channels--;
+	MyFree(chptr);
+	return 1;
 }
 
 /* This function is only used for non-SJOIN servers. So don't bother with mtags support.. */

src/modules/chanmodes/secureonly.c

@@ -33,7 +33,7 @@ Cmode_t EXTCMODE_SECUREONLY;
 #define IsSecureOnly(chptr)    (chptr->mode.extmode & EXTCMODE_SECUREONLY)
 
 int secureonly_check_join(aClient *sptr, aChannel *chptr, char *key, char *parv[]);
-void secureonly_channel_sync (aChannel* chptr, int merge, int removetheirs, int nomode);
+int secureonly_channel_sync (aChannel* chptr, int merge, int removetheirs, int nomode);
 int secureonly_send_channel(aClient *acptr, aChannel* chptr);
 int secureonly_check_secure(aChannel* chptr);
 int secureonly_check_sajoin(aClient *acptr, aChannel* chptr, aClient *sptr);
@@ -56,7 +56,7 @@ MOD_INIT(secureonly)
 
 	HookAdd(modinfo->handle, HOOKTYPE_PRE_LOCAL_JOIN, 0, secureonly_specialcheck);
 	HookAdd(modinfo->handle, HOOKTYPE_CAN_JOIN, 0, secureonly_check_join);
-	HookAddVoid(modinfo->handle, HOOKTYPE_CHANNEL_SYNCED, 0, secureonly_channel_sync);
+	HookAdd(modinfo->handle, HOOKTYPE_CHANNEL_SYNCED, 0, secureonly_channel_sync);
 	HookAdd(modinfo->handle, HOOKTYPE_IS_CHANNEL_SECURE, 0, secureonly_check_secure);
 	HookAdd(modinfo->handle, HOOKTYPE_SEND_CHANNEL, 0, secureonly_send_channel);
 	HookAdd(modinfo->handle, HOOKTYPE_CAN_SAJOIN, 0, secureonly_check_sajoin);
@@ -77,8 +77,10 @@ MOD_UNLOAD(secureonly)
 }
 
 
-/** Kicks all insecure users on a +z channel */
-static void secureonly_kick_insecure_users(aChannel *chptr)
+/** Kicks all insecure users on a +z channel
+ * Returns 1 if the channel was destroyed as a result of this.
+ */
+static int secureonly_kick_insecure_users(aChannel *chptr)
 {
 	Member *member, *mb2;
 	aClient *cptr;
@@ -87,7 +89,7 @@ static void secureonly_kick_insecure_users(aChannel *chptr)
 	char *comment = "Insecure user not allowed on secure channel (+z)";
 
 	if (!IsSecureOnly(chptr))
-		return;
+		return 0;
 
 	for (member = chptr->members; member; member = mb2)
 	{
@@ -120,9 +122,11 @@ static void secureonly_kick_insecure_users(aChannel *chptr)
 
 			free_message_tags(mtags);
 
-			remove_user_from_channel(cptr, chptr);
+			if (remove_user_from_channel(cptr, chptr) == 1)
+				return 1; /* channel was destroyed */
 		}
 	}
+	return 0;
 }
 
 int secureonly_check_join(aClient *sptr, aChannel *chptr, char *key, char *parv[])
@@ -155,12 +159,11 @@ int secureonly_check_secure(aChannel *chptr)
 	return 0;
 }
 
-void secureonly_channel_sync(aChannel *chptr, int merge, int removetheirs, int nomode)
+int secureonly_channel_sync(aChannel *chptr, int merge, int removetheirs, int nomode)
 {
-	if ((!merge && !removetheirs && !nomode) || (merge && !nomode))
-	{
-		secureonly_kick_insecure_users(chptr);
-	}
+	if (!merge && !removetheirs && !nomode)
+		return secureonly_kick_insecure_users(chptr); /* may return 1, meaning channel is destroyed */
+	return 0;
 }
 
 int secureonly_send_channel(aClient *acptr, aChannel *chptr)

src/modules/sjoin.c

@@ -987,7 +987,9 @@ getnick:
 
 	for (h = Hooks[HOOKTYPE_CHANNEL_SYNCED]; h; h = h->next)
 	{
-		(*(h->func.voidfunc))(chptr,merge,removetheirs,nomode);
+		int i = (*(h->func.intfunc))(chptr,merge,removetheirs,nomode);
+		if (i == 1)
+			return -1; /* channel no longer exists */
 	}
 
 	/* we should be synched by now, */