Fixed memory leaks in expression parser

3 files changed, 144 insertions, 152 deletions

diff --git a/src/GCodes/GCodeBuffer/ExpressionParser.cpp b/src/GCodes/GCodeBuffer/ExpressionParser.cpp
index ce5e2fc5..25ef1a24 100644
--- a/src/GCodes/GCodeBuffer/ExpressionParser.cpp
+++ b/src/GCodes/GCodeBuffer/ExpressionParser.cpp
@@ -146,11 +146,11 @@ void ExpressionParser::ParseInternal(ExpressionValue& val, bool evaluate, uint8_
 			ParseInternal(val, evaluate, UnaryPriority);
 			if (val.GetType() == TypeCode::CString)
 			{
-				val.Set((int32_t)strlen(val.sVal));
+				val.SetInt((int32_t)strlen(val.sVal));
 			}
 			else if (val.GetType() == TypeCode::HeapString)
 			{
-				val.Set((int32_t)val.shVal.GetLength());
+				val.SetInt((int32_t)val.shVal.GetLength());
 			}
 			else
 			{
@@ -389,133 +389,130 @@ void ExpressionParser::ParseInternal(ExpressionValue& val, bool evaluate, uint8_
 
 				case '>':
 					BalanceTypes(val, val2, evaluate);
-					switch (val.GetType())
 					{
-					case TypeCode::Int32:
-						val.bVal = (val.iVal > val2.iVal);
-						break;
+						bool bResult;
+						switch (val.GetType())
+						{
+						case TypeCode::Int32:
+							bResult = (val.iVal > val2.iVal);
+							break;
 
-					case TypeCode::Float:
-						val.bVal = (val.fVal > val2.fVal);
-						break;
+						case TypeCode::Float:
+							bResult = (val.fVal > val2.fVal);
+							break;
 
-					case TypeCode::DateTime_tc:
-						val.bVal = val.Get56BitValue() > val2.Get56BitValue();
-						break;
+						case TypeCode::DateTime_tc:
+							bResult = val.Get56BitValue() > val2.Get56BitValue();
+							break;
 
-					case TypeCode::Bool:
-						val.bVal = (val.bVal && !val2.bVal);
-						break;
+						case TypeCode::Bool:
+							bResult = (val.bVal && !val2.bVal);
+							break;
 
-					default:
-						if (evaluate)
-						{
-							ThrowParseException("expected numeric or Boolean operands to comparison operator");
+						default:
+							if (evaluate)
+							{
+								ThrowParseException("expected numeric or Boolean operands to comparison operator");
+							}
+							bResult = false;
+							break;
 						}
-						val.bVal = false;
-						break;
-					}
-					val.SetType(TypeCode::Bool);
-					if (invert)
-					{
-						val.bVal = !val.bVal;
+						val.SetBool((invert) ? !bResult : bResult);
 					}
 					break;
 
 				case '<':
 					BalanceTypes(val, val2, evaluate);
-					switch (val.GetType())
 					{
-					case TypeCode::Int32:
-						val.bVal = (val.iVal < val2.iVal);
-						break;
-
-					case TypeCode::Float:
-						val.bVal = (val.fVal < val2.fVal);
-						break;
-
-					case TypeCode::DateTime_tc:
-						val.bVal = val.Get56BitValue() < val2.Get56BitValue();
-						break;
-
-					case TypeCode::Bool:
-						val.bVal = (!val.bVal && val2.bVal);
-						break;
-
-					default:
-						if (evaluate)
-						{
-							ThrowParseException("expected numeric or Boolean operands to comparison operator");
-						}
-						val.bVal = false;
-						break;
-					}
-					val.SetType(TypeCode::Bool);
-					if (invert)
-					{
-						val.bVal = !val.bVal;
-					}
-					break;
-
-				case '=':
-					// Before balancing, handle comparisons with null
-					if (val.GetType() == TypeCode::None)
-					{
-						val.bVal = (val2.GetType() == TypeCode::None);
-					}
-					else if (val2.GetType() == TypeCode::None)
-					{
-						val.bVal = false;
-					}
-					else
-					{
-						BalanceTypes(val, val2, evaluate);
+						bool bResult;
 						switch (val.GetType())
 						{
-						case TypeCode::ObjectModel_tc:
-							ThrowParseException("cannot compare objects");
-
 						case TypeCode::Int32:
-							val.bVal = (val.iVal == val2.iVal);
-							break;
-
-						case TypeCode::Uint32:
-							val.bVal = (val.uVal == val2.uVal);
+							bResult = (val.iVal < val2.iVal);
 							break;
 
 						case TypeCode::Float:
-							val.bVal = (val.fVal == val2.fVal);
+							bResult = (val.fVal < val2.fVal);
 							break;
 
 						case TypeCode::DateTime_tc:
-							val.bVal = val.Get56BitValue() == val2.Get56BitValue();
+							bResult = val.Get56BitValue() < val2.Get56BitValue();
 							break;
 
 						case TypeCode::Bool:
-							val.bVal = (val.bVal == val2.bVal);
-							break;
-
-						case TypeCode::CString:
-							val.bVal = (strcmp(val.sVal, (val2.GetType() == TypeCode::HeapString) ? val2.shVal.Get().Ptr() : val2.sVal) == 0);
-							break;
-
-						case TypeCode::HeapString:
-							val.bVal = (strcmp(val.shVal.Get().Ptr(), (val2.GetType() == TypeCode::HeapString) ? val2.shVal.Get().Ptr() : val2.sVal) == 0);
+							bResult = (!val.bVal && val2.bVal);
 							break;
 
 						default:
 							if (evaluate)
 							{
-								ThrowParseException("unexpected operand type to equality operator");
+								ThrowParseException("expected numeric or Boolean operands to comparison operator");
 							}
-							val.bVal = false;
+							bResult = false;
 							break;
 						}
+						val.SetBool((invert) ? !bResult : bResult);
 					}
-					val.SetType(TypeCode::Bool);
-					if (invert)
+					break;
+
+				case '=':
 					{
-						val.bVal = !val.bVal;
+						bool bResult;
+						// Before balancing, handle comparisons with null
+						if (val.GetType() == TypeCode::None)
+						{
+							bResult = (val2.GetType() == TypeCode::None);
+						}
+						else if (val2.GetType() == TypeCode::None)
+						{
+							bResult = false;
+						}
+						else
+						{
+							BalanceTypes(val, val2, evaluate);
+							switch (val.GetType())
+							{
+							case TypeCode::ObjectModel_tc:
+								ThrowParseException("cannot compare objects");
+
+							case TypeCode::Int32:
+								bResult = (val.iVal == val2.iVal);
+								break;
+
+							case TypeCode::Uint32:
+								bResult = (val.uVal == val2.uVal);
+								break;
+
+							case TypeCode::Float:
+								bResult = (val.fVal == val2.fVal);
+								break;
+
+							case TypeCode::DateTime_tc:
+								bResult = val.Get56BitValue() == val2.Get56BitValue();
+								break;
+
+							case TypeCode::Bool:
+								bResult = (val.bVal == val2.bVal);
+								break;
+
+							case TypeCode::CString:
+								bResult = (strcmp(val.sVal, (val2.GetType() == TypeCode::HeapString) ? val2.shVal.Get().Ptr() : val2.sVal) == 0);
+								break;
+
+							case TypeCode::HeapString:
+								bResult = (strcmp(val.shVal.Get().Ptr(), (val2.GetType() == TypeCode::HeapString) ? val2.shVal.Get().Ptr() : val2.sVal) == 0);
+								break;
+
+							default:
+								if (evaluate)
+								{
+									ThrowParseException("unexpected operand type to equality operator");
+								}
+								bResult = false;
+								break;
+							}
+						}
+						val.SetBool((invert) ? !bResult : bResult);
 					}
 					break;
 
@@ -536,7 +533,7 @@ void ExpressionParser::ParseInternal(ExpressionValue& val, bool evaluate, uint8_
     val.AppendAsString(str.GetRef());
     val2.AppendAsString(str.GetRef());
     StringHandle sh(str.c_str());
-    val.Set(sh);
+    val.SetStringHandle(sh);
 }
 
 bool ExpressionParser::ParseBoolean() THROWS(GCodeException)
@@ -673,8 +670,8 @@ void ExpressionParser::BalanceNumericTypes(ExpressionValue& val1, ExpressionValu
 		{
 			ThrowParseException("expected numeric operands");
 		}
-		val1.Set((int32_t)0);
-		val2.Set((int32_t)0);
+		val1.SetInt(0);
+		val2.SetInt(0);
 	}
 }
 
@@ -734,34 +731,29 @@ void ExpressionParser::BalanceTypes(ExpressionValue& val1, ExpressionValue& val2
 		{
 			ThrowParseException("cannot convert operands to same type");
 		}
-		val1.Set((int32_t)0);
-		val2.Set((int32_t)0);
+		val1.SetInt(0);
+		val2.SetInt(0);
 	}
 }
 
 void ExpressionParser::ConvertToFloat(ExpressionValue& val, bool evaluate) const THROWS(GCodeException)
 {
+	float fVal;
 	switch (val.GetType())
 	{
+	case TypeCode::Float:
+		return;							// no conversion needed, leave the precision alone
+
 	case TypeCode::Uint32:
-		val.SetType(TypeCode::Float);
-		val.fVal = (float)val.uVal;
-		val.param = 1;
+		fVal = (float)val.uVal;
 		break;
 
 	case TypeCode::Uint64:
-		val.SetType(TypeCode::Float);
-		val.fVal = (float)val.Get56BitValue();
-		val.param = 1;
+		fVal = (float)val.Get56BitValue();
 		break;
 
 	case TypeCode::Int32:
-		val.fVal = (float)val.iVal;
-		val.SetType(TypeCode::Float);
-		val.param = 1;
-		break;
-
-	case TypeCode::Float:
+		fVal = (float)val.iVal;
 		break;
 
 	default:
@@ -769,8 +761,10 @@ void ExpressionParser::ConvertToFloat(ExpressionValue& val, bool evaluate) const
 		{
 			ThrowParseException("expected numeric operand");
 		}
-		val.Set(0.0f, 1);
+		fVal = 0.0f;
+		break;
 	}
+	val.SetFloat(fVal, 1);
 }
 
 void ExpressionParser::ConvertToBool(ExpressionValue& val, bool evaluate) const THROWS(GCodeException)
@@ -781,7 +775,7 @@ void ExpressionParser::ConvertToBool(ExpressionValue& val, bool evaluate) const
 		{
 			ThrowParseException("expected Boolean operand");
 		}
-		val.Set(false);
+		val.SetBool(false);
 	}
 }
 
@@ -794,11 +788,11 @@ void ExpressionParser::ConvertToString(ExpressionValue& val, bool evaluate) noex
 			String<MaxStringExpressionLength> str;
 			val.AppendAsString(str.GetRef());
 			StringHandle sh(str.c_str());
-			val.Set(sh);
+			val.SetStringHandle(sh);
 		}
 		else
 		{
-			val.Set("");
+			val.SetCString("");
 		}
 	}
 }
@@ -812,9 +806,9 @@ void ExpressionParser::ConvertToDriverId(ExpressionValue& val, bool evaluate) co
 
 	case TypeCode::Int32:
 #if SUPPORT_CAN_EXPANSION
-		val.Set(DriverId(0, val.uVal));
+		val.SetDriverId(DriverId(0, val.uVal));
 #else
-		val.Set(DriverId(val.uVal));
+		val.SetDriverId(DriverId(val.uVal));
 #endif
 		break;
 
@@ -825,12 +819,12 @@ void ExpressionParser::ConvertToDriverId(ExpressionValue& val, bool evaluate) co
 #if SUPPORT_CAN_EXPANSION
 			if (ival >= 0 && fabsf(f10val - (float)ival) <= 0.002)
 			{
-				val.Set(DriverId(ival/10, ival % 10));
+				val.SetDriverId(DriverId(ival/10, ival % 10));
 			}
 #else
 			if (ival >= 0 && ival < 10 && fabsf(f10val - (float)ival) <= 0.002)
 			{
-				val.Set(DriverId(ival % 10));
+				val.SetDriverId(DriverId(ival % 10));
 			}
 #endif
 			else
@@ -874,11 +868,11 @@ void ExpressionParser::ParseNumber(ExpressionValue& rslt) noexcept
 
 	if (conv.FitsInInt32())
 	{
-		rslt.Set(conv.GetInt32());
+		rslt.SetInt(conv.GetInt32());
 	}
 	else
 	{
-		rslt.Set(conv.GetFloat(), constrain<unsigned int>(conv.GetDigitsAfterPoint(), 1, MaxFloatDigitsDisplayedAfterPoint));
+		rslt.SetFloat(conv.GetFloat(), constrain<unsigned int>(conv.GetDigitsAfterPoint(), 1, MaxFloatDigitsDisplayedAfterPoint));
 	}
 }
 
@@ -917,7 +911,7 @@ void ExpressionParser::ParseIdentifierExpression(ExpressionValue& rslt, bool eva
 				{
 					ThrowParseException("expected integer expression");
 				}
-				index.Set((int32_t)0);
+				index.SetInt(0);
 			}
 			AdvancePointer();										// skip the ']'
 			context.ProvideIndex(index.iVal);
@@ -941,19 +935,19 @@ void ExpressionParser::ParseIdentifierExpression(ExpressionValue& rslt, bool eva
 		switch (whichConstant.RawValue())
 		{
 		case NamedConstant::_true:
-			rslt.Set(true);
+			rslt.SetBool(true);
 			return;
 
 		case NamedConstant::_false:
-			rslt.Set(false);
+			rslt.SetBool(false);
 			return;
 
 		case NamedConstant::_null:
-			rslt.Set(nullptr);
+			rslt.SetNull(nullptr);
 			return;
 
 		case NamedConstant::pi:
-			rslt.Set(Pi);
+			rslt.SetFloat(Pi);
 			return;
 
 		case NamedConstant::iterations:
@@ -963,7 +957,7 @@ void ExpressionParser::ParseIdentifierExpression(ExpressionValue& rslt, bool eva
 				{
 					ThrowParseException("'iterations' used when not inside a loop");
 				}
-				rslt.Set(v);
+				rslt.SetInt(v);
 			}
 			return;
 
@@ -985,12 +979,12 @@ void ExpressionParser::ParseIdentifierExpression(ExpressionValue& rslt, bool eva
 					res = 2;
 					break;
 				}
-				rslt.Set(res);
+				rslt.SetInt(res);
 			}
 			return;
 
 		case NamedConstant::line:
-			rslt.Set((int32_t)gb.GetLineNumber());
+			rslt.SetInt((int32_t)gb.GetLineNumber());
 			return;
 
 		default:
@@ -1043,7 +1037,7 @@ void ExpressionParser::ParseIdentifierExpression(ExpressionValue& rslt, bool eva
 					{
 						ThrowParseException("expected numeric operand");
 					}
-					rslt.Set((int32_t)0);
+					rslt.SetInt(0);
 				}
 				break;
 
@@ -1122,8 +1116,7 @@ void ExpressionParser::ParseIdentifierExpression(ExpressionValue& rslt, bool eva
 
 			case Function::isnan:
 				ConvertToFloat(rslt, evaluate);
-				rslt.SetType(TypeCode::Bool);
-				rslt.bVal = (std::isnan(rslt.fVal) != 0);
+				rslt.SetBool(std::isnan(rslt.fVal) != 0);
 				break;
 
 			case Function::floor:
@@ -1132,8 +1125,7 @@ void ExpressionParser::ParseIdentifierExpression(ExpressionValue& rslt, bool eva
 					const float f = floorf(rslt.fVal);
 					if (f <= (float)std::numeric_limits<int32_t>::max() && f >= (float)std::numeric_limits<int32_t>::min())
 					{
-						rslt.SetType(TypeCode::Int32);
-						rslt.iVal = (int32_t)f;
+						rslt.SetInt((int32_t)f);
 					}
 					else
 					{
@@ -1237,7 +1229,7 @@ void ExpressionParser::ParseIdentifierExpression(ExpressionValue& rslt, bool eva
 					{
 						ThrowParseException("expected positive integer");
 					}
-					rslt.Set((int32_t)random(limit));
+					rslt.SetInt((int32_t)random(limit));
 				}
 				break;
 
@@ -1270,8 +1262,7 @@ void ExpressionParser::ParseIdentifierExpression(ExpressionValue& rslt, bool eva
 					default:
 						ThrowParseException("can't convert value to DateTime");
 					}
-					rslt.SetType(TypeCode::DateTime_tc);
-					rslt.Set56BitValue(val);
+					rslt.SetDateTime(val);
 				}
 				break;
 
@@ -1316,7 +1307,7 @@ void ExpressionParser::ParseIdentifierExpression(ExpressionValue& rslt, bool eva
 		// "exists(global)" will anyway because "global" is a root key in the object model. Handle the other two here.
 		if (applyExists && (strcmp(id.c_str(), "param") == 0 || strcmp(id.c_str(), "var") == 0))
 		{
-			rslt.Set(true);
+			rslt.SetBool(true);
 			return;
 		}
 
@@ -1329,7 +1320,7 @@ void ExpressionParser::ParseIdentifierExpression(ExpressionValue& rslt, bool eva
 		}
 		return;
 	}
-	rslt.Set(nullptr);
+	rslt.SetNull(nullptr);
 }
 
 // Parse a string to a DateTime
@@ -1349,7 +1340,7 @@ void ExpressionParser::GetVariableValue(ExpressionValue& rslt, const VariableSet
 	const Variable* var = vars->Lookup(name);
 	if (wantExists)
 	{
-		rslt.Set(var != nullptr);
+		rslt.SetBool(var != nullptr);
 		return;
 	}
 
@@ -1381,7 +1372,7 @@ void ExpressionParser::ParseQuotedString(ExpressionValue& rslt) THROWS(GCodeExce
 			if (CurrentCharacter() != c)
 			{
 				StringHandle sh(str.c_str());
-				rslt.Set(sh);
+				rslt.SetStringHandle(sh);
 				return;
 			}
 			AdvancePointer();
diff --git a/src/GCodes/GCodeBuffer/StringParser.cpp b/src/GCodes/GCodeBuffer/StringParser.cpp
index a4ea34a4..bd8b2c74 100644
--- a/src/GCodes/GCodeBuffer/StringParser.cpp
+++ b/src/GCodes/GCodeBuffer/StringParser.cpp
@@ -1929,7 +1929,7 @@ void StringParser::AddParameters(VariableSet& vs, int codeRunning) noexcept
 										catch (const GCodeException&)
 										{
 											//TODO can we report the error anywhere?
-											ev.Set(nullptr);
+											ev.SetNull(nullptr);
 										}
 										char paramName[2] = { letter, 0 };
 										vs.InsertNewParameter(paramName, ev);
diff --git a/src/ObjectModel/ObjectModel.h b/src/ObjectModel/ObjectModel.h
index a957aff6..146e8f49 100644
--- a/src/ObjectModel/ObjectModel.h
+++ b/src/ObjectModel/ObjectModel.h
@@ -156,13 +156,13 @@ struct ExpressionValue
 	void SetType(TypeCode t) noexcept { type = (uint32_t)t; }
 	bool IsStringType() const noexcept { return type == (uint32_t)TypeCode::CString || type == (uint32_t)TypeCode::HeapString; }
 
-	void Set(bool b) noexcept { Release(); type = (uint32_t)TypeCode::Bool; bVal = b; }
-	void Set(char c) noexcept { Release(); type = (uint32_t)TypeCode::Char; cVal = c; }
-	void Set(int32_t i) noexcept { Release(); type = (uint32_t)TypeCode::Int32; iVal = i; }
-	void Set(float f) noexcept { Release(); type = (uint32_t)TypeCode::Float; fVal = f; param = MaxFloatDigitsDisplayedAfterPoint; }
-	void Set(float f, uint32_t digits) noexcept { Release(); type = (uint32_t)TypeCode::Float; fVal = f; param = digits; }
-	void Set(const char *_ecv_array s) noexcept { Release(); type = (uint32_t)TypeCode::CString; sVal = s; }
-	void Set(DriverId did) noexcept
+	void SetBool(bool b) noexcept { Release(); type = (uint32_t)TypeCode::Bool; bVal = b; }
+	void SetChar(char c) noexcept { Release(); type = (uint32_t)TypeCode::Char; cVal = c; }
+	void SetInt(int32_t i) noexcept { Release(); type = (uint32_t)TypeCode::Int32; iVal = i; }
+	void SetFloat(float f) noexcept { Release(); type = (uint32_t)TypeCode::Float; fVal = f; param = MaxFloatDigitsDisplayedAfterPoint; }
+	void SetFloat(float f, uint32_t digits) noexcept { Release(); type = (uint32_t)TypeCode::Float; fVal = f; param = digits; }
+	void SetCString(const char *_ecv_array s) noexcept { Release(); type = (uint32_t)TypeCode::CString; sVal = s; }
+	void SetDriverId(DriverId did) noexcept
 	{
 		Release();
 		type = (uint32_t)TypeCode::DriverId_tc;
@@ -174,8 +174,9 @@ struct ExpressionValue
 		uVal = did.localDriver;
 	}
 
-	void Set(StringHandle sh) noexcept { Release(); type = (uint32_t)TypeCode::HeapString; shVal = sh; }
-	void Set(std::nullptr_t dummy) noexcept { Release();  type = (uint32_t)TypeCode::None; }
+	void SetStringHandle(StringHandle sh) noexcept { Release(); type = (uint32_t)TypeCode::HeapString; shVal = sh; }
+	void SetNull(std::nullptr_t dummy) noexcept { Release();  type = (uint32_t)TypeCode::None; }
+	void SetDateTime(time_t t) noexcept { Release(); type = (uint32_t)TypeCode::DateTime_tc; Set56BitValue(t); }
 
 	// Store a 56-bit value
 	void Set56BitValue(uint64_t v) { Release(); param = (uint32_t)(v >> 32) & 0x00FFFFFFu; uVal = (uint32_t)v; }