cpp/openttd-patchpack/source Changeset - r28551:f762d05f819e

Changeset - r28551:f762d05f819e

Parent rev.

Child rev.

[Not reviewed]

master

0 4 0

Patric Stout - 3 months ago 2024-01-22 18:42:47
truebrain@openttd.org

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().

4 files changed with 121 insertions and 51 deletions:

src/settings.cpp

src/string.cpp

src/string_func.h

src/tests/string_func.cpp

0 comments (0 inline, 0 general)

src/settings.cpp

➞

Show inline comments

@@ @@ -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;

src/string.cpp

➞

Show inline comments

@@ @@ -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

src/string_func.h

➞

Show inline comments

@@ @@ -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; }

src/tests/string_func.cpp

➞

Show inline comments

@@ @@ -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);
+}

0 comments (0 inline, 0 general)