Codechange: Use std::string_view in FormatString, and validate string bounds while parsing.

src/newgrf_text.cpp

@@ -23,6 +23,7 @@
 #include "newgrf_storage.h"
 #include "newgrf_text.h"
 #include "newgrf_cargo.h"
+#include "string_base.h"
 #include "string_func.h"
 #include "timer/timer_game_calendar.h"
 #include "debug.h"
@@ -767,7 +768,7 @@ static void HandleNewGRFStringControlCodes(std::string_view str, TextRefStack &s
  * @param stack The TextRefStack.
  * @param[out] params Output parameters
  */
-static void ProcessNewGRFStringControlCode(char32_t scc, const char *&str, TextRefStack &stack, std::vector<StringParameter> &params)
+static void ProcessNewGRFStringControlCode(char32_t scc, StringConsumer &str, TextRefStack &stack, std::vector<StringParameter> &params)
 {
 	/* There is data on the NewGRF text stack, and we want to move them to OpenTTD's string stack.
 	 * After this call, a new call is made with `modify_parameters` set to false when the string is finally formatted. */
@@ -775,15 +776,15 @@ static void ProcessNewGRFStringControlCode(char32_t scc, const char *&str, TextR
 		default: return;
 
 		case SCC_PLURAL_LIST:
-			++str; // plural form
+			(void)str.Uint8Consume(); // plural form
 			[[fallthrough]];
 		case SCC_GENDER_LIST: {
-			++str; // offset
+			(void)str.Uint8Consume(); // offset
 			/* plural and gender choices cannot contain any string commands, so just skip the whole thing */
-			uint num = static_cast<uint8_t>(*str++);
+			uint num = str.Uint8Consume();
 			uint total_len = 0;
 			for (uint i = 0; i != num; i++) {
-				total_len += static_cast<uint8_t>(*str++);
+				total_len += str.Uint8Consume();
 			}
 			str += total_len;
 			break;
@@ -791,17 +792,18 @@ static void ProcessNewGRFStringControlCode(char32_t scc, const char *&str, TextR
 
 		case SCC_SWITCH_CASE: {
 			/* skip all cases and continue with default case */
-			uint num = static_cast<uint8_t>(*str++);
+			uint num = str.Uint8Consume();
 			for (uint i = 0; i != num; i++) {
-				str += 3 + static_cast<uint8_t>(str[1]) + (static_cast<uint8_t>(str[2]) << 8);
+				(void)str.Uint8Consume(); // case index
+				str += str.Uint16LEConsume();
 			}
-			str += 2; // length of default
+			(void)str.Uint16LEConsume(); // default case length
 			break;
 		}
 
 		case SCC_GENDER_INDEX:
 		case SCC_SET_CASE:
-			++str;
+			(void)str.Uint8Consume();
 			break;
 
 		case SCC_ARG_INDEX:
@@ -841,7 +843,7 @@ static void ProcessNewGRFStringControlCode(char32_t scc, const char *&str, TextR
 		case SCC_NEWGRF_DISCARD_WORD:           stack.PopUnsignedWord(); break;
 
 		case SCC_NEWGRF_ROTATE_TOP_4_WORDS:     stack.RotateTop4Words(); break;
-		case SCC_NEWGRF_PUSH_WORD:              stack.PushWord(Utf8Consume(&str)); break;
+		case SCC_NEWGRF_PUSH_WORD:              stack.PushWord(str.Utf8Consume()); break;
 
 		case SCC_NEWGRF_PRINT_WORD_CARGO_LONG:
 		case SCC_NEWGRF_PRINT_WORD_CARGO_SHORT:
@@ -851,7 +853,7 @@ static void ProcessNewGRFStringControlCode(char32_t scc, const char *&str, TextR
 			break;
 
 		case SCC_NEWGRF_STRINL: {
-			StringID stringid = Utf8Consume(str);
+			StringID stringid = str.Utf8Consume();
 			/* We also need to handle the substring's stack usage. */
 			HandleNewGRFStringControlCodes(GetStringPtr(stringid), stack, params);
 			break;
@@ -879,7 +881,7 @@ static void ProcessNewGRFStringControlCode(char32_t scc, const char *&str, TextR
  * @param[in,out] str String iterator, moved forward if SCC_NEWGRF_PUSH_WORD is found.
  * @returns String code to use.
  */
-char32_t RemapNewGRFStringControlCode(char32_t scc, const char **str)
+char32_t RemapNewGRFStringControlCode(char32_t scc, StringConsumer &str)
 {
 	switch (scc) {
 		default:
@@ -947,7 +949,7 @@ char32_t RemapNewGRFStringControlCode(char32_t scc, const char **str)
 
 		/* These NewGRF string codes modify the NewGRF stack or otherwise do not map to OpenTTD string codes. */
 		case SCC_NEWGRF_PUSH_WORD:
-			Utf8Consume(str);
+			(void)str.Utf8Consume();
 			return 0;
 
 		case SCC_NEWGRF_DISCARD_WORD:
@@ -964,9 +966,9 @@ char32_t RemapNewGRFStringControlCode(char32_t scc, const char **str)
  */
 static void HandleNewGRFStringControlCodes(std::string_view str, TextRefStack &stack, std::vector<StringParameter> &params)
 {
-	for (const char *p = str.data(), *end = str.data() + str.size(); p < end; /* nothing */) {
+	StringConsumer p(str);
-		char32_t scc;
+	while (!p.empty()) {
-		p += Utf8Decode(&scc, p);
+		char32_t scc = p.Utf8Consume();
 		ProcessNewGRFStringControlCode(scc, p, stack, params);
 	}
 }

src/string.cpp

@@ -18,6 +18,7 @@
 
 #include <sstream>
 #include <iomanip>
+#include <charconv>
 
 #ifdef _MSC_VER
 #	define strncasecmp strnicmp
@@ -448,28 +449,29 @@ bool IsValidChar(char32_t key, CharSetFilter afilter)
  * @param s Character stream to retrieve character from.
  * @return Number of characters in the sequence.
  */
-size_t Utf8Decode(char32_t *c, const char *s)
+size_t Utf8Decode(char32_t *c, const char *s, size_t maxlen)
 {
 	assert(c != nullptr);
+	assert(maxlen > 0);
 
 	if (!HasBit(s[0], 7)) {
 		/* Single byte character: 0xxxxxxx */
 		*c = s[0];
 		return 1;
 	} else if (GB(s[0], 5, 3) == 6) {
-		if (IsUtf8Part(s[1])) {
+		if (maxlen >= 2 && IsUtf8Part(s[1])) {
 			/* Double byte character: 110xxxxx 10xxxxxx */
 			*c = GB(s[0], 0, 5) << 6 | GB(s[1], 0, 6);
 			if (*c >= 0x80) return 2;
 		}
 	} else if (GB(s[0], 4, 4) == 14) {
-		if (IsUtf8Part(s[1]) && IsUtf8Part(s[2])) {
+		if (maxlen >= 3 && IsUtf8Part(s[1]) && IsUtf8Part(s[2])) {
 			/* Triple byte character: 1110xxxx 10xxxxxx 10xxxxxx */
 			*c = GB(s[0], 0, 4) << 12 | GB(s[1], 0, 6) << 6 | GB(s[2], 0, 6);
 			if (*c >= 0x800) return 3;
 		}
 	} else if (GB(s[0], 3, 5) == 30) {
-		if (IsUtf8Part(s[1]) && IsUtf8Part(s[2]) && IsUtf8Part(s[3])) {
+		if (maxlen >= 4 && IsUtf8Part(s[1]) && IsUtf8Part(s[2]) && IsUtf8Part(s[3])) {
 			/* 4 byte character: 11110xxx 10xxxxxx 10xxxxxx 10xxxxxx */
 			*c = GB(s[0], 0, 3) << 18 | GB(s[1], 0, 6) << 12 | GB(s[2], 0, 6) << 6 | GB(s[3], 0, 6);
 			if (*c >= 0x10000 && *c <= 0x10FFFF) return 4;
@@ -1072,3 +1074,45 @@ public:
 #endif /* defined(WITH_COCOA) && !defined(STRGEN) && !defined(SETTINGSGEN) */
 
 #endif
+
+/**
+ * Consume and return a UTF-8 character.
+ */
+char32_t StringConsumer::Utf8Consume()
+{
+	if (this->empty()) {
+		assert(false);
+		return '?';
+	}
+	char32_t res;
+	size_t len = Utf8Decode(&res, this->data(), this->size());
+	*this += len;
+	return res;
+}
+
+template<class T>
+static T ParseInt(std::string_view& str, int base)
+{
+	T res;
+	const char *begin = str.data();
+	const char *end = begin + str.size();
+	const char *next = std::from_chars(begin, end, res, base).ptr;
+	assert(begin <= next && next <= end);
+	str.remove_prefix(next - begin);
+	return next == begin ? 0 : res;
+}
+
+/**
+ * Parse string to integer, and consume the involved characters.
+ * @param base Number base.
+ * @return parsed value
+ */
+uint32_t StringConsumer::Uint32Parse(int base)
+{
+	return ParseInt<uint32_t>(this->string, base);
+}
+
+uint64_t StringConsumer::Uint64Parse(int base)
+{
+	return ParseInt<uint64_t>(this->string, base);
+}

src/string_base.h

@@ -61,4 +61,180 @@ protected:
 	StringIterator() {}
 };
 
+/**
+ * Input iterator from std::string_view.
+ */
+class StringConsumer {
+	std::string_view string;
+
+public:
+	using size_type = std::string_view::size_type;
+
+	/**
+	 * Create a consumer for an external string.
+	 */
+	explicit StringConsumer(std::string_view string) : string(string) {}
+
+	/**
+	 * Check whether no bytes are left.
+	 * @return true if empty.
+	 */
+	[[nodiscard]] bool empty() const noexcept { return this->string.empty(); }
+
+	/**
+	 * Get number of bytes left.
+	 * @return Number of bytes left.
+	 */
+	[[nodiscard]] size_type size() const noexcept { return this->string.size(); }
+
+	/**
+	 * Advance by one byte.
+	 * @return this
+	 */
+	StringConsumer &operator++()
+	{
+		*this += 1;
+		return *this;
+	}
+
+	/**
+	 * Advance by one byte.
+	 * @return Copy of this before advancing.
+	 */
+	StringConsumer operator++(int)
+	{
+		auto res = *this;
+		*this += 1;
+		return res;
+	}
+
+	/**
+	 * Advance by "count" byte.
+	 * @param count Number of bytes to skip.
+	 * @return this
+	 */
+	StringConsumer &operator+=(size_type count)
+	{
+		if (count <= this->string.size()) {
+			this->string.remove_prefix(count);
+		} else {
+			assert(false);
+			this->string.remove_prefix(this->string.size());
+		}
+		return *this;
+	}
+
+	/**
+	 * Peek first byte.
+	 * @return First byte.
+	 */
+	[[nodiscard]] char operator*() const
+	{
+		if (this->string.empty()) {
+			assert(false);
+			return '?';
+		} else {
+			return this->string.front();
+		}
+	}
+
+	/**
+	 * Get buffer of all remaining bytes.
+	 * @return Buffer start.
+	 */
+	[[nodiscard]] const char *data() const { return this->string.data(); }
+
+	/**
+	 * Get view of all remaining bytes.
+	 * @return Remaining bytes.
+	 */
+	[[nodiscard]] std::string_view str() const noexcept { return this->string; }
+
+	/**
+	 * Find position of first occurrence of some byte.
+	 * @param c Byte to search for.
+	 * @return Offset to first occurrence, or "npos" if no occurrence.
+	 */
+	[[nodiscard]] size_type find(char c) const { return this->string.find(c); }
+
+	/**
+	 * Special value for "find" and "StrConsume".
+	 */
+	static constexpr size_type npos = std::string_view::npos;
+
+	/**
+	 * Read a UTF-8 character and advance the consumer.
+	 * @return Read character.
+	 */
+	[[nodiscard]] char32_t Utf8Consume();
+
+	/**
+	 * Read a uint8_t and advance the consumer.
+	 * @return Read integer.
+	 */
+	[[nodiscard]] uint8_t Uint8Consume()
+	{
+		if (this->string.empty()) {
+			assert(false);
+			return 0;
+		} else {
+			uint8_t res = **this;
+			*this += 1;
+			return res;
+		}
+	}
+
+	/**
+	 * Read a uint16_t in little endian and advance the consumer.
+	 * @return Read integer.
+	 */
+	[[nodiscard]] uint16_t Uint16LEConsume()
+	{
+		uint16_t res = this->Uint8Consume();
+		res |= this->Uint8Consume() << 8;
+		return res;
+	}
+
+	/**
+	 * Read a sequence of bytes and advance the consumer.
+	 * @param len Number of bytes to read.
+	 * @return Read string.
+	 */
+	[[nodiscard]] std::string_view StrConsume(size_type len)
+	{
+		if (len != npos && len > this->string.size()) {
+			assert(false);
+			len = npos;
+		}
+		if (len == npos) {
+			std::string_view res = this->string;
+			this->string.remove_prefix(this->string.size());
+			return res;
+		} else {
+			std::string_view res = this->string.substr(0, len);
+			this->string.remove_prefix(len);
+			return res;
+		}
+	}
+
+	/**
+	 * Read and parse a uint32_t and advance the consumer.
+	 * @param base Number base.
+	 * @return Parsed integer.
+	 */
+	[[nodiscard]] uint32_t Uint32Parse(int base);
+
+	/**
+	 * Read and parse a uint64_t and advance the consumer.
+	 * @param base Number base.
+	 * @return Parsed integer.
+	 */
+	[[nodiscard]] uint64_t Uint64Parse(int base);
+
+	/**
+	 * Discard all remaining bytes.
+	 */
+	void clear() { this->string.remove_prefix(this->string.size()); }
+};
+
 #endif /* STRING_BASE_H */

src/string_func.h

@@ -76,7 +76,7 @@ inline size_t ttd_strnlen(const char *str, size_t maxlen)
 
 bool IsValidChar(char32_t key, CharSetFilter afilter);
 
-size_t Utf8Decode(char32_t *c, const char *s);
+size_t Utf8Decode(char32_t *c, const char *s, size_t maxlen = 4);
 /* std::string_view::iterator might be char *, in which case we do not want this templated variant to be taken. */
 template <typename T> requires (!std::is_same_v<T, char *> && (std::is_same_v<std::string_view::iterator, T> || std::is_same_v<std::string::iterator, T>))
 inline size_t Utf8Decode(char32_t *c, T &s) { return Utf8Decode(c, &*s); }

src/strings.cpp

@@ -28,6 +28,7 @@
 #include "engine_base.h"
 #include "language.h"
 #include "townname_func.h"
+#include "string_base.h"
 #include "string_func.h"
 #include "company_base.h"
 #include "smallmap_gui.h"
@@ -776,22 +777,25 @@ static int DeterminePluralForm(int64_t count, int plural_form)
 	}
 }
 
-static const char *ParseStringChoice(const char *b, uint form, StringBuilder &builder)
+static void ParseStringChoice(StringConsumer& str, uint form, StringBuilder &builder)
 {
 	/* <NUM> {Length of each string} {each string} */
-	uint n = (uint8_t)*b++;
+	uint n = str.Uint8Consume();
-	size_t form_offset = 0, form_len = 0, total_len = 0;
+	size_t form_pre = 0, form_len = 0, form_post = 0;
 	for (uint i = 0; i != n; i++) {
-		uint len = (uint8_t)*b++;
+		uint len = str.Uint8Consume();
-		if (i == form) {
+		if (i < form) {
-			form_offset = total_len;
+			form_pre += len;
+		} else if (i > form) {
+			form_post += len;
+		} else {
 			form_len = len;
 		}
-		total_len += len;
 	}
 
-	builder += std::string_view(b + form_offset, form_len);
+	str += form_pre;
-	return b + total_len;
+	builder += str.StrConsume(form_len);
+	str += form_post;
 }
 
 /** Helper for unit conversion. */
@@ -998,62 +1002,59 @@ uint ConvertDisplaySpeedToKmhishSpeed(uint speed, VehicleType type)
  * @param builder The string builder to write the string to.
  * @returns Updated position position in input buffer.
  */
-static const char *DecodeEncodedString(const char *str, bool game_script, StringBuilder &builder)
+static void DecodeEncodedString(StringConsumer &str, bool game_script, StringBuilder &builder)
 {
 	std::vector<StringParameter> sub_args;
 
-	char *p;
+	StringIndexInTab id(str.Uint32Parse(16));
-	StringIndexInTab id(std::strtoul(str, &p, 16));
+	if (!str.empty() && *str != SCC_RECORD_SEPARATOR) {
-	if (*p != SCC_RECORD_SEPARATOR && *p != '\0') {
+		str.clear();
-		while (*p != '\0') p++;
 		builder += "(invalid SCC_ENCODED)";
-		return p;
+		return;
 	}
 	if (game_script && id >= TAB_SIZE_GAMESCRIPT) {
-		while (*p != '\0') p++;
+		str.clear();
 		builder += "(invalid StringID)";
-		return p;
+		return;
 	}
 
-	while (*p != '\0') {
+	while (!str.empty()) {
 		/* The start of parameter. */
-		const char *s = ++p;
+		++str;
 
 		/* Find end of the parameter. */
-		for (; *p != '\0' && *p != SCC_RECORD_SEPARATOR; ++p) {}
+		StringConsumer record{str.StrConsume(str.find(SCC_RECORD_SEPARATOR))};
-
+		if (record.empty()) {
-		if (s == p) {
 			/* This is an empty parameter. */
 			sub_args.emplace_back(std::monostate{});
 			continue;
 		}
 
 		/* Get the parameter type. */
-		char32_t parameter_type;
+		char32_t parameter_type = record.Utf8Consume();
-		size_t len = Utf8Decode(&parameter_type, s);
-		s += len;
-
 		switch (parameter_type) {
 			case SCC_ENCODED: {
-				uint64_t param = std::strtoull(s, &p, 16);
+				uint64_t param = record.Uint64Parse(16);
 				if (param >= TAB_SIZE_GAMESCRIPT) {
-					while (*p != '\0') p++;
+					str.clear();
 					builder += "(invalid sub-StringID)";
-					return p;
+					return;
 				}
+				assert(record.empty());
 				param = MakeStringID(TEXT_TAB_GAMESCRIPT_START, StringIndexInTab(param));
 				sub_args.emplace_back(param);
 				break;
 			}
 
 			case SCC_ENCODED_NUMERIC: {
-				uint64_t param = std::strtoull(s, &p, 16);
+				uint64_t param = record.Uint64Parse(16);
+				assert(record.empty());
 				sub_args.emplace_back(param);
 				break;
 			}
 
 			case SCC_ENCODED_STRING: {
-				sub_args.emplace_back(std::string(s, p - s));
+				sub_args.push_back(std::string(record.str()));
 				break;
 			}
 
@@ -1066,8 +1067,6 @@ static const char *DecodeEncodedString(const char *str, bool game_script, String
 
 	StringID stringid = game_script ? MakeStringID(TEXT_TAB_GAMESCRIPT_START, id) : StringID{id.base()};
 	GetStringWithArgs(builder, stringid, sub_args, true);
-
-	return p;
 }
 
 /**
@@ -1098,33 +1097,28 @@ static void FormatString(StringBuilder &builder, std::string_view str_arg, Strin
 	}
 	uint next_substr_case_index = 0;
 	struct StrStackItem {
-		const char *str;
+		StringConsumer str;
-		const char *end;
 		size_t first_param_offset;
 		uint case_index;
-
-		StrStackItem(std::string_view view, size_t first_param_offset, uint case_index)
-			: str(view.data()), end(view.data() + view.size()), first_param_offset(first_param_offset), case_index(case_index)
-		{}
 	};
 	std::stack<StrStackItem, std::vector<StrStackItem>> str_stack;
-	str_stack.emplace(str_arg, orig_first_param_offset, orig_case_index);
+	str_stack.emplace(StringConsumer(str_arg), orig_first_param_offset, orig_case_index);
 
 	for (;;) {
 		try {
-			while (!str_stack.empty() && str_stack.top().str >= str_stack.top().end) {
+			while (!str_stack.empty() && str_stack.top().str.empty()) {
 				str_stack.pop();
 			}
 			if (str_stack.empty()) break;
-			const char *&str = str_stack.top().str;
+			StringConsumer &str = str_stack.top().str;
 			const size_t ref_param_offset = str_stack.top().first_param_offset;
 			const uint case_index = str_stack.top().case_index;
-			char32_t b = Utf8Consume(&str);
+			char32_t b = str.Utf8Consume();
 			assert(b != 0);
 
 			if (SCC_NEWGRF_FIRST <= b && b <= SCC_NEWGRF_LAST) {
 				/* We need to pass some stuff as it might be modified. */
-				b = RemapNewGRFStringControlCode(b, &str);
+				b = RemapNewGRFStringControlCode(b, str);
 				if (b == 0) continue;
 			}
 
@@ -1137,13 +1131,13 @@ static void FormatString(StringBuilder &builder, std::string_view str_arg, Strin
 			switch (b) {
 				case SCC_ENCODED:
 				case SCC_ENCODED_INTERNAL:
-					str = DecodeEncodedString(str, b == SCC_ENCODED, builder);
+					DecodeEncodedString(str, b == SCC_ENCODED, builder);
 					break;
 
 				case SCC_NEWGRF_STRINL: {
-					StringID substr = Utf8Consume(&str);
+					StringID substr = str.Utf8Consume();
 					std::string_view ptr = GetStringPtr(substr);
-					str_stack.emplace(ptr, args.GetOffset(), next_substr_case_index); // this may invalidate "str"
+					str_stack.emplace(StringConsumer(ptr), args.GetOffset(), next_substr_case_index); // this may invalidate "str"
 					next_substr_case_index = 0;
 					break;
 				}
@@ -1151,14 +1145,14 @@ static void FormatString(StringBuilder &builder, std::string_view str_arg, Strin
 				case SCC_NEWGRF_PRINT_WORD_STRING_ID: {
 					StringID substr = args.GetNextParameter<StringID>();
 					std::string_view ptr = GetStringPtr(substr);
-					str_stack.emplace(ptr, args.GetOffset(), next_substr_case_index); // this may invalidate "str"
+					str_stack.emplace(StringConsumer(ptr), args.GetOffset(), next_substr_case_index); // this may invalidate "str"
 					next_substr_case_index = 0;
 					break;
 				}
 
 				case SCC_GENDER_LIST: { // {G 0 Der Die Das}
 					/* First read the meta data from the language file. */
-					size_t offset = ref_param_offset + (uint8_t)*str++;
+					size_t offset = ref_param_offset + str.Uint8Consume();
 					int gender = 0;
 					if (offset >= args.GetNumParameters()) {
 						/* The offset may come from an external NewGRF, and be invalid. */
@@ -1167,50 +1161,54 @@ static void FormatString(StringBuilder &builder, std::string_view str_arg, Strin
 						/* Now we need to figure out what text to resolve, i.e.
 						 * what do we need to draw? So get the actual raw string
 						 * first using the control code to get said string. */
-						char input[4 + 1];
+						std::string input;
-						char *p = input + Utf8Encode(input, args.GetTypeAtOffset(offset));
+						{
-						*p = '\0';
+							StringBuilder tmp_builder(input);
+							tmp_builder.Utf8Encode(args.GetTypeAtOffset(offset));
+						}
 
 						/* The gender is stored at the start of the formatted string. */
-						bool old_sgd = _scan_for_gender_data;
-						_scan_for_gender_data = true;
 						std::string buffer;
-						StringBuilder tmp_builder(buffer);
+						{
-						StringParameters tmp_params = args.GetRemainingParameters(offset);
+							bool old_sgd = _scan_for_gender_data;
-						FormatString(tmp_builder, input, tmp_params);
+							_scan_for_gender_data = true;
-						_scan_for_gender_data = old_sgd;
+							StringBuilder tmp_builder(buffer);
+							StringParameters tmp_params = args.GetRemainingParameters(offset);
+							FormatString(tmp_builder, input, tmp_params);
+							_scan_for_gender_data = old_sgd;
+						}
 
 						/* And determine the string. */
-						const char *s = buffer.c_str();
+						StringConsumer tmp_consumer(buffer);
-						char32_t c = Utf8Consume(&s);
+						char32_t c = tmp_consumer.Utf8Consume();
 						/* Does this string have a gender, if so, set it */
-						if (c == SCC_GENDER_INDEX) gender = (uint8_t)s[0];
+						if (c == SCC_GENDER_INDEX) gender = tmp_consumer.Uint8Consume();
 					}
-					str = ParseStringChoice(str, gender, builder);
+					ParseStringChoice(str, gender, builder);
 					break;
 				}
 
 				/* This sets up the gender for the string.
 				 * We just ignore this one. It's used in {G 0 Der Die Das} to determine the case. */
-				case SCC_GENDER_INDEX: // {GENDER 0}
+				case SCC_GENDER_INDEX: { // {GENDER 0}
+					uint8_t gender = str.Uint8Consume();
 					if (_scan_for_gender_data) {
 						builder.Utf8Encode(SCC_GENDER_INDEX);
-						builder += *str++;
+						builder += gender;
-					} else {
-						str++;
 					}
 					break;
+				}
 
 				case SCC_PLURAL_LIST: { // {P}
-					int plural_form = *str++;          // contains the plural form for this string
+					int plural_form = str.Uint8Consume(); // contains the plural form for this string
-					size_t offset = ref_param_offset + (uint8_t)*str++;
+					size_t offset = ref_param_offset + str.Uint8Consume();
 					const uint64_t *v = nullptr;
 					/* The offset may come from an external NewGRF, and be invalid. */
 					if (offset < args.GetNumParameters()) {
 						v = std::get_if<uint64_t>(&args.GetParam(offset)); // contains the number that determines plural
 					}
 					if (v != nullptr) {
-						str = ParseStringChoice(str, DeterminePluralForm(static_cast<int64_t>(*v), plural_form), builder);
+						ParseStringChoice(str, DeterminePluralForm(static_cast<int64_t>(*v), plural_form), builder);
 					} else {
 						builder += "(invalid PLURAL parameter)";
 					}
@@ -1218,38 +1216,34 @@ static void FormatString(StringBuilder &builder, std::string_view str_arg, Strin
 				}
 
 				case SCC_ARG_INDEX: { // Move argument pointer
-					args.SetOffset(ref_param_offset + (uint8_t)*str++);
+					args.SetOffset(ref_param_offset + str.Uint8Consume());
 					break;
 				}
 
 				case SCC_SET_CASE: { // {SET_CASE}
 					/* This is a pseudo command, it's outputted when someone does {STRING.ack}
 					 * The modifier is added to all subsequent GetStringWithArgs that accept the modifier. */
-					next_substr_case_index = (uint8_t)*str++;
+					next_substr_case_index = str.Uint8Consume();
 					break;
 				}
 
 				case SCC_SWITCH_CASE: { // {Used to implement case switching}
 					/* <0x9E> <NUM CASES> <CASE1> <LEN1> <STRING1> <CASE2> <LEN2> <STRING2> <CASE3> <LEN3> <STRING3> <LENDEFAULT> <STRINGDEFAULT>
 					 * Each LEN is printed using 2 bytes in little endian order. */
-					uint num = (uint8_t)*str++;
+					uint num = str.Uint8Consume();
 					std::optional<std::string_view> found;
 					for (; num > 0; --num) {
-						uint8_t index = static_cast<uint8_t>(str[0]);
+						uint8_t index = str.Uint8Consume();
-						uint16_t len = static_cast<uint8_t>(str[1]) + (static_cast<uint8_t>(str[2]) << 8);
+						uint16_t len = str.Uint16LEConsume();
-						str += 3;
+						std::string_view case_data = str.StrConsume(len);
 						if (index == case_index) {
 							/* Found the case */
-							found.emplace(str, len);
+							found = case_data;
 						}
-						str += len;
 					}
-					uint16_t default_len = static_cast<uint8_t>(str[0]) + (static_cast<uint8_t>(str[1]) << 8);
+					uint16_t default_len = str.Uint16LEConsume();
-					str += 2;
+					std::string_view default_data = str.StrConsume(default_len);
-					if (!found.has_value()) found.emplace(str, default_len);
+					str_stack.emplace(StringConsumer(found.value_or(default_data)), ref_param_offset, case_index); // this may invalidate "str"
-					str += default_len;
-					assert(str <= str_stack.top().end);
-					str_stack.emplace(*found, ref_param_offset, case_index); // this may invalidate "str"
 					break;
 				}
 

src/strings_internal.h

@@ -311,6 +311,6 @@ void GenerateTownNameString(StringBuilder &builder, size_t lang, uint32_t seed);
 void GetTownName(StringBuilder &builder, const struct Town *t);
 void GRFTownNameGenerate(StringBuilder &builder, uint32_t grfid, uint16_t gen, uint32_t seed);
 
-char32_t RemapNewGRFStringControlCode(char32_t scc, const char **str);
+char32_t RemapNewGRFStringControlCode(char32_t scc, class StringConsumer &str);
 
 #endif /* STRINGS_INTERNAL_H */