# HG changeset patch
# User Patric Stout <truebrain@openttd.org>
# Date 2024-01-22 18:42:47
# Node ID f762d05f819e1e020a2f17fff6dcec390e8e7bba
# Parent  8618ddb90e723c6bd040fc0e9514d8c13a24c497

Codechange: refactor DecodeHexText to a generic purpose ConvertHexToBytes (#11866)

DecodeHexText() does more than just decoding hex. ConvertHexToBytes()
now only does pure hex decoding. This required a bit of refactoring
for the code using DecodeHexText().

diff --git a/src/settings.cpp b/src/settings.cpp
--- a/src/settings.cpp
+++ b/src/settings.cpp
@@ -949,40 +949,6 @@ static void GameLoadConfig(const IniFile
 }
 
 /**
- * Convert a character to a hex nibble value, or \c -1 otherwise.
- * @param c Character to convert.
- * @return Hex value of the character, or \c -1 if not a hex digit.
- */
-static int DecodeHexNibble(char c)
-{
-	if (c >= '0' && c <= '9') return c - '0';
-	if (c >= 'A' && c <= 'F') return c + 10 - 'A';
-	if (c >= 'a' && c <= 'f') return c + 10 - 'a';
-	return -1;
-}
-
-/**
- * Parse a sequence of characters (supposedly hex digits) into a sequence of bytes.
- * After the hex number should be a \c '|' character.
- * @param pos First character to convert.
- * @param[out] dest Output byte array to write the bytes.
- * @param dest_size Number of bytes in \a dest.
- * @return Whether reading was successful.
- */
-static bool DecodeHexText(const char *pos, uint8_t *dest, size_t dest_size)
-{
-	while (dest_size > 0) {
-		int hi = DecodeHexNibble(pos[0]);
-		int lo = (hi >= 0) ? DecodeHexNibble(pos[1]) : -1;
-		if (lo < 0) return false;
-		*dest++ = (hi << 4) | lo;
-		pos += 2;
-		dest_size--;
-	}
-	return *pos == '|';
-}
-
-/**
  * Load BaseGraphics set selection and configuration.
  */
 static void GraphicsSetLoadConfig(IniFile &ini)
@@ -1034,29 +1000,40 @@ static GRFConfig *GRFLoadConfig(const In
 	for (const IniItem &item : group->items) {
 		GRFConfig *c = nullptr;
 
-		uint8_t grfid_buf[4];
+		std::array<uint8_t, 4> grfid_buf;
 		MD5Hash md5sum;
-		const char *filename = item.name.c_str();
-		bool has_grfid = false;
+		std::string_view item_name = item.name;
 		bool has_md5sum = false;
 
 		/* Try reading "<grfid>|" and on success, "<md5sum>|". */
-		has_grfid = DecodeHexText(filename, grfid_buf, lengthof(grfid_buf));
-		if (has_grfid) {
-			filename += 1 + 2 * lengthof(grfid_buf);
-			has_md5sum = DecodeHexText(filename, md5sum.data(), md5sum.size());
-			if (has_md5sum) filename += 1 + 2 * md5sum.size();
+		auto grfid_pos = item_name.find("|");
+		if (grfid_pos != std::string_view::npos) {
+			std::string_view grfid_str = item_name.substr(0, grfid_pos);
+
+			if (ConvertHexToBytes(grfid_str, grfid_buf)) {
+				item_name = item_name.substr(grfid_pos + 1);
+
+				auto md5sum_pos = item_name.find("|");
+				if (md5sum_pos != std::string_view::npos) {
+					std::string_view md5sum_str = item_name.substr(0, md5sum_pos);
 
-			uint32_t grfid = grfid_buf[0] | (grfid_buf[1] << 8) | (grfid_buf[2] << 16) | (grfid_buf[3] << 24);
-			if (has_md5sum) {
-				const GRFConfig *s = FindGRFConfig(grfid, FGCM_EXACT, &md5sum);
-				if (s != nullptr) c = new GRFConfig(*s);
-			}
-			if (c == nullptr && !FioCheckFileExists(filename, NEWGRF_DIR)) {
-				const GRFConfig *s = FindGRFConfig(grfid, FGCM_NEWEST_VALID);
-				if (s != nullptr) c = new GRFConfig(*s);
+					has_md5sum = ConvertHexToBytes(md5sum_str, md5sum);
+					if (has_md5sum) item_name = item_name.substr(md5sum_pos + 1);
+				}
+
+				uint32_t grfid = grfid_buf[0] | (grfid_buf[1] << 8) | (grfid_buf[2] << 16) | (grfid_buf[3] << 24);
+				if (has_md5sum) {
+					const GRFConfig *s = FindGRFConfig(grfid, FGCM_EXACT, &md5sum);
+					if (s != nullptr) c = new GRFConfig(*s);
+				}
+				if (c == nullptr && !FioCheckFileExists(std::string(item_name), NEWGRF_DIR)) {
+					const GRFConfig *s = FindGRFConfig(grfid, FGCM_NEWEST_VALID);
+					if (s != nullptr) c = new GRFConfig(*s);
+				}
 			}
 		}
+		std::string filename = std::string(item_name);
+
 		if (c == nullptr) c = new GRFConfig(filename);
 
 		/* Parse parameters */
@@ -1084,7 +1061,7 @@ static GRFConfig *GRFLoadConfig(const In
 				SetDParam(1, STR_CONFIG_ERROR_INVALID_GRF_UNKNOWN);
 			}
 
-			SetDParamStr(0, StrEmpty(filename) ? item.name.c_str() : filename);
+			SetDParamStr(0, filename.empty() ? item.name.c_str() : filename);
 			ShowErrorMessage(STR_CONFIG_ERROR, STR_CONFIG_ERROR_INVALID_GRF, WL_CRITICAL);
 			delete c;
 			continue;
diff --git a/src/string.cpp b/src/string.cpp
--- a/src/string.cpp
+++ b/src/string.cpp
@@ -702,6 +702,55 @@ static int ICUStringContains(const std::
 	return ci_str.find(ci_value) != CaseInsensitiveStringView::npos;
 }
 
+/**
+ * Convert a single hex-nibble to a byte.
+ *
+ * @param c The hex-nibble to convert.
+ * @return The byte the hex-nibble represents, or -1 if it is not a valid hex-nibble.
+ */
+static int ConvertHexNibbleToByte(char c)
+{
+	if (c >= '0' && c <= '9') return c - '0';
+	if (c >= 'A' && c <= 'F') return c + 10 - 'A';
+	if (c >= 'a' && c <= 'f') return c + 10 - 'a';
+	return -1;
+}
+
+/**
+ * Convert a hex-string to a byte-array, while validating it was actually hex.
+ *
+ * @param hex The hex-string to convert.
+ * @param bytes The byte-array to write the result to.
+ *
+ * @note The length of the hex-string has to be exactly twice that of the length
+ * of the byte-array, otherwise conversion will fail.
+ *
+ * @return True iff the hex-string was valid and the conversion succeeded.
+ */
+bool ConvertHexToBytes(std::string_view hex, std::span<uint8_t> bytes)
+{
+	if (bytes.size() != hex.size() / 2) {
+		return false;
+	}
+
+	/* Hex-string lengths are always divisible by 2. */
+	if (hex.size() % 2 != 0) {
+		return false;
+	}
+
+	for (size_t i = 0; i < hex.size() / 2; i++) {
+		auto hi = ConvertHexNibbleToByte(hex[i * 2]);
+		auto lo = ConvertHexNibbleToByte(hex[i * 2 + 1]);
+
+		if (hi < 0 || lo < 0) {
+			return false;
+		}
+
+		bytes[i] = (hi << 4) | lo;
+	}
+
+	return true;
+}
 
 #ifdef WITH_UNISCRIBE
 
diff --git a/src/string_func.h b/src/string_func.h
--- a/src/string_func.h
+++ b/src/string_func.h
@@ -39,6 +39,8 @@ void StrTrimInPlace(std::string &str);
 [[nodiscard]] bool StrNaturalContains(const std::string_view str, const std::string_view value);
 [[nodiscard]] bool StrNaturalContainsIgnoreCase(const std::string_view str, const std::string_view value);
 
+bool ConvertHexToBytes(std::string_view hex, std::span<uint8_t> bytes);
+
 /** Case insensitive comparator for strings, for example for use in std::map. */
 struct CaseInsensitiveComparator {
 	bool operator()(const std::string_view s1, const std::string_view s2) const { return StrCompareIgnoreCase(s1, s2) < 0; }
diff --git a/src/tests/string_func.cpp b/src/tests/string_func.cpp
--- a/src/tests/string_func.cpp
+++ b/src/tests/string_func.cpp
@@ -342,3 +342,45 @@ TEST_CASE("FormatArrayAsHex")
 	CHECK(FormatArrayAsHex(std::array<byte, 1>{0x12}) == "12");
 	CHECK(FormatArrayAsHex(std::array<byte, 4>{0x13, 0x38, 0x42, 0xAF}) == "133842AF");
 }
+
+TEST_CASE("ConvertHexToBytes")
+{
+	CHECK(ConvertHexToBytes("", {}) == true);
+	CHECK(ConvertHexToBytes("1", {}) == false);
+	CHECK(ConvertHexToBytes("12", {}) == false);
+
+	std::array<uint8_t, 1> bytes1;
+	CHECK(ConvertHexToBytes("1", bytes1) == false);
+	CHECK(ConvertHexToBytes("12", bytes1) == true);
+	CHECK(bytes1[0] == 0x12);
+	CHECK(ConvertHexToBytes("123", bytes1) == false);
+	CHECK(ConvertHexToBytes("1g", bytes1) == false);
+	CHECK(ConvertHexToBytes("g1", bytes1) == false);
+
+	std::array<uint8_t, 2> bytes2;
+	CHECK(ConvertHexToBytes("12", bytes2) == false);
+	CHECK(ConvertHexToBytes("1234", bytes2) == true);
+	CHECK(bytes2[0] == 0x12);
+	CHECK(bytes2[1] == 0x34);
+
+	std::array<uint8_t, 8> bytes3;
+	CHECK(ConvertHexToBytes("123456789abcdef0", bytes3) == true);
+	CHECK(bytes3[0] == 0x12);
+	CHECK(bytes3[1] == 0x34);
+	CHECK(bytes3[2] == 0x56);
+	CHECK(bytes3[3] == 0x78);
+	CHECK(bytes3[4] == 0x9a);
+	CHECK(bytes3[5] == 0xbc);
+	CHECK(bytes3[6] == 0xde);
+	CHECK(bytes3[7] == 0xf0);
+
+	CHECK(ConvertHexToBytes("123456789ABCDEF0", bytes3) == true);
+	CHECK(bytes3[0] == 0x12);
+	CHECK(bytes3[1] == 0x34);
+	CHECK(bytes3[2] == 0x56);
+	CHECK(bytes3[3] == 0x78);
+	CHECK(bytes3[4] == 0x9a);
+	CHECK(bytes3[5] == 0xbc);
+	CHECK(bytes3[6] == 0xde);
+	CHECK(bytes3[7] == 0xf0);
+}