Move impure-related string allocation to helper methods (#8609)

* Move impure-related string allocation to helper methods

* Use better name for impure string make method

* Check dsc_length via type_lengths array

* Use different names for new impure helper methods

---------

Co-authored-by: Artyom Abakumov <[email protected]>

Author: Noremos

src/dsql/ExprNodes.cpp

@@ -3755,25 +3755,8 @@ dsc* CastNode::perform(thread_db* tdbb, impure_value* impure, dsc* value,
 			impure->vlu_desc.dsc_length = length;
 		}
 
-		length = impure->vlu_desc.dsc_length;
-
 		// Allocate a string block of sufficient size.
-
-		auto string = impure->vlu_string;
-
-		if (string && string->str_length < length)
-		{
-			delete string;
-			string = nullptr;
-		}
-
-		if (!string)
-		{
-			string = impure->vlu_string = FB_NEW_RPT(*tdbb->getDefaultPool(), length) VaryingString();
-			string->str_length = length;
-		}
-
-		impure->vlu_desc.dsc_address = string->str_data;
+		impure->makeTextValueAddress(*tdbb->getDefaultPool());
 	}
 
 	EVL_validate(tdbb, Item(Item::TYPE_CAST), itemInfo,
@@ -7143,29 +7126,7 @@ dsc* FieldNode::execute(thread_db* tdbb, Request* request) const
 		dsc desc = impure->vlu_desc;
 		impure->vlu_desc = format->fmt_desc[fieldId];
 
-		if (impure->vlu_desc.isText())
-		{
-			// Allocate a string block of sufficient size.
-			VaryingString* string = impure->vlu_string;
-
-			if (string && string->str_length < impure->vlu_desc.dsc_length)
-			{
-				delete string;
-				string = NULL;
-			}
-
-			if (!string)
-			{
-				string = impure->vlu_string = FB_NEW_RPT(*tdbb->getDefaultPool(),
-					impure->vlu_desc.dsc_length) VaryingString();
-				string->str_length = impure->vlu_desc.dsc_length;
-			}
-
-			impure->vlu_desc.dsc_address = string->str_data;
-		}
-		else
-			impure->vlu_desc.dsc_address = (UCHAR*) &impure->vlu_misc;
-
+		impure->makeValueAddress(*tdbb->getDefaultPool());
 		MOV_move(tdbb, &desc, &impure->vlu_desc);
 	}
 
@@ -13694,30 +13655,7 @@ dsc* UdfCallNode::execute(thread_db* tdbb, Request* request) const
 		const Parameter* const returnParam = function->getOutputFields()[0];
 		value->vlu_desc = returnParam->prm_desc;
 
-		// If the return data type is any of the string types, allocate space to hold value.
-
-		if (value->vlu_desc.dsc_dtype <= dtype_varying)
-		{
-			const USHORT retLength = value->vlu_desc.dsc_length;
-			VaryingString* string = value->vlu_string;
-
-			if (string && string->str_length < retLength)
-			{
-				delete string;
-				string = NULL;
-			}
-
-			if (!string)
-			{
-				string = FB_NEW_RPT(*tdbb->getDefaultPool(), retLength) VaryingString;
-				string->str_length = retLength;
-				value->vlu_string = string;
-			}
-
-			value->vlu_desc.dsc_address = string->str_data;
-		}
-		else
-			value->vlu_desc.dsc_address = (UCHAR*) &value->vlu_misc;
+		value->makeValueAddress(*tdbb->getDefaultPool());
 
 		if (!impureArea->temp)
 		{

src/dsql/StmtNodes.cpp

@@ -2248,19 +2248,7 @@ const StmtNode* DeclareVariableNode::execute(thread_db* tdbb, Request* request,
 		variable->vlu_desc = varDesc;
 		variable->vlu_desc.clearFlags();
 
-		if (variable->vlu_desc.dsc_dtype <= dtype_varying)
-		{
-			if (!variable->vlu_string)
-			{
-				const USHORT len = variable->vlu_desc.dsc_length;
-				variable->vlu_string = FB_NEW_RPT(*tdbb->getDefaultPool(), len) VaryingString();
-				variable->vlu_string->str_length = len;
-			}
-
-			variable->vlu_desc.dsc_address = variable->vlu_string->str_data;
-		}
-		else
-			variable->vlu_desc.dsc_address = (UCHAR*) &variable->vlu_misc;
+		variable->makeValueAddress(*tdbb->getDefaultPool());
 
 		request->req_operation = Request::req_return;
 	}

src/jrd/SysFunction.cpp

@@ -5545,31 +5545,7 @@ dsc* evlMaxMinValue(thread_db* tdbb, const SysFunction* function, const NestValu
 
 	DataTypeUtil(tdbb).makeFromList(&impure->vlu_desc, function->name, argTypes.getCount(), argTypes.begin());
 
-	if (impure->vlu_desc.isText())
-	{
-		const USHORT length = impure->vlu_desc.dsc_length;
-
-		// Allocate a string block of sufficient size
-
-		auto string = impure->vlu_string;
-
-		if (string && string->str_length < length)
-		{
-			delete string;
-			string = nullptr;
-		}
-
-		if (!string)
-		{
-			string = impure->vlu_string = FB_NEW_RPT(*tdbb->getDefaultPool(), length) VaryingString();
-			string->str_length = length;
-		}
-
-		impure->vlu_desc.dsc_address = string->str_data;
-	}
-	else
-		impure->vlu_desc.dsc_address = (UCHAR*) &impure->vlu_misc;
-
+	impure->makeValueAddress(*tdbb->getDefaultPool());
 	MOV_move(tdbb, result, &impure->vlu_desc);
 
 	if (impure->vlu_desc.dsc_dtype == dtype_text)

src/jrd/evl.cpp

@@ -573,22 +573,10 @@ void EVL_make_value(thread_db* tdbb, const dsc* desc, impure_value* value, Memor
 
 	// Allocate a string block of sufficient size.
 
-	VaryingString* string = value->vlu_string;
+	if (!pool)
+		pool = tdbb->getDefaultPool();
 
-	if (string && string->str_length < length)
-	{
-		delete string;
-		string = NULL;
-	}
-
-	if (!string)
-	{
-		if (!pool)
-			pool = tdbb->getDefaultPool();
-
-		string = value->vlu_string = FB_NEW_RPT(*pool, length) VaryingString();
-		string->str_length = length;
-	}
+	VaryingString* string = value->getString(*pool, length);
 
 	value->vlu_desc.dsc_length = length;
 	UCHAR* target = string->str_data;

src/jrd/val.h

@@ -35,6 +35,7 @@
 #include "../jrd/MetaName.h"
 #include "../jrd/RecordNumber.h"
 #include "../common/dsc.h"
+#include "../jrd/align.h"
 
 #define FLAG_BYTES(n)	(((n + BITS_PER_LONG) & ~((ULONG)BITS_PER_LONG - 1)) >> 3)
 
@@ -168,6 +169,13 @@ struct impure_value
 	void make_double(const double val);
 	void make_decimal128(const Firebird::Decimal128 val);
 	void make_decimal_fixed(const Firebird::Int128 val, const signed char scale);
+
+	template<class T>
+	VaryingString* getString(MemoryPool& pool, const T length) = delete; // Prevent dangerous length shrink
+	VaryingString* getString(MemoryPool& pool, const USHORT length);
+
+	void makeValueAddress(MemoryPool& pool);
+	void makeTextValueAddress(MemoryPool& pool);
 };
 
 // Do not use these methods where dsc_sub_type is not explicitly set to zero.
@@ -221,6 +229,40 @@ inline void impure_value::make_decimal_fixed(const Firebird::Int128 val, const s
 	this->vlu_desc.dsc_address = reinterpret_cast<UCHAR*>(&this->vlu_misc.vlu_int128);
 }
 
+inline VaryingString* impure_value::getString(MemoryPool& pool, const USHORT length)
+{
+	if (vlu_string && vlu_string->str_length < length)
+	{
+		delete vlu_string;
+		vlu_string = nullptr;
+	}
+
+	if (!vlu_string)
+	{
+		vlu_string = FB_NEW_RPT(pool, length) VaryingString();
+		vlu_string->str_length = length;
+	}
+
+	return vlu_string;
+}
+
+inline void impure_value::makeValueAddress(MemoryPool& pool)
+{
+	if (type_lengths[vlu_desc.dsc_dtype] == 0)
+	{
+		// If the data type is any of the string types, allocate space to hold value.
+		makeTextValueAddress(pool);
+	}
+	else
+		vlu_desc.dsc_address = (UCHAR*) &vlu_misc;
+}
+
+inline void impure_value::makeTextValueAddress(MemoryPool& pool)
+{
+	vlu_desc.dsc_address = getString(pool, vlu_desc.dsc_length)->str_data;
+}
+
+
 struct impure_value_ex : public impure_value
 {
 	SINT64 vlux_count;