Enhance UEFI test code

Changes:
  - variable_store_tests.cpp
  	- eliminate duplicated string size calculation
	- simplify compare_variable_name(). The function is changed to fail
	  if size of the two string is not matching.
	- There was a repeating pattern creating uint8_t msg_buffer on the
	  stack. This is test code, but a large enough name + data could
	  cause stack underrun. I added a vector and use it as a buffer.
	  This moves allocation to the HEAP and the compiler will take care
	  of cleaning up the memory when the variable goes out of scope.

  - variable_index_tests.cpp
    - eliminate duplicated string size calculation
	- convert null_name to a class member to remove code duplication

Signed-off-by: Gyorgy Szing <Gyorgy.Szing@arm.com>
Change-Id: Ieb108bf84dde21b7659c90452c42c8e7428909e0
diff --git a/components/service/uefi/smm_variable/backend/test/variable_index_tests.cpp b/components/service/uefi/smm_variable/backend/test/variable_index_tests.cpp
index 507ad8a..1b7a6b8 100644
--- a/components/service/uefi/smm_variable/backend/test/variable_index_tests.cpp
+++ b/components/service/uefi/smm_variable/backend/test/variable_index_tests.cpp
@@ -44,6 +44,7 @@
 		name_1 = to_variable_name(u"var1");
 		name_2 = to_variable_name(u"var2_nv");
 		name_3 = to_variable_name(u"var3_nv");
+		null_name = to_variable_name(u"");
 	}
 
 	void teardown()
@@ -59,23 +60,28 @@
 		return var_name;
 	}
 
+	size_t string_get_size_in_bytes(const std::u16string &string)
+	{
+		return string.size() * sizeof(typename std::u16string::value_type);
+	}
+
 	void create_variables()
 	{
 		struct variable_info *info = NULL;
 
 		info = variable_index_add_entry(&m_variable_index, &guid_1,
-						name_1.size() * sizeof(int16_t), (int16_t *) name_1.data());
+						string_get_size_in_bytes(name_1), (int16_t *) name_1.data());
 		CHECK_TRUE(info);
 		variable_index_set_variable(info, EFI_VARIABLE_BOOTSERVICE_ACCESS);
 
 		info = variable_index_add_entry(&m_variable_index, &guid_2,
-						name_2.size() * sizeof(int16_t), (int16_t *) name_2.data());
+						string_get_size_in_bytes(name_2), (int16_t *)  name_2.data());
 		CHECK_TRUE(info);
 		variable_index_set_variable(info, EFI_VARIABLE_NON_VOLATILE |
 							  EFI_VARIABLE_BOOTSERVICE_ACCESS);
 
 		info = variable_index_add_entry(&m_variable_index, &guid_1,
-						name_3.size() * sizeof(int16_t), (int16_t *) name_3.data());
+						string_get_size_in_bytes(name_3), (int16_t *)  name_3.data());
 		CHECK_TRUE(info);
 		variable_index_set_variable(info, EFI_VARIABLE_NON_VOLATILE |
 							  EFI_VARIABLE_RUNTIME_ACCESS |
@@ -90,6 +96,7 @@
 	std::u16string name_1;
 	std::u16string name_2;
 	std::u16string name_3;
+	std::u16string null_name;
 };
 
 TEST(UefiVariableIndexTests, emptyIndexOperations)
@@ -98,12 +105,12 @@
 	struct variable_info *info = NULL;
 
 	/* Expect not to find a variable */
-	info = variable_index_find(&m_variable_index, &guid_1, name_1.size() * sizeof(int16_t),
+	info = variable_index_find(&m_variable_index, &guid_1, string_get_size_in_bytes(name_1),
 				   (const int16_t *) name_1.data());
 	POINTERS_EQUAL(NULL, info);
 
 	/* Expect also find next to be rejected */
-	info = variable_index_find_next(&m_variable_index, &guid_1, name_1.size() * sizeof(int16_t),
+	info = variable_index_find_next(&m_variable_index, &guid_1, string_get_size_in_bytes(name_1),
 					(const int16_t *) name_1.data(), &status);
 	POINTERS_EQUAL(NULL, info);
 	UNSIGNED_LONGLONGS_EQUAL(EFI_INVALID_PARAMETER, status);
@@ -120,7 +127,7 @@
 	name = to_variable_name(
 		u"a long variable name that exceeds the length limit with a few chars");
 
-	info = variable_index_add_entry(&m_variable_index, &guid_1, name.size() * sizeof(int16_t),
+	info = variable_index_add_entry(&m_variable_index, &guid_1, string_get_size_in_bytes(name),
 					(int16_t *) name.data());
 
 	/* Expect the add to fail because of an oversized name */
@@ -128,7 +135,7 @@
 
 	name = to_variable_name(u"a long variable name that fits!");
 
-	info = variable_index_add_entry(&m_variable_index, &guid_1, name.size() * sizeof(int16_t),
+	info = variable_index_add_entry(&m_variable_index, &guid_1, string_get_size_in_bytes(name),
 					(int16_t *) name.data());
 
 	/* Expect the add succeed */
@@ -143,7 +150,7 @@
 	/* Expect to be able to fill the index */
 	for (size_t i = 0; i < MAX_VARIABLES; ++i) {
 		info = variable_index_add_entry(&m_variable_index, &guid,
-						name_1.size() * sizeof(int16_t),  (int16_t *) name_1.data());
+						string_get_size_in_bytes(name_1), (int16_t *) name_1.data());
 
 		CHECK_TRUE(info);
 
@@ -152,7 +159,7 @@
 	}
 
 	/* Variable index should now be full */
-	info = variable_index_add_entry(&m_variable_index, &guid, name_1.size() * sizeof(int16_t),
+	info = variable_index_add_entry(&m_variable_index, &guid, string_get_size_in_bytes(name_1),
 					(int16_t *) name_1.data());
 
 	POINTERS_EQUAL(NULL, info);
@@ -161,13 +168,12 @@
 TEST(UefiVariableIndexTests, enumerateStore)
 {
 	const struct variable_info *info = NULL;
-	const std::u16string null_name = to_variable_name(u"");
 	efi_status_t status = EFI_NOT_FOUND;
 
 	create_variables();
 
 	info = variable_index_find_next(&m_variable_index, &guid_1,
-					null_name.size() * sizeof(int16_t), (const int16_t *) null_name.data(),
+					string_get_size_in_bytes(null_name), (const int16_t *) null_name.data(),
 					&status);
 	CHECK_TRUE(info);
 	UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
@@ -232,10 +238,9 @@
 	/* Enumerate and now expect only NV variables to be present */
 	status = EFI_NOT_FOUND;
 	const struct variable_info *info = NULL;
-	std::u16string null_name = to_variable_name(u"");
 
 	info = variable_index_find_next(&m_variable_index, &guid_1,
-					null_name.size() * sizeof(int16_t), (const int16_t *) null_name.data(),
+					string_get_size_in_bytes(null_name),  (const int16_t *) null_name.data(),
 					&status);
 	CHECK_TRUE(info);
 	UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
@@ -281,7 +286,7 @@
 	create_variables();
 
 	/* Remove one of the NV variables */
-	info = variable_index_find(&m_variable_index, &guid_2, name_2.size() * sizeof(int16_t),
+	info = variable_index_find(&m_variable_index, &guid_2, string_get_size_in_bytes(name_2),
 				   (const int16_t *) name_2.data());
 
 	variable_index_clear_variable(&m_variable_index, info);
@@ -294,7 +299,7 @@
 	UNSIGNED_LONGS_EQUAL((sizeof(struct variable_metadata) * 1), dump_len);
 
 	/* Remove the volatile variable */
-	info = variable_index_find(&m_variable_index, &guid_1, name_1.size() * sizeof(int16_t),
+	info = variable_index_find(&m_variable_index, &guid_1, string_get_size_in_bytes(name_1),
 				   (const int16_t *) name_1.data());
 
 	variable_index_clear_variable(&m_variable_index, info);
@@ -307,7 +312,7 @@
 	UNSIGNED_LONGS_EQUAL((sizeof(struct variable_metadata) * 1), dump_len);
 
 	/* Remove the remaining NV variable */
-	info = variable_index_find(&m_variable_index, &guid_1, name_3.size() * sizeof(int16_t),
+	info = variable_index_find(&m_variable_index, &guid_1, string_get_size_in_bytes(name_3),
 				   (const int16_t *) name_3.data());
 
 	variable_index_clear_variable(&m_variable_index, info);
@@ -322,10 +327,9 @@
 	/* Enumerate and now expect an empty index */
 	info = NULL;
 	efi_status_t status = EFI_SUCCESS;
-	std::u16string null_name = to_variable_name(u"");
 
 	info = variable_index_find_next(&m_variable_index, &guid_1,
-					null_name.size() * sizeof(int16_t), (const int16_t *) null_name.data(),
+					string_get_size_in_bytes(null_name),  (const int16_t *) null_name.data(),
 					&status);
 	POINTERS_EQUAL(NULL, info);
 	UNSIGNED_LONGLONGS_EQUAL(EFI_NOT_FOUND, status);
@@ -345,7 +349,7 @@
 	/* Check first entry is as expected */
 	info = variable_index_iterator_current(&iter);
 	CHECK_TRUE(info);
-	UNSIGNED_LONGS_EQUAL(name_1.size() * sizeof(int16_t), info->metadata.name_size);
+	UNSIGNED_LONGS_EQUAL(string_get_size_in_bytes(name_1), info->metadata.name_size);
 	MEMCMP_EQUAL(name_1.data(), info->metadata.name, info->metadata.name_size);
 
 	variable_index_iterator_next(&iter);
@@ -354,7 +358,7 @@
 	/* Check next is as expected */
 	info = variable_index_iterator_current(&iter);
 	CHECK_TRUE(info);
-	UNSIGNED_LONGS_EQUAL(name_2.size() * sizeof(int16_t), info->metadata.name_size);
+	UNSIGNED_LONGS_EQUAL(string_get_size_in_bytes(name_2), info->metadata.name_size);
 	MEMCMP_EQUAL(name_2.data(), info->metadata.name, info->metadata.name_size);
 
 	struct variable_info *info_to_remove = info;
@@ -365,7 +369,7 @@
 	/* Check next is as expected */
 	info = variable_index_iterator_current(&iter);
 	CHECK_TRUE(info);
-	UNSIGNED_LONGS_EQUAL(name_3.size() * sizeof(int16_t), info->metadata.name_size);
+	UNSIGNED_LONGS_EQUAL(string_get_size_in_bytes(name_3), info->metadata.name_size);
 	MEMCMP_EQUAL(name_3.data(), info->metadata.name, info->metadata.name_size);
 
 	/* Expect iterating to be done */
@@ -383,7 +387,7 @@
 	/* Check first entry is as expected */
 	info = variable_index_iterator_current(&iter);
 	CHECK_TRUE(info);
-	UNSIGNED_LONGS_EQUAL(name_1.size() * sizeof(int16_t), info->metadata.name_size);
+	UNSIGNED_LONGS_EQUAL(string_get_size_in_bytes(name_1), info->metadata.name_size);
 	MEMCMP_EQUAL(name_1.data(), info->metadata.name, info->metadata.name_size);
 
 	variable_index_iterator_next(&iter);
@@ -392,7 +396,7 @@
 	/* Check next entry is as expected */
 	info = variable_index_iterator_current(&iter);
 	CHECK_TRUE(info);
-	UNSIGNED_LONGS_EQUAL(name_3.size() * sizeof(int16_t), info->metadata.name_size);
+	UNSIGNED_LONGS_EQUAL(string_get_size_in_bytes(name_3), info->metadata.name_size);
 	MEMCMP_EQUAL(name_3.data(), info->metadata.name, info->metadata.name_size);
 
 	/* Expect iterating to be done */
@@ -418,7 +422,7 @@
 
 	/* Set check constraints on one of the variables */
 	struct variable_info *info = variable_index_find(
-		&m_variable_index, &guid_2, name_2.size() * sizeof(int16_t), (const int16_t *) name_2.data());
+		&m_variable_index, &guid_2, string_get_size_in_bytes(name_2), (const int16_t *) name_2.data());
 
 	CHECK_TRUE(info);
 	CHECK_TRUE(info->is_variable_set);
@@ -434,7 +438,7 @@
 	 */
 	variable_index_clear_variable(&m_variable_index, info);
 
-	info = variable_index_find(&m_variable_index, &guid_2, name_2.size() * sizeof(int16_t),
+	info = variable_index_find(&m_variable_index, &guid_2, string_get_size_in_bytes(name_2),
 				   (const int16_t *) name_2.data());
 
 	CHECK_TRUE(info);
@@ -444,10 +448,9 @@
 	/* Enumerate over variables, only expecting to find the two remaining 'set' variables. */
 	info = NULL;
 	efi_status_t status = EFI_NOT_FOUND;
-	std::u16string null_name = to_variable_name(u"");
 
 	info = variable_index_find_next(&m_variable_index, &guid_1,
-					null_name.size() * sizeof(int16_t), (const int16_t *) null_name.data(),
+					string_get_size_in_bytes(null_name),  (const int16_t *) null_name.data(),
 					&status);
 	CHECK_TRUE(info);
 	UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
@@ -477,7 +480,7 @@
 	/* Check first entry is as expected */
 	info = variable_index_iterator_current(&iter);
 	CHECK_TRUE(info);
-	UNSIGNED_LONGS_EQUAL(name_1.size() * sizeof(int16_t), info->metadata.name_size);
+	UNSIGNED_LONGS_EQUAL(string_get_size_in_bytes(name_1), info->metadata.name_size);
 	MEMCMP_EQUAL(name_1.data(), info->metadata.name, info->metadata.name_size);
 
 	variable_index_iterator_next(&iter);
@@ -486,7 +489,7 @@
 	/* Check next is as expected */
 	info = variable_index_iterator_current(&iter);
 	CHECK_TRUE(info);
-	UNSIGNED_LONGS_EQUAL(name_2.size() * sizeof(int16_t), info->metadata.name_size);
+	UNSIGNED_LONGS_EQUAL(string_get_size_in_bytes(name_2), info->metadata.name_size);
 	MEMCMP_EQUAL(name_2.data(), info->metadata.name, info->metadata.name_size);
 
 	variable_index_iterator_next(&iter);
@@ -495,7 +498,7 @@
 	/* Check next is as expected */
 	info = variable_index_iterator_current(&iter);
 	CHECK_TRUE(info);
-	UNSIGNED_LONGS_EQUAL(name_3.size() * sizeof(int16_t), info->metadata.name_size);
+	UNSIGNED_LONGS_EQUAL(string_get_size_in_bytes(name_3), info->metadata.name_size);
 	MEMCMP_EQUAL(name_3.data(), info->metadata.name, info->metadata.name_size);
 
 	/* Expect iterating to be done */
@@ -517,14 +520,15 @@
 
 	/* Initially expect no variable_info */
 	struct variable_info *info = variable_index_find(
-		&m_variable_index, &guid_2, name_2.size() * sizeof(int16_t),
+		&m_variable_index, &guid_2, string_get_size_in_bytes(name_2),
 		(const int16_t *) name_2.data());
 
 	CHECK_FALSE(info);
 
 	/* Adding the check constraints should result in an entry being added */
-	info = variable_index_add_entry(&m_variable_index, &guid_2, name_2.size() * sizeof(int16_t),
-					(const int16_t *) name_2.data());
+	info = variable_index_add_entry(
+		&m_variable_index, &guid_2, string_get_size_in_bytes(name_2),
+		(const int16_t *) name_2.data());
 	CHECK_TRUE(info);
 
 	variable_index_set_constraints(info, &constraints);
diff --git a/components/service/uefi/smm_variable/backend/test/variable_store_tests.cpp b/components/service/uefi/smm_variable/backend/test/variable_store_tests.cpp
index 6235c00..0d3450e 100644
--- a/components/service/uefi/smm_variable/backend/test/variable_store_tests.cpp
+++ b/components/service/uefi/smm_variable/backend/test/variable_store_tests.cpp
@@ -56,32 +56,34 @@
 		return var_name;
 	}
 
+	size_t string_get_size_in_bytes(const std::u16string &string)
+	{
+		return string.size() * sizeof(uint16_t);
+	}
+
 	bool compare_variable_name(const std::u16string &expected, const int16_t *name,
 				   size_t name_size)
 	{
-		bool is_equal = (expected.size() + 1 <= name_size / sizeof(int16_t));
+		std::u16string var_name = to_variable_name(expected);
 
-		for (size_t i = 0; is_equal && i < expected.size(); i++) {
-			if (name[i] != (int16_t)expected[i]) {
-				is_equal = false;
-				break;
-			}
-		}
+		if (name_size != string_get_size_in_bytes(var_name))
+			return false;
 
-		return is_equal;
+		return 0==memcmp(name, var_name.data(), name_size);
 	}
 
 	efi_status_t set_variable(const std::u16string &name, const uint8_t *data, size_t data_size,
 				  uint32_t attributes, EFI_GUID *guid = NULL)
 	{
 		std::u16string var_name = to_variable_name(name);
-		size_t name_size = var_name.size() * sizeof(int16_t);
+		size_t name_size = string_get_size_in_bytes(var_name);
 
-		uint8_t msg_buffer[SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_SIZE(name_size,
-										 data_size)];
+		/* Use a vector as a temporary buffer to move allocation to the HEAP and for RAII benefits. */
+		std::vector<uint8_t> msg_buffer(
+			(std::size_t)SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_SIZE(name_size, data_size));
 
 		SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *access_variable =
-			(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *)msg_buffer;
+			(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE  *) msg_buffer.data();
 
 		if (!guid)
 			guid = &m_common_guid;
@@ -92,9 +94,9 @@
 		memcpy(access_variable->Name, var_name.data(), name_size);
 
 		access_variable->DataSize = data_size;
-		memcpy(&msg_buffer[SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_DATA_OFFSET(
-			       access_variable)],
-		       data, data_size);
+		memcpy(msg_buffer.data() +
+			SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_DATA_OFFSET(access_variable),
+			data, data_size);
 
 		efi_status_t status =
 			uefi_variable_store_set_variable(&m_uefi_variable_store, access_variable);
@@ -105,7 +107,7 @@
 	efi_status_t set_variable(const std::u16string &name, const std::string &data,
 				  uint32_t attributes, EFI_GUID *guid = NULL)
 	{
-		return set_variable(name, static_cast<uint8_t *>(data.data()), data.size(),
+		return set_variable(name, (uint8_t *) data.data(), data.size(),
 					attributes, guid);
 	}
 
@@ -113,12 +115,13 @@
 				  size_t data_len_clamp = VARIABLE_BUFFER_SIZE)
 	{
 		std::u16string var_name = to_variable_name(name);
-		size_t name_size = var_name.size() * sizeof(uint16_t);
+		size_t name_size = string_get_size_in_bytes(var_name);
+
 		size_t total_size = 0;
-		uint8_t msg_buffer[VARIABLE_BUFFER_SIZE];
+		std::vector<uint8_t> msg_buffer(VARIABLE_BUFFER_SIZE);
 
 		SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *access_variable =
-			(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *)msg_buffer;
+			(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE  *) msg_buffer.data();
 
 		access_variable->Guid = m_common_guid;
 		access_variable->Attributes = 0;
@@ -142,7 +145,7 @@
 
 		if (status == EFI_SUCCESS) {
 			const char *data_start =
-				(const char *)(msg_buffer +
+				(const char *) (msg_buffer.data() +
 					       SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_DATA_OFFSET(
 						       access_variable));
 
@@ -192,11 +195,13 @@
 					    const VAR_CHECK_VARIABLE_PROPERTY &check_property)
 	{
 		std::u16string var_name = to_variable_name(name);
-		size_t name_size = var_name.size() * sizeof(uint16_t);
+		size_t name_size = string_get_size_in_bytes(var_name);
 
+		std::vector<uint8_t> msg_buffer(
+			SMM_VARIABLE_COMMUNICATE_VAR_CHECK_VARIABLE_PROPERTY_SIZE(name_size));
 
 		SMM_VARIABLE_COMMUNICATE_VAR_CHECK_VARIABLE_PROPERTY *check_var =
-			(SMM_VARIABLE_COMMUNICATE_VAR_CHECK_VARIABLE_PROPERTY *)msg_buffer;
+			(SMM_VARIABLE_COMMUNICATE_VAR_CHECK_VARIABLE_PROPERTY *) msg_buffer.data();
 
 		check_var->Guid = m_common_guid;
 		check_var->NameSize = name_size;
@@ -213,7 +218,7 @@
 	void zap_stored_variable(const std::u16string &name)
 	{
 		std::u16string var_name = to_variable_name(name);
-		size_t name_size = var_name.size() * sizeof(int16_t);
+		size_t name_size = string_get_size_in_bytes(var_name);
 
 		/* Create the condition where a variable is indexed but
 		 * there is no corresponding stored object.
@@ -493,9 +498,9 @@
 	UNSIGNED_LONGLONGS_EQUAL(EFI_NOT_FOUND, status);
 
 	/* Expect access to be blocked for get_next_variable_name */
-	uint8_t msg_buffer[VARIABLE_BUFFER_SIZE];
+	std::vector<uint8_t> msg_buffer(VARIABLE_BUFFER_SIZE);
 	SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *next_name =
-		(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *)msg_buffer;
+		(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *) msg_buffer.data();
 	size_t max_name_len =
 		VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET;
 
@@ -566,15 +571,15 @@
 
 	/* Prepare to enumerate */
 	size_t total_len = 0;
-	uint8_t msg_buffer[VARIABLE_BUFFER_SIZE];
+	std::vector<uint8_t> msg_buffer(VARIABLE_BUFFER_SIZE);
 	SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *next_name =
-		(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *)msg_buffer;
+		(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *) msg_buffer.data();
 	size_t max_name_len =
 		VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET;
 
 	/* First check handling of invalid variable name */
 	std::u16string bogus_name = to_variable_name(u"bogus_variable");
-	size_t bogus_name_size = bogus_name.size() * sizeof(uint16_t);
+	size_t bogus_name_size = string_get_size_in_bytes(bogus_name);
 	next_name->Guid = m_common_guid;
 	next_name->NameSize = bogus_name_size;
 	memcpy(next_name->Name, bogus_name.data(), bogus_name_size);
@@ -658,9 +663,9 @@
 	 * to have gone and for the index to have been cleaned up
 	 * for the failed variable 3.
 	 */
-	uint8_t msg_buffer[VARIABLE_BUFFER_SIZE];
+	std::vector<uint8_t> msg_buffer(VARIABLE_BUFFER_SIZE);
 	SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *next_name =
-		(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *)msg_buffer;
+		(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *) msg_buffer.data();
 	size_t max_name_len =
 		VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET;