Skip to content

Commit

Permalink
Individualize NameHashSet Hashing & Revisit #709 (#740) (1.9-dev) (#866)
Browse files Browse the repository at this point in the history
This is a clone of #740, but without the amtl ke::AString lowercase which was implemented in a new version of amtl that 1.9-dev isn't pinned to. Updating this pin and moving fixes is beyond what should go in 1.9, and this fixes a annoying and user-impactful bug with reload/unloading plugins on windows.

Currently in 1.9, once a plugin is loaded into the pluginsys, they must be used with lowercase characters *only*, since pr #709 ignorantly modified their names. 

```
// test.smx exists in /plugins/
sm plugins load TEST.smx // successful
sm plugins unload TEST.smx // TEST.smx not found, it's actually test.smx
```

This pr fixes that error by converting *all* lookups, not just loads.
  • Loading branch information
Headline authored and asherkin committed Aug 11, 2018
1 parent c6303d1 commit dd456dc
Show file tree
Hide file tree
Showing 15 changed files with 117 additions and 40 deletions.
5 changes: 5 additions & 0 deletions core/ConVarManager.h
Expand Up @@ -43,6 +43,7 @@
#include <compat_wrappers.h>
#include "concmd_cleaner.h"
#include "PlayerManager.h"
#include <sm_stringhashmap.h>

using namespace SourceHook;

Expand All @@ -67,6 +68,10 @@ struct ConVarInfo
{
return strcmp(name, info->pVar->GetName()) == 0;
}
static inline uint32_t hash(const detail::CharsAndLength &key)
{
return key.hash();
}
};

/**
Expand Down
4 changes: 4 additions & 0 deletions core/EventManager.h
Expand Up @@ -77,6 +77,10 @@ struct EventHook
{
return strcmp(name, hook->name.chars()) == 0;
}
static inline uint32_t hash(const detail::CharsAndLength &key)
{
return key.hash();
}
};

enum EventHookMode
Expand Down
14 changes: 13 additions & 1 deletion core/HalfLife2.h
Expand Up @@ -92,13 +92,21 @@ struct DataTableInfo
{
return strcmp(name, info.prop->GetName()) == 0;
}
static inline uint32_t hash(const detail::CharsAndLength &key)
{
return key.hash();
}
};

static inline bool matches(const char *name, const DataTableInfo *info)
{
return strcmp(name, info->sc->GetName()) == 0;
}

static inline uint32_t hash(const detail::CharsAndLength &key)
{
return key.hash();
}

DataTableInfo(ServerClass *sc)
: sc(sc)
{
Expand All @@ -114,6 +122,10 @@ struct DataMapCachePolicy
{
return strcmp(name, info.prop->fieldName) == 0;
}
static inline uint32_t hash(const detail::CharsAndLength &key)
{
return key.hash();
}
};

typedef NameHashSet<sm_datatable_info_t, DataMapCachePolicy> DataMapCache;
Expand Down
4 changes: 4 additions & 0 deletions core/logic/AdminCache.h
Expand Up @@ -82,6 +82,10 @@ struct AuthMethod
{
return strcmp(name, method->name.c_str()) == 0;
}
static inline uint32_t hash(const detail::CharsAndLength &key)
{
return key.hash();
}
};

struct UserAuth
Expand Down
4 changes: 4 additions & 0 deletions core/logic/GameConfigs.h
Expand Up @@ -72,6 +72,10 @@ class CGameConfig :
{
return strcmp(key, value->m_File) == 0;
}
static inline uint32_t hash(const detail::CharsAndLength &key)
{
return key.hash();
}
private:
char m_File[PLATFORM_MAX_PATH];
char m_CurFile[PLATFORM_MAX_PATH];
Expand Down
4 changes: 4 additions & 0 deletions core/logic/HandleSys.h
Expand Up @@ -110,6 +110,10 @@ struct QHandleType
{
return type->name && type->name->compare(key) == 0;
}
static inline uint32_t hash(const detail::CharsAndLength &key)
{
return key.hash();
}
};

typedef ke::Lambda<void(const char *)> HandleReporter;
Expand Down
5 changes: 5 additions & 0 deletions core/logic/Native.h
Expand Up @@ -36,6 +36,7 @@
#include <am-string.h>
#include <am-utility.h>
#include <am-refcounting.h>
#include <sm_stringhashmap.h>
#include "common_logic.h"

class CNativeOwner;
Expand Down Expand Up @@ -93,6 +94,10 @@ struct Native : public ke::Refcounted<Native>
{
return strcmp(name, entry->name()) == 0;
}
static inline uint32_t hash(const detail::CharsAndLength &key)
{
return key.hash();
}
};


Expand Down
31 changes: 3 additions & 28 deletions core/logic/PluginSys.cpp
Expand Up @@ -31,7 +31,6 @@

#include <stdio.h>
#include <stdarg.h>
#include <ctype.h>
#include "PluginSys.h"
#include "ShareSys.h"
#include <ILibrarySys.h>
Expand All @@ -47,7 +46,6 @@
#include "frame_tasks.h"
#include <amtl/am-string.h>
#include <amtl/am-linkedlist.h>
#include <amtl/am-uniqueptr.h>
#include <bridge/include/IVEngineServerBridge.h>
#include <bridge/include/CoreProvider.h>

Expand Down Expand Up @@ -934,38 +932,16 @@ void CPluginManager::LoadPluginsFromDir(const char *basedir, const char *localpa
libsys->CloseDirectory(dir);
}

#if defined PLATFORM_WINDOWS || defined PLATFORM_APPLE
char *strdup_tolower(const char *input)
{
char *str = strdup(input);

for (char *c = str; *c; c++)
{
*c = tolower((unsigned char)*c);
}

return str;
}
#endif

LoadRes CPluginManager::LoadPlugin(CPlugin **aResult, const char *path, bool debug, PluginType type)
{
if (m_LoadingLocked)
return LoadRes_NeverLoad;

/* For windows & mac, we convert the path to lower-case in order to avoid duplicate plugin loading */
#if defined PLATFORM_WINDOWS || defined PLATFORM_APPLE
ke::UniquePtr<char> finalPath = ke::UniquePtr<char>(strdup_tolower(path));
#else
ke::UniquePtr<char> finalPath = ke::UniquePtr<char>(strdup(path));
#endif


/**
* Does this plugin already exist?
*/
CPlugin *pPlugin;
if (m_LoadLookup.retrieve(finalPath.get(), &pPlugin))
if (m_LoadLookup.retrieve(path, &pPlugin))
{
/* Check to see if we should try reloading it */
if (pPlugin->GetStatus() == Plugin_BadLoad
Expand All @@ -978,12 +954,11 @@ LoadRes CPluginManager::LoadPlugin(CPlugin **aResult, const char *path, bool deb
{
if (aResult)
*aResult = pPlugin;

return LoadRes_AlreadyLoaded;
}
}

CPlugin *plugin = CompileAndPrep(finalPath.get());
CPlugin *plugin = CompileAndPrep(path);

// Assign our outparam so we can return early. It must be set.
*aResult = plugin;
Expand Down Expand Up @@ -2416,4 +2391,4 @@ static OldPluginAPI sOldPluginAPI;
IPluginManager *CPluginManager::GetOldAPI()
{
return &sOldPluginAPI;
}
}
54 changes: 48 additions & 6 deletions core/logic/PluginSys.h
Expand Up @@ -143,11 +143,6 @@ class CPlugin :
*/
static CPlugin *Create(const char *file);

static inline bool matches(const char *file, const CPlugin *plugin)
{
return strcmp(plugin->m_filename, file) == 0;
}

public:
// Evicts the plugin from memory and sets an error state.
void EvictWithError(PluginStatus status, const char *error_fmt, ...);
Expand Down Expand Up @@ -483,7 +478,54 @@ class CPluginManager :
typedef decltype(m_listeners)::iterator ListenerIter;
typedef decltype(m_plugins)::iterator PluginIter;

NameHashSet<CPlugin *> m_LoadLookup;
struct CPluginPolicy
{
static inline uint32_t hash(const detail::CharsAndLength &key)
{
/* For windows & mac, we convert the path to lower-case in order to avoid duplicate plugin loading */
#if defined PLATFORM_WINDOWS || defined PLATFORM_APPLE
const char *original = key.chars();
char *copy = strdup(original);

for (size_t i = 0; copy[i]; ++i)
{
copy[i] = tolower(copy[i]);
}

uint32_t hash = detail::CharsAndLength(copy).hash();
free(copy);
return hash;
#else
return key.hash();
#endif
}

static inline bool matches(const char *file, const CPlugin *plugin)
{
const char *pluginFile = const_cast<CPlugin*>(plugin)->GetFilename();
#if defined PLATFORM_WINDOWS || defined PLATFORM_APPLE
size_t fileLen = strlen(file);
if (fileLen != strlen(pluginFile))
{
return false;
}

for (size_t i = 0; i < fileLen; ++i)
{
if (tolower(file[i]) != tolower(pluginFile[i]))
{
return false;
}
}

return true;
#else
return strcmp(pluginFile, file) == 0;
#endif
}
};
NameHashSet<CPlugin *, CPluginPolicy> m_LoadLookup;

bool m_AllPluginsLoaded;
IdentityToken_t *m_MyIdent;

Expand Down
4 changes: 4 additions & 0 deletions core/logic/RootConsoleMenu.h
Expand Up @@ -46,6 +46,10 @@ struct ConsoleEntry
{
return strcmp(name, entry->command.c_str()) == 0;
}
static inline uint32_t hash(const detail::CharsAndLength &key)
{
return key.hash();
}
};

class RootConsoleMenu :
Expand Down
4 changes: 4 additions & 0 deletions core/logic/smn_maplists.cpp
Expand Up @@ -58,6 +58,10 @@ struct maplist_info_t
{
return strcmp(value->name, key) == 0;
}
static inline uint32_t hash(const detail::CharsAndLength &key)
{
return key.hash();
}
};

#define MAPLIST_FLAG_MAPSFOLDER (1<<0) /**< On failure, use all maps in the maps folder. */
Expand Down
4 changes: 4 additions & 0 deletions core/smn_console.cpp
Expand Up @@ -184,6 +184,10 @@ class CommandFlagsHelper : public IConCommandTracker
{
return strcmp(name, base->GetName()) == 0;
}
static inline uint32_t hash(const detail::CharsAndLength &key)
{
return key.hash();
}
};
NameHashSet<ConCommandBase *, ConCommandPolicy> m_CmdFlags;
} s_CommandFlagsHelper;
Expand Down
4 changes: 4 additions & 0 deletions extensions/clientprefs/cookie.h
Expand Up @@ -96,6 +96,10 @@ struct Cookie
{
return strcmp(name, cookie->name) == 0;
}
static inline uint32_t hash(const detail::CharsAndLength &key)
{
return key.hash();
}
};

class CookieManager : public IClientListener, public IPluginsListener
Expand Down
4 changes: 4 additions & 0 deletions extensions/topmenus/TopMenu.h
Expand Up @@ -77,6 +77,10 @@ struct topmenu_object_t
{
return strcmp(name, topmenu->name) == 0;
}
static inline uint32_t hash(const detail::CharsAndLength &key)
{
return key.hash();
}
};

struct topmenu_category_t
Expand Down
12 changes: 7 additions & 5 deletions public/sm_namehashset.h
Expand Up @@ -48,10 +48,12 @@
namespace SourceMod
{

// The HashPolicy type must have this method:
// The HashPolicy type must have these methods:
// static bool matches(const char *key, const T &value);
// static uint32_t hash(const CharsAndLength &key);
//
// Depending on what lookup types are used.
// Depending on what lookup types are used, and how hashing should be done.
// Most of the time, key hashing will just call the key's hash() method.
//
// If these members are available on T, then the HashPolicy type can be left
// default. It is okay to use |T *|, the functions will still be looked up
Expand All @@ -69,7 +71,7 @@ class NameHashSet : public ke::SystemAllocatorPolicy

static uint32_t hash(const CharsAndLength &key)
{
return key.hash();
return KeyPolicyType::hash(key);
}

static bool matches(const CharsAndLength &key, const KeyType &value)
Expand All @@ -85,9 +87,9 @@ class NameHashSet : public ke::SystemAllocatorPolicy
{
typedef KeyType *Payload;

static uint32_t hash(const detail::CharsAndLength &key)
static uint32_t hash(const CharsAndLength &key)
{
return key.hash();
return KeyType::hash(key);
}

static bool matches(const CharsAndLength &key, const KeyType *value)
Expand Down

0 comments on commit dd456dc

Please sign in to comment.