Fix memory leak from string handling in VScriptExecute

scripting/vscript.sp

@@ -2,7 +2,7 @@
 
 #include "include/vscript.inc"
 
-#define PLUGIN_VERSION			"1.8.3"
+#define PLUGIN_VERSION			"1.8.4"
 #define PLUGIN_VERSION_REVISION	"manual"
 
 char g_sOperatingSystem[16];
@@ -583,10 +583,18 @@ public any Native_Execute_AddParam(Handle hPlugin, int iNumParams)
 
 public any Native_Execute_AddParamString(Handle hPlugin, int iNumParams)
 {
+	VScriptExecute aExecute = GetNativeCell(1);
+
+	int iLength;
+	GetNativeStringLength(3, iLength);
+
+	char[] sBuffer = new char[iLength + 1];
+	GetNativeString(3, sBuffer, iLength + 1);
+
 	ExecuteParam param;
 	param.nType = GetNativeCell(2);
-	param.pValue = StoreNativeStringToMemory(3).Address;
-	Execute_AddParam(GetNativeCell(1), param);
+	int iParam = Execute_AddParam(aExecute, param);
+	Execute_SetParamString(aExecute, iParam, sBuffer);
 	return 0;
 }
 
@@ -610,10 +618,19 @@ public any Native_Execute_SetParam(Handle hPlugin, int iNumParams)
 
 public any Native_Execute_SetParamString(Handle hPlugin, int iNumParams)
 {
+	VScriptExecute aExecute = GetNativeCell(1);
+	int iParam = GetNativeCell(2);
+
+	int iLength;
+	GetNativeStringLength(4, iLength);
+
+	char[] sBuffer = new char[iLength + 1];
+	GetNativeString(4, sBuffer, iLength + 1);
+
 	ExecuteParam param;
 	param.nType = GetNativeCell(3);
-	param.pValue = StoreNativeStringToMemory(4).Address;
-	Execute_SetParam(GetNativeCell(1), GetNativeCell(2), param);
+	Execute_SetParam(aExecute, iParam, param);
+	Execute_SetParamString(aExecute, iParam, sBuffer);
 	return 0;
 }
 
@@ -649,12 +666,9 @@ public any Native_Execute_ReturnValueGet(Handle hPlugin, int iNumParams)
 
 public any Native_Execute_GetReturnString(Handle hPlugin, int iNumParams)
 {
-	Execute execute;
-	Execute_GetInfo(GetNativeCell(1), execute);
-
 	int iLength = GetNativeCell(3);
 	char[] sBuffer = new char[iLength];
-	LoadStringFromAddress(execute.nReturn.pValue, sBuffer, iLength);
+	Execute_GetParamString(GetNativeCell(1), 0, sBuffer, iLength);
 	SetNativeString(2, sBuffer, iLength);
 	return 0;
 }

scripting/vscript/execute.sp

@@ -1,25 +1,19 @@
-// VScriptExecute is an ArrayList, with index 0 as Execute, index 1 and above as ExecuteParam
+// VScriptExecute is an ArrayList, with index 0 as Execute, index 1 to arg count as ExecuteParam, and rest above as value of string
 
 enum struct ExecuteParam
 {
 	fieldtype_t nType;
 	any nValue;			// int
-	Address pValue;		// string
 	float vecValue[3];	// vector
-
-	void Delete()
-	{
-		// Should ideally also be called when VScriptExecute is deleted
-		if (this.pValue)
-			Memory_DeleteAddress(this.pValue);
-	}
+	int iStringCount;	// Amount of array indexs used to store string value
 }
 
 enum struct Execute
 {
+	ExecuteParam nReturn;	// must be first in this struct
 	HSCRIPT pHScript;
-	ExecuteParam nReturn;
 	HSCRIPT hScope;
+	int iNumParams;
 }
 
 static Handle g_hSDKCallExecuteFunction;
@@ -40,6 +34,20 @@ VScriptExecute Execute_Create(HSCRIPT pHScript, HSCRIPT hScope)
 	return view_as<VScriptExecute>(aExecute);
 }
 
+static void Execute_InsertAt(VScriptExecute aExecute, int iParam, any[] nValues)
+{
+	int iLength = view_as<ArrayList>(aExecute).Length;
+	if (iLength == iParam)
+	{
+		view_as<ArrayList>(aExecute).PushArray(nValues);
+	}
+	else
+	{
+		view_as<ArrayList>(aExecute).ShiftUp(iParam);
+		view_as<ArrayList>(aExecute).SetArray(iParam, nValues);
+	}
+}
+
 void Execute_GetInfo(VScriptExecute aExecute, Execute execute)
 {
 	view_as<ArrayList>(aExecute).GetArray(0, execute);
@@ -52,12 +60,20 @@ static void Execute_SetInfo(VScriptExecute aExecute, Execute execute)
 
 int Execute_GetParamCount(VScriptExecute aExecute)
 {
-	return view_as<ArrayList>(aExecute).Length - 1;
+	return view_as<ArrayList>(aExecute).Get(0, Execute::iNumParams);
 }
 
-void Execute_AddParam(VScriptExecute aExecute, ExecuteParam param)
+static void Execute_SetParamCount(VScriptExecute aExecute, int iNumParams)
 {
-	view_as<ArrayList>(aExecute).PushArray(param);
+	view_as<ArrayList>(aExecute).Set(0, iNumParams, Execute::iNumParams);
+}
+
+int Execute_AddParam(VScriptExecute aExecute, ExecuteParam param)
+{
+	int iNumParams = Execute_GetParamCount(aExecute) + 1;
+	Execute_SetParamCount(aExecute, iNumParams);
+	Execute_InsertAt(aExecute, iNumParams, param);
+	return iNumParams;
 }
 
 void Execute_SetParam(VScriptExecute aExecute, int iParam, ExecuteParam param)
@@ -66,16 +82,72 @@ void Execute_SetParam(VScriptExecute aExecute, int iParam, ExecuteParam param)
 	{
 		// Fill any new params between as void
 		ExecuteParam nothing;
-		view_as<ArrayList>(aExecute).PushArray(nothing);
+		Execute_SetParamCount(aExecute, i);
+		Execute_InsertAt(aExecute, i, nothing);
 	}
 
-	ExecuteParam del;
-	view_as<ArrayList>(aExecute).GetArray(iParam, del);
-	del.Delete();
-
+	Execute_ClearParamString(aExecute, iParam);
 	view_as<ArrayList>(aExecute).SetArray(iParam, param);
 }
 
+int Execute_GetParamStringCount(VScriptExecute aExecute, int iParam)
+{
+	return view_as<ArrayList>(aExecute).Get(iParam, ExecuteParam::iStringCount);
+}
+
+void Execute_GetParamString(VScriptExecute aExecute, int iParam, char[] sBuffer, int iLength)
+{
+	int iStartingIndex = Execute_GetParamCount(aExecute) + 1;
+	for (int i = 0; i < iParam; i++)
+		iStartingIndex += Execute_GetParamStringCount(aExecute, i);
+
+	int iStringCount = Execute_GetParamStringCount(aExecute, iParam);
+	for (int i = iStartingIndex; i < iStartingIndex + iStringCount; i++)
+	{
+		char sValue[sizeof(Execute)];
+		view_as<ArrayList>(aExecute).GetString(i, sValue, sizeof(sValue));
+		StrCat(sBuffer, iLength, sValue);
+	}
+}
+
+int Execute_SetParamString(VScriptExecute aExecute, int iParam, const char[] sBuffer)
+{
+	Execute_ClearParamString(aExecute, iParam);
+
+	int iStartingIndex = Execute_GetParamCount(aExecute) + 1;
+	for (int i = 0; i < iParam; i++)
+		iStartingIndex += Execute_GetParamStringCount(aExecute, i);
+
+	// How many indexes do we need?
+	int iStringCount = RoundToCeil(float(strlen(sBuffer)) / float(sizeof(Execute) - 1));
+	view_as<ArrayList>(aExecute).Set(iParam, iStringCount, ExecuteParam::iStringCount);
+
+	for (int i = iStartingIndex; i < iStartingIndex + iStringCount; i++)
+	{
+		char sValue[sizeof(Execute)];
+		strcopy(sValue, sizeof(sValue), sBuffer[(i - iStartingIndex) * (sizeof(Execute) - 1)]);
+		Execute_InsertAt(aExecute, i, view_as<any>(sValue));
+	}
+
+	return iStringCount;
+}
+
+void Execute_ClearParamString(VScriptExecute aExecute, int iParam)
+{
+	int iStringCount = Execute_GetParamStringCount(aExecute, iParam);
+	if (!iStringCount)
+		return;
+
+	int iStartingIndex = Execute_GetParamCount(aExecute) + 1;
+	for (int i = 0; i < iParam; i++)
+		iStartingIndex += Execute_GetParamStringCount(aExecute, i);
+
+	for (int i = 0; i < iStringCount; i++)
+		view_as<ArrayList>(aExecute).Erase(iStartingIndex);
+
+	view_as<ArrayList>(aExecute).Set(iParam, 0, ExecuteParam::iStringCount);
+}
+
 ScriptStatus_t Execute_Execute(VScriptExecute aExecute)
 {
 	MemoryBlock hArgs = null;
@@ -105,7 +177,12 @@ ScriptStatus_t Execute_Execute(VScriptExecute aExecute)
 			}
 			case SMField_String:
 			{
-				nValue = param.pValue;
+				int iLength = Execute_GetParamStringCount(aExecute, iParam + 1) * (sizeof(Execute) - 1) + 1;
+				char[] sBuffer = new char[iLength];
+				Execute_GetParamString(aExecute, iParam + 1, sBuffer, iLength);
+
+				hValue[iParam] = CreateStringMemory(sBuffer);
+				nValue = hValue[iParam].Address;
 			}
 			case SMField_Vector:
 			{
@@ -120,6 +197,8 @@ ScriptStatus_t Execute_Execute(VScriptExecute aExecute)
 
 	ScriptStatus_t nStatus = SDKCall(g_hSDKCallExecuteFunction, GetScriptVM(), execute.pHScript, hArgs ? hArgs.Address : Address_Null, iNumParams, pReturn.Address, execute.hScope, true);
 
+	Execute_ClearParamString(aExecute, 0);	// Clear previous return string value
+
 	switch (Field_GetSMField(pReturn.nType))
 	{
 		case SMField_Void:
@@ -132,7 +211,11 @@ ScriptStatus_t Execute_Execute(VScriptExecute aExecute)
 		}
 		case SMField_String:
 		{
-			execute.nReturn.pValue = pReturn.nValue;
+			int iLength = pReturn.GetStringLength();
+			char[] sBuffer = new char[iLength];
+			pReturn.GetString(sBuffer, iLength);
+
+			execute.nReturn.iStringCount = Execute_SetParamString(aExecute, 0, sBuffer);
 		}
 		case SMField_Vector:
 		{

scripting/vscript/util.sp

@@ -29,6 +29,12 @@ int LoadStringFromAddress(Address pString, char[] sBuffer, int iMaxLen)
 	return iChar;
 }
 
+int LoadPointerStringLengthFromAddress(Address pPointer)
+{
+	Address pString = LoadFromAddress(pPointer, NumberType_Int32);
+	return LoadStringLengthFromAddress(pString);
+}
+
 int LoadStringLengthFromAddress(Address pString)
 {
 	int iChar;
@@ -67,18 +73,6 @@ void StoreNativePointerStringToAddress(Address pAddress, int iParam)
 	Memory_SetAddress(pAddress, hString);
 }
 
-MemoryBlock StoreNativeStringToMemory(int iParam)
-{
-	int iLength;
-	GetNativeStringLength(iParam, iLength);
-	iLength++;
-
-	char[] sBuffer = new char[iLength];
-	GetNativeString(iParam, sBuffer, iLength);
-
-	return CreateStringMemory(sBuffer);
-}
-
 void LoadVectorFromAddress(Address pVector, float vecBuffer[3])
 {
 	for (int i = 0; i < sizeof(vecBuffer); i++)

scripting/vscript/variant.sp

@@ -98,6 +98,11 @@ methodmap ScriptVariant_t < MemoryBlock
 		LoadPointerStringFromAddress(this.Address + view_as<Address>(g_iScriptVariant_union), sBuffer, iLength);
 	}
 
+	public int GetStringLength()
+	{
+		return LoadPointerStringLengthFromAddress(this.Address + view_as<Address>(g_iScriptVariant_union));
+	}
+
 	public void GetVector(float vecBuffer[3])
 	{
 		LoadVectorFromAddress(this.nValue, vecBuffer);

scripting/vscript_test.sp

@@ -5,7 +5,7 @@
 
 #define TEST_INTEGER	322
 #define TEST_FLOAT		3.14159
-#define TEST_CSTRING	"Message"
+#define TEST_CSTRING	"VScript Is Awesome!"
 
 #define TEST_BOOLEAN	true
 #define TEST_BOOLEAN_STR	"true"