From 7eb9ed90cb825da9a0205884b86da3ec33e3fab9 Mon Sep 17 00:00:00 2001 From: Bram Matthys Date: Sat, 7 Sep 2019 14:45:47 +0200 Subject: [PATCH] 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 | 4 +- include/modules.h | 2 +- src/channel.c | 194 +++++++++++++++-------------- src/modules/chanmodes/secureonly.c | 25 ++-- src/modules/sjoin.c | 4 +- 5 files changed, 123 insertions(+), 106 deletions(-) diff --git a/include/h.h b/include/h.h index d295a6973..14cad25a5 100644 --- a/include/h.h +++ b/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); diff --git a/include/modules.h b/include/modules.h index 769c5ec5a..937f19a9a 100644 --- a/include/modules.h +++ b/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); diff --git a/src/channel.c b/src/channel.c index 8e3a70349..59449d6d1 100644 --- a/src/channel.c +++ b/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.. */ diff --git a/src/modules/chanmodes/secureonly.c b/src/modules/chanmodes/secureonly.c index 53fa87b1d..4180fcedd 100644 --- a/src/modules/chanmodes/secureonly.c +++ b/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) diff --git a/src/modules/sjoin.c b/src/modules/sjoin.c index c66baf3b4..960c4520e 100644 --- a/src/modules/sjoin.c +++ b/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, */