Don't leak references to Strings on script errors (#583)

openclonk · Feb 4, 2016 · 6866375c6742d2be1bbd886306ffc69d14b8cafc · 6866375
1 parent e2c6358
commit 6866375c6742d2be1bbd886306ffc69d14b8cafc
Unified Split

Showing with 73 additions and 41 deletions.

+12 −12 src/script/C4AulParse.cpp

+1 −27 src/script/C4AulScriptFunc.cpp

+60 −2 src/script/C4AulScriptFunc.h
diff --git a/src/script/C4AulParse.cpp b/src/script/C4AulParse.cpp
@@ -1075,7 +1075,7 @@ C4V_Type C4AulParse::GetLastRetType(C4V_Type to)
 C4AulBCC C4AulParse::MakeSetter(bool fLeaveValue)
 {
-  
-	if(Type != PARSER) { C4AulBCC Dummy; Dummy.bccType = AB_ERR; return Dummy; }
+  
+	if(Type != PARSER) { return C4AulBCC(AB_ERR, 0); }
 	C4AulBCC Value = *(Fn->GetLastCode()), Setter = Value;
 	// Check type
 	switch (Value.bccType)
@@ -1090,11 +1090,9 @@ C4AulBCC C4AulParse::MakeSetter(bool fLeaveValue)
 	case AB_STACK_SET: Setter.bccType = AB_STACK_SET; break;
 	case AB_LOCALN:
 		Setter.bccType = AB_LOCALN_SET;
-  
-		Setter.Par.s->IncRef(); // so string isn't dropped by RemoveLastBCC, see also C4AulScript::AddBCC
 		break;
 	case AB_PROP:
 		Setter.bccType = AB_PROP_SET;
-  
-		Setter.Par.s->IncRef(); // so string isn't dropped by RemoveLastBCC, see also C4AulScript::AddBCC
 		break;
 	case AB_GLOBALN: Setter.bccType = AB_GLOBALN_SET; break;
 	default: 
@@ -2391,15 +2389,17 @@ void C4AulParse::Parse_Expression(int iParentPrio)
 				Fn->GetLastCode()->Par.i = - Fn->GetLastCode()->Par.i;
 				break;
 			}
-  
-		// changer? make a setter BCC, leave value for operator
-  
-		C4AulBCC Changer;
-  
-		if(op->Changer)
-  
-			Changer = MakeSetter(true);
-  
-		// write byte code
-  
-		AddBCC(op->Code, 0);
-  
-		// writter setter
-  
-		if(op->Changer)
-  
-			AddBCC(Changer.bccType, Changer.Par.X);
+  
+		{
+  
+			// changer? make a setter BCC, leave value for operator
+  
+			C4AulBCC Changer;
+  
+			if(op->Changer)
+  
+				Changer = MakeSetter(true);
+  
+			// write byte code
+  
+			AddBCC(op->Code, 0);
+  
+			// writter setter
+  
+			if(op->Changer)
+  
+				AddBCC(Changer.bccType, Changer.Par.X);
+  
+		}
 		break;
 	case ATT_BOPEN:
 		Shift();

diff --git a/src/script/C4AulScriptFunc.cpp b/src/script/C4AulScriptFunc.cpp
@@ -62,38 +62,12 @@ void C4AulScriptFunc::SetOverloaded(C4AulFunc * f)
 void C4AulScriptFunc::AddBCC(C4AulBCCType eType, intptr_t X, const char * SPos)
 {
 	// store chunk
-  
-	C4AulBCC bcc;
-  
-	bcc.bccType = eType;
-  
-	bcc.Par.X = X;
-  
-	Code.push_back(bcc);
+  
+	Code.emplace_back(eType, X);
 	PosForCode.push_back(SPos);
-  
-
-  
-	switch (eType)
-  
-	{
-  
-	case AB_STRING: case AB_CALL: case AB_CALLFS: case AB_LOCALN: case AB_PROP:
-  
-	/* case AB_LOCALN_SET/AB_PROP_SET: -- expected to already have a reference upon creation, see MakeSetter */
-  
-		bcc.Par.s->IncRef();
-  
-		break;
-  
-	case AB_CARRAY:
-  
-		bcc.Par.a->IncRef();
-  
-		break;
-  
-	default: break;
-  
-	}
 }
 void C4AulScriptFunc::RemoveLastBCC()
 {
-  
-	C4AulBCC *pBCC = &Code.back();
-  
-	switch (pBCC->bccType)
-  
-	{
-  
-	case AB_STRING: case AB_CALL: case AB_CALLFS: case AB_LOCALN: case AB_LOCALN_SET: case AB_PROP: case AB_PROP_SET:
-  
-		pBCC->Par.s->DecRef();
-  
-		break;
-  
-	case AB_CARRAY:
-  
-		pBCC->Par.a->DecRef();
-  
-		break;
-  
-	default: break;
-  
-	}
 	Code.pop_back();
 	PosForCode.pop_back();
 }

diff --git a/src/script/C4AulScriptFunc.h b/src/script/C4AulScriptFunc.h
@@ -93,18 +93,76 @@ enum C4AulBCCType
 };
 // byte code chunk
-  
-struct C4AulBCC
+  
+class C4AulBCC
 {
+  
+public:
 	C4AulBCCType bccType; // chunk type
 	union
 	{
+  
+		intptr_t X;
 		int32_t i;
 		C4String * s;
 		C4PropList * p;
 		C4ValueArray * a;
 		C4AulFunc * f;
-  
-		intptr_t X;
 	} Par;    // extra info
+  
+	C4AulBCC(): bccType(AB_ERR) { }
+  
+	C4AulBCC(C4AulBCCType bccType, intptr_t X): bccType(bccType), Par{X}
+  
+	{
+  
+		IncRef();
+  
+	}
+  
+	C4AulBCC(const C4AulBCC & from): C4AulBCC(from.bccType, from.Par.X) { }
+  
+	C4AulBCC & operator = (const C4AulBCC & from)
+  
+	{
+  
+		DecRef();
+  
+		bccType = from.bccType;
+  
+		Par = from.Par;
+  
+		IncRef();
+  
+		return *this;
+  
+	}
+  
+	C4AulBCC(C4AulBCC && from): bccType(from.bccType), Par(from.Par)
+  
+	{
+  
+		from.bccType = AB_ERR;
+  
+	}
+  
+	C4AulBCC & operator = (C4AulBCC && from)
+  
+	{
+  
+		DecRef();
+  
+		bccType = from.bccType;
+  
+		Par = from.Par;
+  
+		from.bccType = AB_ERR;
+  
+		return *this;
+  
+	}
+  
+	~C4AulBCC()
+  
+	{
+  
+		DecRef();
+  
+	}
+  
+private:
+  
+	void IncRef()
+  
+	{
+  
+		switch (bccType)
+  
+		{
+  
+		case AB_STRING: case AB_CALL: case AB_CALLFS: case AB_LOCALN: case AB_LOCALN_SET: case AB_PROP: case AB_PROP_SET:
+  
+			Par.s->IncRef();
+  
+			break;
+  
+		case AB_CARRAY:
+  
+			Par.a->IncRef();
+  
+			break;
+  
+		default: break;
+  
+		}
+  
+	}
+  
+	void DecRef()
+  
+	{
+  
+		switch (bccType)
+  
+		{
+  
+		case AB_STRING: case AB_CALL: case AB_CALLFS: case AB_LOCALN: case AB_LOCALN_SET: case AB_PROP: case AB_PROP_SET:
+  
+			Par.s->DecRef();
+  
+			break;
+  
+		case AB_CARRAY:
+  
+			Par.a->DecRef();
+  
+			break;
+  
+		default: break;
+  
+		}
+  
+	}
 };
 // script function class