From ba927964c8b671a3331fde0dd98b99ba414bbbd1 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Sat, 31 Aug 2013 11:51:23 -0700 Subject: [PATCH] Clean up ConCmdManager (bug 5900, r=fyren). --- core/ConCmdManager.cpp | 489 +++++++++++------------------------- core/ConCmdManager.h | 44 ++-- core/smn_console.cpp | 2 +- public/amtl/am-inlinelist.h | 16 ++ 4 files changed, 190 insertions(+), 361 deletions(-) diff --git a/core/ConCmdManager.cpp b/core/ConCmdManager.cpp index b0375ab7..200390a7 100644 --- a/core/ConCmdManager.cpp +++ b/core/ConCmdManager.cpp @@ -50,15 +50,8 @@ ConCmdManager g_ConCmds; SH_DECL_HOOK1_void(IServerGameClients, SetCommandClient, SH_NOATTRIB, false, int); -struct PlCmdInfo -{ - ConCmdInfo *pInfo; - CmdHook *pHook; - CmdType type; -}; -typedef List CmdList; - -void AddToPlCmdList(CmdList *pList, const PlCmdInfo &info); +typedef ke::LinkedList PluginHookList; +void RegisterInPlugin(CmdHook *hook); ConCmdManager::ConCmdManager() : m_Strings(1024) { @@ -92,116 +85,56 @@ void ConCmdManager::OnUnlinkConCommandBase(ConCommandBase *pBase, const char *na if (!m_Cmds.retrieve(name, &pInfo)) return; - RemoveConCmds(pInfo->srvhooks); - RemoveConCmds(pInfo->conhooks); + CmdHookList::iterator iter = pInfo->hooks.begin(); + while (iter != pInfo->hooks.end()) + { + CmdHook *hook = *iter; + + IPluginContext *pContext = hook->pf->GetParentContext(); + IPlugin *pPlugin = scripts->FindPluginByContext(pContext->GetContext()); + + // The list is guaranteed to exist. + PluginHookList *list; + pPlugin->GetProperty("CommandList", (void **)&list, false); + for (PluginHookList::iterator hiter = list->begin(); hiter != list->end(); hiter++) + { + if (*hiter == hook) + { + list->erase(hiter); + break; + } + } + + iter = pInfo->hooks.erase(iter); + delete hook; + } RemoveConCmd(pInfo, name, is_read_safe, false); } -void ConCmdManager::RemoveConCmds(List &cmdlist) -{ - List::iterator iter = cmdlist.begin(); - - while (iter != cmdlist.end()) - { - CmdHook *pHook = (*iter); - IPluginContext *pContext = pHook->pf->GetParentContext(); - IPlugin *pPlugin = scripts->FindPluginByContext(pContext->GetContext()); - CmdList *pList = NULL; - - //gaben - if (!pPlugin->GetProperty("CommandList", (void **)&pList, false) || !pList) - { - continue; - } - - CmdList::iterator p_iter = pList->begin(); - while (p_iter != pList->end()) - { - PlCmdInfo &cmd = (*p_iter); - if (cmd.pHook == pHook) - { - p_iter = pList->erase(p_iter); - } - else - { - p_iter++; - } - } - - delete pHook->pAdmin; - delete pHook; - - iter = cmdlist.erase(iter); - } -} - -void ConCmdManager::RemoveConCmds(List &cmdlist, IPluginContext *pContext) -{ - List::iterator iter = cmdlist.begin(); - CmdHook *pHook; - - while (iter != cmdlist.end()) - { - pHook = (*iter); - if (pHook->pf->GetParentContext() == pContext) - { - delete pHook->pAdmin; - delete pHook; - iter = cmdlist.erase(iter); - } - else - { - iter++; - } - } -} - void ConCmdManager::OnPluginDestroyed(IPlugin *plugin) { - CmdList *pList; - List removed; - if (plugin->GetProperty("CommandList", (void **)&pList, true)) + PluginHookList *pList; + if (!plugin->GetProperty("CommandList", (void **)&pList, true)) + return; + + PluginHookList::iterator iter = pList->begin(); + while (iter != pList->end()) { - IPluginContext *pContext = plugin->GetBaseContext(); - CmdList::iterator iter; - /* First pass! - * Only bother if there's an actual command list on this plugin... - */ - for (iter=pList->begin(); - iter!=pList->end(); - iter++) - { - PlCmdInfo &cmd = (*iter); - ConCmdInfo *pInfo = cmd.pInfo; + CmdHook *hook = *iter; - /* Has this chain already been fully cleaned/removed? */ - if (removed.find(pInfo) != removed.end()) - { - continue; - } + hook->info->hooks.remove(hook); - /* Remove any hooks from us on this command */ - RemoveConCmds(pInfo->conhooks, pContext); - RemoveConCmds(pInfo->srvhooks, pContext); + if (hook->info->hooks.empty()) + RemoveConCmd(hook->info, hook->info->pCmd->GetName(), true, true); - /* See if there are still hooks */ - if (pInfo->srvhooks.size()) - { - continue; - } - if (pInfo->conhooks.size()) - { - continue; - } - - /* Remove the command, it should be safe now */ - RemoveConCmd(pInfo, pInfo->pCmd->GetName(), true, true); - removed.push_back(pInfo); - } - delete pList; + iter = pList->erase(iter); + delete hook; } + + delete pList; } + #if SOURCE_ENGINE == SE_DOTA void CommandCallback(const CCommandContext &context, const CCommand &command) { @@ -261,44 +194,34 @@ ResultType ConCmdManager::DispatchClientCommand(int client, const char *cmd, int } cell_t result = type; - cell_t tempres = result; - List::iterator iter; - CmdHook *pHook; - for (iter=pInfo->conhooks.begin(); - iter!=pInfo->conhooks.end(); - iter++) + for (CmdHookList::iterator iter = pInfo->hooks.begin(); iter != pInfo->hooks.end(); iter++) { - pHook = (*iter); - if (!pHook->pf->IsRunnable()) - { + CmdHook *hook = *iter; + + if (hook->type == CmdHook::Server || !hook->pf->IsRunnable()) continue; - } - if (pHook->pAdmin && !CheckAccess(client, cmd, pHook->pAdmin)) + + if (hook->admin && !CheckAccess(client, cmd, hook->admin)) { if (result < Pl_Handled) - { result = Pl_Handled; - } continue; } - pHook->pf->PushCell(client); - pHook->pf->PushCell(args); - if (pHook->pf->Execute(&tempres) == SP_ERROR_NONE) + + hook->pf->PushCell(client); + hook->pf->PushCell(args); + + cell_t tempres = result; + if (hook->pf->Execute(&tempres) == SP_ERROR_NONE) { if (tempres > result) - { result = tempres; - } if (result == Pl_Stop) - { break; - } } } - type = (ResultType)result; - - return type; + return (ResultType)result; } void ConCmdManager::InternalDispatch(const CCommand &command) @@ -309,9 +232,7 @@ void ConCmdManager::InternalDispatch(const CCommand &command) { CPlayer *pPlayer = g_Players.GetPlayerByIndex(client); if (!pPlayer || !pPlayer->IsConnected()) - { return; - } } /** @@ -343,109 +264,61 @@ void ConCmdManager::InternalDispatch(const CCommand &command) * "nicer" when we expose explicit say hooks. */ if (g_ChatTriggers.WasFloodedMessage()) - { return; - } cell_t result = Pl_Continue; int args = command.ArgC() - 1; - List::iterator iter; - CmdHook *pHook; + // On a listen server, sometimes the server host's client index can be set + // as 0. So index 1 is passed to the command callback to correct this + // potential problem. + int realClient = (!engine->IsDedicatedServer() && client == 0) + ? g_Players.ListenClient() + : client; + int dedicatedClient = engine->IsDedicatedServer() ? 0 : g_Players.ListenClient(); - /* Execute server-only commands if viable */ - if (client == 0 && pInfo->srvhooks.size()) + for (CmdHookList::iterator iter = pInfo->hooks.begin(); iter != pInfo->hooks.end(); iter++) { - cell_t tempres = result; - for (iter=pInfo->srvhooks.begin(); - iter!=pInfo->srvhooks.end(); - iter++) - { - pHook = (*iter); - if (!pHook->pf->IsRunnable()) - { - continue; - } - pHook->pf->PushCell(args); - if (pHook->pf->Execute(&tempres) == SP_ERROR_NONE) - { - if (tempres > result) - { - result = tempres; - } - if (result == Pl_Stop) - { - break; - } - } - } - - /* Check if there's an early stop */ - if (result >= Pl_Stop) - { - if (!pInfo->sourceMod) - { - RETURN_META(MRES_SUPERCEDE); - } - return; - } - } + CmdHook *hook = *iter; - /* Execute console commands */ - if (pInfo->conhooks.size()) - { - cell_t tempres = result; - for (iter=pInfo->conhooks.begin(); - iter!=pInfo->conhooks.end(); - iter++) + if (!hook->pf->IsRunnable()) + continue; + + if (hook->type == CmdHook::Server) { - pHook = (*iter); - if (!pHook->pf->IsRunnable()) - { + // Don't execute unless we're in the server console. + if (realClient != dedicatedClient) continue; - } - if (client - && pHook->pAdmin - && !CheckAccess(client, cmd, pHook->pAdmin)) + } else { + // Check admin rights if needed. realClient isn't needed since we + // should bypass admin checks if client == 0 anyway. + if (client && hook->admin && !CheckAccess(client, cmd, hook->admin)) { if (result < Pl_Handled) - { result = Pl_Handled; - } continue; } - /* On a listen server, sometimes the server host's client index can be set as 0. - * So index 1 is passed to the command callback to correct this potential problem. - */ - if (!engine->IsDedicatedServer()) - { - client = g_Players.ListenClient(); - } + // Client hooks get a client argument. + hook->pf->PushCell(realClient); + } - pHook->pf->PushCell(client); - pHook->pf->PushCell(args); + hook->pf->PushCell(args); - if (pHook->pf->Execute(&tempres) == SP_ERROR_NONE) - { - if (tempres > result) - { - result = tempres; - } - if (result == Pl_Stop) - { - break; - } - } + cell_t tempres = result; + if (hook->pf->Execute(&tempres) == SP_ERROR_NONE) + { + if (tempres > result) + result = tempres; + if (result == Pl_Stop) + break; } } if (result >= Pl_Handled) { if (!pInfo->sourceMod) - { RETURN_META(MRES_SUPERCEDE); - } return; } } @@ -575,19 +448,11 @@ bool ConCmdManager::AddAdminCommand(IPluginFunction *pFunction, ConCmdInfo *pInfo = AddOrFindCommand(name, description, flags); if (!pInfo) - { return false; - } - CmdHook *pHook = new CmdHook(); - AdminCmdInfo *pAdmin = new AdminCmdInfo(); + CmdHook *pHook = new CmdHook(CmdHook::Client, pInfo, pFunction, description); - pHook->pf = pFunction; - if (description && description[0]) - { - pHook->helptext.assign(description); - } - pHook->pAdmin = pAdmin; + pHook->admin = new AdminCmdInfo(); int grpid; if (!m_CmdGrps.retrieve(group, &grpid)) @@ -596,46 +461,29 @@ bool ConCmdManager::AddAdminCommand(IPluginFunction *pFunction, m_CmdGrps.insert(group, grpid); } - pAdmin->cmdGrpId = grpid; - pAdmin->flags = adminflags; + pHook->admin->cmdGrpId = grpid; + pHook->admin->flags = adminflags; /* First get the command group override, if any */ bool override = g_Admins.GetCommandOverride(group, Override_CommandGroup, - &(pAdmin->eflags)); + &(pHook->admin->eflags)); /* Next get the command override, if any */ if (g_Admins.GetCommandOverride(name, Override_Command, - &(pAdmin->eflags))) + &(pHook->admin->eflags))) { override = true; } /* Assign normal flags if there were no overrides */ if (!override) - { - pAdmin->eflags = pAdmin->flags; - } - - /* Finally, add the hook */ - pInfo->conhooks.push_back(pHook); - pInfo->admin = *(pHook->pAdmin); - - /* Now add to the plugin */ - CmdList *pList; - IPlugin *pPlugin = scripts->FindPluginByContext(pFunction->GetParentContext()->GetContext()); - if (!pPlugin->GetProperty("CommandList", (void **)&pList)) - { - pList = new CmdList(); - pPlugin->SetProperty("CommandList", pList); - } - PlCmdInfo info; - info.pInfo = pInfo; - info.type = Cmd_Admin; - info.pHook = pHook; - AddToPlCmdList(pList, info); + pHook->admin->eflags = pHook->admin->flags; + pInfo->eflags = pHook->admin->eflags; + pInfo->hooks.append(pHook); + RegisterInPlugin(pHook); return true; } @@ -648,63 +496,42 @@ bool ConCmdManager::AddServerCommand(IPluginFunction *pFunction, ConCmdInfo *pInfo = AddOrFindCommand(name, description, flags); if (!pInfo) - { return false; - } - CmdHook *pHook = new CmdHook(); - - pHook->pf = pFunction; - if (description && description[0]) - { - pHook->helptext.assign(description); - } - - pInfo->srvhooks.push_back(pHook); - - /* Add to the plugin */ - CmdList *pList; - IPlugin *pPlugin = scripts->FindPluginByContext(pFunction->GetParentContext()->GetContext()); - if (!pPlugin->GetProperty("CommandList", (void **)&pList)) - { - pList = new CmdList(); - pPlugin->SetProperty("CommandList", pList); - } - PlCmdInfo info; - info.pInfo = pInfo; - info.type = Cmd_Server; - info.pHook = pHook; - AddToPlCmdList(pList, info); + CmdHook *pHook = new CmdHook(CmdHook::Server, pInfo, pFunction, description); + pInfo->hooks.append(pHook); + RegisterInPlugin(pHook); return true; } -void AddToPlCmdList(CmdList *pList, const PlCmdInfo &info) +void RegisterInPlugin(CmdHook *hook) { - CmdList::iterator iter = pList->begin(); - bool inserted = false; - const char *orig = NULL; + PluginHookList *pList; - orig = info.pInfo->pCmd->GetName(); + IPlugin *pPlugin = scripts->FindPluginByContext(hook->pf->GetParentContext()); + if (!pPlugin->GetProperty("CommandList", (void **)&pList)) + { + pList = new PluginHookList(); + pPlugin->SetProperty("CommandList", pList); + } + + const char *orig = hook->info->pCmd->GetName(); /* Insert this into the help list, SORTED alphabetically. */ + PluginHookList::iterator iter = pList->begin(); while (iter != pList->end()) { - PlCmdInfo &obj = (*iter); - const char *cmd = obj.pInfo->pCmd->GetName(); + const char *cmd = (*iter)->info->pCmd->GetName(); if (strcmp(orig, cmd) < 0) { - pList->insert(iter, info); - inserted = true; - break; + pList->insertBefore(iter, hook); + return; } iter++; } - if (!inserted) - { - pList->push_back(info); - } + pList->append(hook); } void ConCmdManager::AddToCmdList(ConCmdInfo *info) @@ -739,29 +566,22 @@ void ConCmdManager::AddToCmdList(ConCmdInfo *info) void ConCmdManager::UpdateAdminCmdFlags(const char *cmd, OverrideType type, FlagBits bits, bool remove) { - ConCmdInfo *pInfo; - if (type == Override_Command) { + ConCmdInfo *pInfo; if (!m_Cmds.retrieve(cmd, &pInfo)) return; - List::iterator iter; - CmdHook *pHook; - - for (iter=pInfo->conhooks.begin(); iter!=pInfo->conhooks.end(); iter++) + for (CmdHookList::iterator iter = pInfo->hooks.begin(); iter != pInfo->hooks.end(); iter++) { - pHook = (*iter); - if (pHook->pAdmin) - { - if (!remove) - { - pHook->pAdmin->eflags = bits; - } else { - pHook->pAdmin->eflags = pHook->pAdmin->flags; - } - pInfo->admin = *(pHook->pAdmin); - } + if (!iter->admin) + continue; + + if (!remove) + iter->admin->eflags = bits; + else + iter->admin->eflags = iter->admin->flags; + pInfo->eflags = iter->admin->eflags; } } else if (type == Override_CommandGroup) @@ -772,25 +592,20 @@ void ConCmdManager::UpdateAdminCmdFlags(const char *cmd, OverrideType type, Flag /* This is bad :( loop through all commands */ List::iterator iter; - CmdHook *pHook; for (iter=m_CmdList.begin(); iter!=m_CmdList.end(); iter++) { - pInfo = (*iter); - for (List::iterator citer=pInfo->conhooks.begin(); - citer!=pInfo->conhooks.end(); - citer++) + ConCmdInfo *pInfo = *iter; + for (CmdHookList::iterator citer = pInfo->hooks.begin(); citer != pInfo->hooks.end(); citer++) { - pHook = (*citer); - if (pHook->pAdmin && pHook->pAdmin->cmdGrpId == grpid) - { - if (remove) - { - pHook->pAdmin->eflags = bits; - } else { - pHook->pAdmin->eflags = pHook->pAdmin->flags; - } - pInfo->admin = *(pHook->pAdmin); - } + CmdHook *hook = (*citer); + if (!hook->admin || hook->admin->cmdGrpId != grpid) + continue; + + if (remove) + hook->admin->eflags = bits; + else + hook->admin->eflags = hook->admin->flags; + pInfo->eflags = hook->admin->eflags; } } } @@ -843,7 +658,7 @@ bool ConCmdManager::LookForSourceModCommand(const char *cmd) if (!m_Cmds.retrieve(cmd, &pInfo)) return false; - return pInfo->sourceMod && (pInfo->conhooks.size() > 0); + return pInfo->sourceMod && !pInfo->hooks.empty(); } bool ConCmdManager::LookForCommandAdminFlags(const char *cmd, FlagBits *pFlags) @@ -852,7 +667,7 @@ bool ConCmdManager::LookForCommandAdminFlags(const char *cmd, FlagBits *pFlags) if (!m_Cmds.retrieve(cmd, &pInfo)) return false; - *pFlags = pInfo->admin.eflags; + *pFlags = pInfo->eflags; return true; } @@ -915,46 +730,36 @@ void ConCmdManager::OnRootConsoleCommand(const char *cmdname, const CCommand &co const sm_plugininfo_t *plinfo = pPlugin->GetPublicInfo(); const char *plname = IS_STR_FILLED(plinfo->name) ? plinfo->name : pPlugin->GetFilename(); - CmdList *pList; + PluginHookList *pList; if (!pPlugin->GetProperty("CommandList", (void **)&pList)) { g_RootMenu.ConsolePrint("[SM] No commands found for: %s", plname); return; } - if (!pList->size()) + if (pList->empty()) { g_RootMenu.ConsolePrint("[SM] No commands found for: %s", plname); return; } - CmdList::iterator iter; const char *type = NULL; const char *name; const char *help; - g_RootMenu.ConsolePrint("[SM] Listing %d commands for: %s", pList->size(), plname); + g_RootMenu.ConsolePrint("[SM] Listing commands for: %s", plname); g_RootMenu.ConsolePrint(" %-17.16s %-8.7s %s", "[Name]", "[Type]", "[Help]"); - for (iter=pList->begin(); - iter!=pList->end(); - iter++) + for (PluginHookList::iterator iter = pList->begin(); iter != pList->end(); iter++) { - PlCmdInfo &cmd = (*iter); - if (cmd.type == Cmd_Server) - { + CmdHook *hook = *iter; + if (hook->type == CmdHook::Server) type = "server"; - } - else if (cmd.type == Cmd_Admin) - { - type = (cmd.pInfo->admin.eflags == 0)?"console":"admin"; - } - name = cmd.pInfo->pCmd->GetName(); - if (cmd.pHook->helptext.size()) - { - help = cmd.pHook->helptext.c_str(); - } + else + type = hook->info->eflags == 0 ? "console" : "admin"; + + name = hook->info->pCmd->GetName(); + if (hook->helptext.length()) + help = hook->helptext.chars(); else - { - help = cmd.pInfo->pCmd->GetHelpText(); - } + help = hook->info->pCmd->GetHelpText(); g_RootMenu.ConsolePrint(" %-17.16s %-12.11s %s", name, type, help); } diff --git a/core/ConCmdManager.h b/core/ConCmdManager.h index 1de22834..96ac1e52 100644 --- a/core/ConCmdManager.h +++ b/core/ConCmdManager.h @@ -43,15 +43,11 @@ #include "concmd_cleaner.h" #include #include +#include +#include using namespace SourceHook; -enum CmdType -{ - Cmd_Server, - Cmd_Admin, -}; - struct AdminCmdInfo { AdminCmdInfo() @@ -65,30 +61,44 @@ struct AdminCmdInfo FlagBits eflags; /* effective flags */ }; -struct CmdHook +struct ConCmdInfo; + +struct CmdHook : public ke::InlineListNode { - CmdHook() + enum Type { + Server, + Client + }; + + CmdHook(Type type, ConCmdInfo *cmd, IPluginFunction *fun, const char *description) + : type(type), + info(cmd), + pf(fun), + helptext(description) { - pf = NULL; - pAdmin = NULL; } - IPluginFunction *pf; /* function hook */ - String helptext; /* help text */ - AdminCmdInfo *pAdmin; /* admin requirements, if any */ + + Type type; + ConCmdInfo *info; + IPluginFunction *pf; /* function hook */ + ke::AString helptext; /* help text */ + ke::AutoPtr admin; /* admin requirements, if any */ }; +typedef ke::InlineList CmdHookList; + struct ConCmdInfo { ConCmdInfo() { sourceMod = false; pCmd = NULL; + eflags = 0; } bool sourceMod; /**< Determines whether or not concmd was created by a SourceMod plugin */ ConCommand *pCmd; /**< Pointer to the command itself */ - List srvhooks; /**< Hooks as a server command */ - List conhooks; /**< Hooks as a console command */ - AdminCmdInfo admin; /**< Admin info, if any */ + CmdHookList hooks; /**< Hook list */ + FlagBits eflags; /**< Effective admin flags */ }; typedef List ConCmdList; @@ -139,8 +149,6 @@ private: void SetCommandClient(int client); void AddToCmdList(ConCmdInfo *info); void RemoveConCmd(ConCmdInfo *info, const char *cmd, bool is_read_safe, bool untrack); - void RemoveConCmds(List &cmdlist); - void RemoveConCmds(List &cmdlist, IPluginContext *pContext); bool CheckAccess(int client, const char *cmd, AdminCmdInfo *pAdmin); // Case insensitive diff --git a/core/smn_console.cpp b/core/smn_console.cpp index 61dc682a..e7c00c92 100644 --- a/core/smn_console.cpp +++ b/core/smn_console.cpp @@ -1278,7 +1278,7 @@ static cell_t ReadCommandIterator(IPluginContext *pContext, const cell_t *params cell_t *addr; pContext->LocalToPhysAddr(params[4], &addr); - *addr = pInfo->admin.eflags; + *addr = pInfo->eflags; iter->iter++; diff --git a/public/amtl/am-inlinelist.h b/public/amtl/am-inlinelist.h index c0d052ba..99c37793 100644 --- a/public/amtl/am-inlinelist.h +++ b/public/amtl/am-inlinelist.h @@ -140,6 +140,22 @@ class InlineList return iterator(&head_); } + iterator erase(iterator &at) { + iterator next = at; + next++; + + remove(at.iter_); + + // Iterator is no longer valid. + at.iter_ = NULL; + + return next; + } + + bool empty() const { + return head_.next_ == &head_; + } + void remove(Node *t) { t->prev_->next_ = t->next_; t->next_->prev_ = t->prev_;