Rewrote format() parser to be re-entrant and easier to read

Added various optimizations
Fixed memory leak in sh_tinyhash

amxmodx/CLang.cpp

@@ -324,276 +324,243 @@ int CLangMngr::GetKeyEntry(String &key)
 	return val.index;
 }
 
-#define CHECK_PTR(ptr, start, bufsize) if ((ptr) - (start) >= (bufsize)) { \
+/**
-	LogError(amx, AMX_ERR_STACKERR, "Buffer overflow in string formatting"); \
+ * FORMATTING ROUTINES
-	outbuf[0] = 0; \
+ */
-	len = 0; \
+
-	return outbuf; }
+
-#define CHECK_OUTPTR(offset) CHECK_PTR(outptr+offset, outbuf, sizeof(outbuf))
+#define MAX_LEVELS	4
-#define ZEROTERM(buf) buf[(sizeof(buf)/sizeof(buf[0]))-1]=0;
+
-#define NEXT_PARAM()		\
+size_t do_amx_format(AMX *amx, cell *params, int *param, const char **lex, char *output, size_t maxlen, int level);
-	if (parm > paramCount) \
+
-	{ \
+size_t do_amx_format_parameter(AMX *amx, cell *params, const char **fmtstr, int *param, char *output, size_t maxlen, int level)
-		strcpy(outbuf, ""); \
+{
-		len = 0; \
+	static char buffer[MAX_LEVELS][2048];
-		LogError(amx, AMX_ERR_PARAMS, "String formatted incorrectly - parameter %d (total %d)", parm, paramCount); \
+	static char fmt[MAX_LEVELS][32];
-		return outbuf; \
+	size_t len = 0;
+	char *fmtptr = fmt[level];
+	const char *fmtsrc = *fmtstr;
+	char ctrl_code;
+	int numParams = params[0] / sizeof(cell);
+	if (*param > numParams)
+	{
+		LogError(amx, AMX_ERR_PARAMS, "String formatted incorrectly - parameter %d (total %d)", *param, numParams);
+		return 0;
+	}
+	cell _addr = params[*param];
+	cell *addr = get_amxaddr(amx, _addr);
+	(*param)++;
+
+	if (level >= MAX_LEVELS)
+	{
+		output[0] = '\0';
+		return 0;
 	}
 
-char * CLangMngr::FormatAmxString(AMX *amx, cell *params, int parm, int &len)
+	*fmtptr++ = '%';
+	while (fmtsrc[len] && !isalpha((char)fmtsrc[len]))
 	{
-	// number of parameters ( for NEXT_PARAM macro )
+		if (len >= sizeof(fmt)-2)
-	int paramCount = *params / sizeof(cell);
+			break;
+		*fmtptr++ = static_cast<char>(fmtsrc[len++]);
+	} 
+	//get the final character
+	ctrl_code = fmtsrc[len++];
+	//inc the source pointer
+	*fmtstr = &(fmtsrc[len]);
+	//finalize the string
+	*fmtptr++ = ctrl_code;
+	*fmtptr = '\0';
+	len = 0;
+	//reset the format pointer
+	fmtptr = fmt[level];
+
+	char *tmp_buf = buffer[level];
+
+	//we now have the format
+	switch (ctrl_code)
+	{
+	case 's':
+		{
+			get_amxstring_r(amx, _addr, tmp_buf, 2047);
+			return _snprintf(output, maxlen, fmtptr, tmp_buf);
+			break;
+		}
+	case 'g':
+	case 'f':
+		{
+			return _snprintf(output, maxlen, fmtptr, *(REAL *)addr);
+			break;
+		}
+	case 'i':
+	case 'd':
+	case 'c':
+		{
+			return _snprintf(output, maxlen, fmtptr, (int)addr[0]);
+			break;
+		}
+	case 'L':
+		{
+			const char *pLangName = NULL;
+			const char *def = NULL, *key = NULL;
 			int status;
-	
+			int tmpLen;
-	// the output buffer
+			if (addr[0] == LANG_PLAYER)
-	static char outbuf[4096];
-	char *outptr = outbuf;
-	cell *src = get_amxaddr(amx, params[parm++]);
-
-	while (*src)
-	{
-		if (*src == '%')
-		{
-			++src;
-			if (*src == 'L')
-			{
-				cell langName = params[parm];		// "en" case (langName contains the address to the string)
-				NEXT_PARAM();
-				cell *pAmxLangName = get_amxaddr(amx, params[parm++]);	// other cases
-				const char *cpLangName=NULL;
-				// Handle player ids (1-32) and server language
-				
-				if (*pAmxLangName == LANG_PLAYER)	// LANG_PLAYER
 			{
 				if ( (int)CVAR_GET_FLOAT("amx_client_languages") == 0 )
 				{
-						cpLangName = g_vault.get("server_language");
+					pLangName = g_vault.get("server_language");
 				} else {
-						cpLangName = ENTITY_KEYVALUE(GET_PLAYER_POINTER_I(m_CurGlobId)->pEdict, "lang");
+					pLangName = ENTITY_KEYVALUE(GET_PLAYER_POINTER_I(g_langMngr.GetDefLang())->pEdict, "lang");
 				}
-				}
+			} else if (addr[0] == LANG_SERVER) {
-				else if (*pAmxLangName == LANG_SERVER) // LANG_SERVER
+				pLangName = g_vault.get("server_language");
-				{
+			} else if (addr[0] >= 1 && addr[0] <= gpGlobals->maxClients) {
-					cpLangName = g_vault.get("server_language");
-				}
-				else if (*pAmxLangName >= 1 && *pAmxLangName <= 32) // Direct Client Id
-				{
 				if ( (int)CVAR_GET_FLOAT("amx_client_languages") == 0 )
 				{
-						cpLangName = g_vault.get("server_language");
+					pLangName = g_vault.get("server_language");
 				} else {
-						cpLangName = ENTITY_KEYVALUE(GET_PLAYER_POINTER_I(*pAmxLangName)->pEdict, "lang");
+					pLangName = ENTITY_KEYVALUE(GET_PLAYER_POINTER_I(addr[0])->pEdict, "lang");
 				}
-				} else {	// Language Name
+			} else {
-					int tmplen = 0;
+				pLangName = get_amxstring(amx, _addr, 0, tmpLen);
-					cpLangName = get_amxstring(amx, langName, 2, tmplen);
 			}
-				
+			if (!pLangName || !isalpha(pLangName[0]))
-				if (!cpLangName || strlen(cpLangName) < 1)
+				pLangName = "en";
-					cpLangName = "en";
+			//next parameter!
-				
+			_addr = params[*param];
-				int tmplen = 0;
+			(*param)++;
-				NEXT_PARAM();
+			key = get_amxstring(amx, _addr, 1, tmpLen);
-				char *key = get_amxstring(amx, params[parm++], 1, tmplen);
+			def = g_langMngr.GetDef(pLangName, key, status);
-				const char *def = GetDef(cpLangName, key, status);
 				
 			if (def == NULL)
 			{
 				bool a = true;
 				if (status == LANG_STATUS_LNOTFOUND)
 				{
-						AMXXLOG_Log("[AMXX] Language \"%s\" not found", cpLangName);
+					AMXXLOG_Log("[AMXX] Language \"%s\" not found", pLangName);
-					}
+				} else if (status == LANG_STATUS_KLNOTFOUND) {
-					else if (status == LANG_STATUS_KLNOTFOUND)
-					{
 					a = false;
-						AMXXLOG_Log("[AMXX] Language key \"%s\" not found for language \"%s\"", key, cpLangName);
+					AMXXLOG_Log("[AMXX] Language key \"%s\" not found for language \"%s\"", key, pLangName);
 				}
 
-					if (*pAmxLangName != LANG_SERVER)
+				if (addr[0] != LANG_SERVER)
-					{
+					def = g_langMngr.GetDef(g_vault.get("server_language"), key, status);
-						def = GetDef(g_vault.get("server_language"), key, status);
-					}
 				
-					if (!def && (strcmp(cpLangName, "en") != 0 && strcmp(g_vault.get("server_language"), "en") != 0))
+				if (!def && (strcmp(pLangName, "en") != 0 && strcmp(g_vault.get("server_language"), "en") != 0))
-					{
+					def = g_langMngr.GetDef("en", key, status);
-						def = GetDef("en", key, status);
-					}
 
 				if (!def)
 				{
-						static char buf[512];
+					return _snprintf(output, maxlen, "ML_NOTFOUND: %s", key);
-						CHECK_PTR((char*)(buf + 17 + strlen(key)), buf, sizeof(buf));
-						sprintf(buf, "ML_LNOTFOUND: %s", key);
-						def = buf;
-						
 					if (a)
 						AMXXLOG_Log("[AMXX] Language key \"%s\" not found, check \"%s\"", key, GetFileName(amx));
 				}				
 			}
-
+			return do_amx_format(amx, params, param, &def, output, maxlen, level + 1);
-				while (*def)
-				{
-					if (*def == '%')
-					{
-						++def;
-						if (*def == '%' || *def == 0)
-						{
-							*outptr++ = '%';
-							++def;
-						} else {
-							static char format[32];
-							format[0] = '%';
-							char *ptr = format + 1;
-							
-							while (ptr-format<sizeof(format) && !isalpha(*ptr++ = *def++))
-								/*nothing*/;
-							ZEROTERM(format);
-
-							*ptr = 0;
-							
-							switch (*(ptr - 1))
-							{
-								case 's':
-								{
-									static char tmpString[4096];
-									char *tmpPtr = tmpString;
-									NEXT_PARAM();
-									cell *tmpCell = get_amxaddr(amx, params[parm++]);
-									while (tmpPtr-tmpString < sizeof(tmpString) && *tmpCell)
-											*tmpPtr++ = static_cast<char>(*tmpCell++);
-									*tmpPtr = 0;
-									_snprintf(outptr, sizeof(outbuf)-(outptr-outbuf)-1, format, tmpString);
-									ZEROTERM(outbuf);
-									break;
-								}
-								case 'g':
-								case 'f':
-								{
-									NEXT_PARAM();
-									_snprintf(outptr, sizeof(outbuf)-(outptr-outbuf)-1, format, *(REAL*)get_amxaddr(amx, params[parm++]));
-									ZEROTERM(outbuf);
-									break;
-								}
-								case 'i':
-								case 'd':
-								case 'c':
-								{
-									NEXT_PARAM();
-									_snprintf(outptr, sizeof(outbuf)-(outptr-outbuf)-1, format, (int)*get_amxaddr(amx, params[parm++]));
-									ZEROTERM(outbuf);
-									break;
 		}
 	default:
 		{
-									CHECK_OUTPTR(strlen(format) + 1);
+			return _snprintf(output, maxlen, "%s", fmtptr);
-									strcpy(outptr, format);	
 			break;
 		}
 	}
+}
 
-							outptr += strlen(outptr);
+#define DUMP_CP_BUFFER(expr) \
+	if (sofar > 0) { \
+		if (sofar <= (int)maxlen) { \
+			memcpy(output, save, sofar); \
+			output += sofar; \
+			maxlen -= sofar; \
+			sofar = 0; \
+		} else { \
+			expr; \
+		} \
 	}
-					}
+
-					else if (*def == '^')
+size_t do_amx_format(AMX *amx, cell *params, int *param, const char **lex, char *output, size_t maxlen, int level)
 {
-						++def;
+	int sofar = 0;
+	size_t written;
+	size_t orig_maxlen = maxlen;
+	const char *save = *lex;
+	register const char *lexptr = save;
 
-						switch (*def)
+	while (*lexptr)
+	{
+		switch (*lexptr)
+		{
+		case '%':
+			{
+				lexptr++;
+				if (*lexptr == '%' || *lexptr == '\0')
+				{
+					sofar+=2;
+				} else {
+					DUMP_CP_BUFFER(break);
+					written = do_amx_format_parameter(amx, params, &lexptr, param, output, maxlen, level + 1);
+					output += written;
+					maxlen -= written;
+					save = lexptr;
+				}
+				break;
+			}
+		case '^':
+			{
+				if (level)
+				{
+					DUMP_CP_BUFFER(break);
+					lexptr++;
+					switch (*lexptr)
 					{
 						case 'n':
-								CHECK_OUTPTR(1);
+						{
-								*outptr++ = '\n';
+							*output++ = '\n';
 							break;
+						}
 						case 't':
-								CHECK_OUTPTR(1);
-								*outptr++ = '\t';
-								break;
-							case '^':
-								CHECK_OUTPTR(1);
-								*outptr++ = '^';
-								break;
-							default:
-								CHECK_OUTPTR(2);
-								*outptr++ = '^';
-								*outptr++ = *def;
-								break;
-						}
-						
-						++def;
-					} else {
-						CHECK_OUTPTR(1);
-						*outptr++ = *def++;
-					}
-				}
-			} else {
-				static char tmpString[4096];
-				char *tmpPtr = tmpString;
-				int tmpLen = 0;
-				static char format[32] = {'%'};
-				char *ptr = format + 1;
-				
-				if (*src != '%')
 						{
-					while (*src != 0 && ptr-format<sizeof(format) && !isalpha(*ptr++ = static_cast<char>(*src++)))
+							*output++ = '\t';
-						/*nothing*/;
-					*ptr = 0;
-					ZEROTERM(format);
-					--src;
-					
-					switch (*(ptr - 1))
-					{
-						case 's':
-						{
-							NEXT_PARAM();
-							cell *tmpCell = get_amxaddr(amx, params[parm++]);
-							while (tmpPtr-tmpString<sizeof(tmpString) && *tmpCell)
-								*tmpPtr++ = static_cast<char>(*tmpCell++);
-							*tmpPtr = 0;
-							_snprintf(outptr, sizeof(outbuf)-(outptr-outbuf)-1, format, tmpString);
-							ZEROTERM(outbuf);
-							break;
-						}
-						case 'g':
-						case 'f':
-						{
-							NEXT_PARAM();
-							_snprintf(outptr, sizeof(outbuf)-(outptr-outbuf)-1, format, *(REAL*)get_amxaddr(amx, params[parm++]));
-							break;
-						}
-						case 'i':
-						case 'd':
-						case 'c':
-						{
-							NEXT_PARAM();
-							_snprintf(outptr, sizeof(outbuf)-(outptr-outbuf)-1, format, (int)*get_amxaddr(amx, params[parm++]));
 							break;
 						}
 						default:
 						{
-							CHECK_OUTPTR(strlen(format) + 1);
+							*output++ = *lexptr;
-							strcpy(outptr, format);
 							break;
 						}
 					}
+					lexptr++;
+					maxlen--;
+					save = lexptr;
+					break;
+				}
+			}
+		default:
+			{
+				lexptr++;
+				sofar++;
+			}
+		}
+	}
+	DUMP_CP_BUFFER(;);
+	*output = '\0';
 
-					outptr += strlen(outptr);
+	*lex = lexptr;
-				} else {
+
-					CHECK_OUTPTR(1);
+	return (orig_maxlen-maxlen);
-					*outptr++ = '%';
-				}
-			}
-		} else {
-			CHECK_OUTPTR(1);
-			*outptr++ = static_cast<char>(*src);
-		}
-		++src;
 }
 
-	len = outptr - outbuf;
+char * CLangMngr::FormatAmxString(AMX *amx, cell *params, int parm, int &len)
-	CHECK_OUTPTR(1);
+{
-	*outptr++ = 0;
+	//do an initial run through all this 
+	static char mystr[4096];
+	static char outbuf[4096];
+	const char *ptr = mystr;
+
+	get_amxstring_r(amx, params[parm++], mystr, sizeof(mystr)-1);
+
+	len = do_amx_format(amx, params, &parm, &ptr, outbuf, sizeof(outbuf)-1, 0);
 
 	return outbuf;
 }

amxmodx/CLang.h

@@ -59,13 +59,12 @@ public:
 	defentry() : definition(NULL)
 	{
 	};
+	defentry(const defentry &src)
+	{
+		definition = src.definition;
+	}
 	~defentry()
 	{
-		if (definition)
-		{
-			delete definition;
-			definition = NULL;
-		}
 	}
 	String *definition;
 };
@@ -184,6 +183,8 @@ public:
 	// When a language id in a format string in FormatAmxString is LANG_PLAYER, the glob id decides which language to take.
 	void SetDefLang(int id);
 
+	inline int GetDefLang() const { return m_CurGlobId; }
+
 	// Reset
 	void Clear();
 

amxmodx/CString.h

@@ -107,8 +107,10 @@ public:
 		{
 			clear();
 		} else {
-			Grow(strlen(d) + 1, false);
+			size_t len = strlen(d);
-			strcpy(v, d);
+			Grow(len + 1, false);
+			memcpy(v, d, len);
+			v[len] = '\0';
 		}
 	}
 

amxmodx/amxmodx.h

@@ -257,6 +257,7 @@ char* format_amxstring(AMX *amx, cell *params, int parm, int& len);
 AMX* get_amxscript(int, void**, const char**);
 const char* get_amxscriptname(AMX* amx);
 char* get_amxstring(AMX *amx, cell amx_addr, int id, int& len);
+size_t get_amxstring_r(AMX *amx, cell amx_addr, char *destination, int maxlen);
 
 int amxstring_len(cell* cstr);
 int load_amxscript(AMX* amx, void** program, const char* path, char error[64], int debug);

amxmodx/meta_api.cpp

@@ -249,10 +249,14 @@ int	C_Spawn(edict_t *pent)
 	// ###### Load lang
 	char file[256];
 	g_langMngr.LoadCache(build_pathname_r(file, sizeof(file) - 1, "%s/dictionary.cache", get_localinfo("amxx_datadir", "addons/amxmodx/data")));
+	DWORD stop,start=GetTickCount();
 	if (!g_langMngr.Load(build_pathname_r(file, sizeof(file) - 1, "%s/languages.dat", get_localinfo("amxmodx_datadir", "addons/amxmodx/data"))))
 	{
+		LOG_MESSAGE(PLID, "Cache invalidated!");
 		g_langMngr.InvalidateCache();
 	}
+	stop=GetTickCount();
+	LOG_MESSAGE(PLID, "CacheDB load time: %d milliseconds", stop-start);
 
 	// ###### Initialize commands prefixes
 	g_commands.registerPrefix("amx");

amxmodx/sh_tinyhash.h

@@ -13,7 +13,7 @@
 
 #include "sh_list.h"
 
-#define _T_INIT_HASH_SIZE	32
+#define _T_INIT_HASH_SIZE	128
 
 //namespace SourceHook
 //{
@@ -43,7 +43,7 @@
 			K key;
 			V val;
 		};
-		typedef List<THashNode *> *	NodePtr;
+		typedef List<THashNode> *	NodePtr;
 	public:
 		THash() : m_Buckets(NULL), m_numBuckets(0), m_percentUsed(0.0f), m_NumItems(0)
 		{
@@ -93,15 +93,10 @@
 	private:
 		void _Clear()
 		{
-			List<THashNode *>::iterator iter, begin, end;
 			for (size_t i=0; i<m_numBuckets; i++)
 			{
 				if (m_Buckets[i])
 				{
-					begin = m_Buckets[i]->begin();
-					end = m_Buckets[i]->end();
-					for (iter=begin; iter!=end; iter++)
-						delete (*iter);
 					delete m_Buckets[i];
 					m_Buckets[i] = NULL;
 				}
@@ -118,25 +113,33 @@
 			THashNode *pNode = NULL;
 			if (!m_Buckets[place])
 			{
-				m_Buckets[place] = new List<THashNode *>;
+				m_Buckets[place] = new List<THashNode>;
-				pNode = new THashNode(key, V());
+				m_Buckets[place]->push_back(THashNode(key, V()));
-				m_Buckets[place]->push_back(pNode);
 				m_percentUsed += (1.0f / (float)m_numBuckets);
 				m_NumItems++;
+				typename List<THashNode>::iterator iter;
+				iter = m_Buckets[place]->end();
+				iter--;
+				pNode = &(*iter);
 			} else {
-				typename List<THashNode *>::iterator iter;
+				typename List<THashNode>::iterator iter, end=m_Buckets[place]->end();
-				for (iter=m_Buckets[place]->begin(); iter!=m_Buckets[place]->end(); iter++)
+				for (iter=m_Buckets[place]->begin(); iter!=end; iter++)
 				{
-					if (Compare((*iter)->key, key) == 0)
+					if (Compare((*iter).key, key) == 0)
-						return (*iter);
+						return &(*iter);
 				}
 				//node does not exist
-				pNode = new THashNode(key, V());
+				m_Buckets[place]->push_back(THashNode(key, V()));
-				m_Buckets[place]->push_back(pNode);
 				m_NumItems++;
+				iter = m_Buckets[place]->end();
+				iter--;
+				pNode = &(*iter);
 			}
 			if (PercentUsed() > 0.75f)
+			{
 				_Refactor();
+				return _FindOrInsert(key);
+			}
 			return pNode;
 		}
 		void _Refactor()
@@ -151,7 +154,7 @@
 			} else {
 				size_t oldSize = m_numBuckets;
 				m_numBuckets *= 2;
-				typename List<THashNode *>::iterator iter;
+				typename List<THashNode>::iterator iter, end;
 				size_t place;
 				THashNode *pHashNode;
 				NodePtr *temp = new NodePtr[m_numBuckets];
@@ -164,18 +167,19 @@
 					if (m_Buckets[i])
 					{
 						//go through the list of items
-						for (iter = m_Buckets[i]->begin(); iter != m_Buckets[i]->end(); iter++)
+						end = m_Buckets[i]->end();
+						for (iter = m_Buckets[i]->begin(); iter!=end; iter++)
 						{
-							pHashNode = (*iter);
+							pHashNode = &(*iter);
 							//rehash it with the new bucket filter
 							place = HashFunction(pHashNode->key) % m_numBuckets;
 							//add it to the new hash table
 							if (!temp[place])
 							{
-								temp[place] = new List<THashNode *>;
+								temp[place] = new List<THashNode>;
 								m_percentUsed += (1.0f / (float)m_numBuckets);
 							}
-							temp[place]->push_back(pHashNode);
+							temp[place]->push_back((*iter));
 						}
 						//delete that bucket!
 						delete m_Buckets[i];
@@ -219,19 +223,19 @@
 			}
 			const THashNode & operator * () const
 			{
-				return *(*iter);
+				return (*iter);
 			}
 			THashNode & operator * ()
 			{
-				return *(*iter);
+				return (*iter);
 			}
 			const THashNode * operator ->() const
 			{
-				return (*iter);
+				return &(*iter);
 			}
 			THashNode * operator ->()
 			{
-				return (*iter);
+				return &(*iter);
 			}
 			bool operator ==(const iterator &where) const
 			{
@@ -308,7 +312,7 @@
 			}
 		private:
 			int curbucket;
-			typename List<THashNode *>::iterator iter;
+			typename List<THashNode>::iterator iter;
 			THash *hash;
 			bool end;
 		};

amxmodx/string.cpp

@@ -87,6 +87,20 @@ int set_amxstring(AMX *amx, cell amx_addr, const char *source, int max)
 	return dest - start;
 }
 
+size_t get_amxstring_r(AMX *amx, cell amx_addr, char *destination, int maxlen)
+{
+	register cell *source = (cell *)(amx->base + (int)(((AMX_HEADER *)amx->base)->dat + amx_addr));
+	register char *dest = destination;
+	char *start = dest;
+	
+	while (*source && maxlen-- > 0)
+		*dest++=(char)(*source++);
+	if (dest)
+		*dest = '\0';
+
+	return --dest - start;
+}
+
 char* get_amxstring(AMX *amx, cell amx_addr, int id, int& len)
 {
 	static char buffor[4][3072];