mirror
/
OpenTTD
mirror of https://github.com/OpenTTD/OpenTTD
1
0
Fork 0
Code Issues

Codechange: ParseInteger optionally clamps out-of-range values, instead of treating them as invalid.

pull/14288/head
frosch 2025-05-16 12:07:12 +02:00 committed by frosch
parent ecadf1b322
commit c1389c77b2
2 changed files with 60 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
src/core/string_consumer.hpp
View File

@ -797,12 +797,12 @@ public:
private: private:
template <class T> template <class T>
[[nodiscard]] static std::pair<size_type, T> ParseIntegerBase(std::string_view src, int base, bool log_errors) [[nodiscard]] static std::pair<size_type, T> ParseIntegerBase(std::string_view src, int base, bool clamp, bool log_errors)
{ {
if (base == 0) { if (base == 0) {
/* Try positive hex */ /* Try positive hex */
if (src.starts_with("0x") || src.starts_with("0X")) { if (src.starts_with("0x") || src.starts_with("0X")) {
auto [len, value] = ParseIntegerBase<T>(src.substr(2), 16, log_errors); auto [len, value] = ParseIntegerBase<T>(src.substr(2), 16, clamp, log_errors);
if (len == 0) return {}; if (len == 0) return {};
return {len + 2, value}; return {len + 2, value};
} }
@ -810,18 +810,21 @@ private:
/* Try negative hex */ /* Try negative hex */
if (std::is_signed_v<T> && (src.starts_with("-0x") || src.starts_with("-0X"))) { if (std::is_signed_v<T> && (src.starts_with("-0x") || src.starts_with("-0X"))) {
using Unsigned = std::make_unsigned_t<T>; using Unsigned = std::make_unsigned_t<T>;
auto [len, uvalue] = ParseIntegerBase<Unsigned>(src.substr(3), 16, log_errors); auto [len, uvalue] = ParseIntegerBase<Unsigned>(src.substr(3), 16, clamp, log_errors);
if (len == 0) return {}; if (len == 0) return {};
T value = static_cast<T>(0 - uvalue); T value = static_cast<T>(0 - uvalue);
if (value > 0) { if (value > 0) {
if (log_errors) LogError(fmt::format("Integer out of range: '{}'", src.substr(0, len + 3))); if (!clamp) {
return {}; if (log_errors) LogError(fmt::format("Integer out of range: '{}'", src.substr(0, len + 3)));
return {};
}
value = std::numeric_limits<T>::lowest();
} }
return {len + 3, value}; return {len + 3, value};
} }
/* Try decimal */ /* Try decimal */
return ParseIntegerBase<T>(src, 10, log_errors); return ParseIntegerBase<T>(src, 10, clamp, log_errors);
} }
T value{}; T value{};
@ -829,10 +832,16 @@ private:
auto result = std::from_chars(src.data(), src.data() + src.size(), value, base); auto result = std::from_chars(src.data(), src.data() + src.size(), value, base);
auto len = result.ptr - src.data(); auto len = result.ptr - src.data();
if (result.ec == std::errc::result_out_of_range) { if (result.ec == std::errc::result_out_of_range) {
if (log_errors) LogError(fmt::format("Integer out of range: '{}'+'{}'", src.substr(0, len), src.substr(len, 4))); if (!clamp) {
return {}; if (log_errors) LogError(fmt::format("Integer out of range: '{}'+'{}'", src.substr(0, len), src.substr(len, 4)));
} return {};
if (result.ec != std::errc{}) { }
if (src.starts_with("-")) {
value = std::numeric_limits<T>::lowest();
} else {
value = std::numeric_limits<T>::max();
}
} else if (result.ec != std::errc{}) {
if (log_errors) LogError(fmt::format("Cannot parse integer: '{}'+'{}'", src.substr(0, len), src.substr(len, 4))); if (log_errors) LogError(fmt::format("Cannot parse integer: '{}'+'{}'", src.substr(0, len), src.substr(len, 4)));
return {}; return {};
} }
@ -843,24 +852,28 @@ public:
/** /**
* Peek and parse an integer in number 'base'. * Peek and parse an integer in number 'base'.
* If 'base == 0', then a prefix '0x' decides between base 16 or base 10. * If 'base == 0', then a prefix '0x' decides between base 16 or base 10.
* @param clamp If the value is a valid number, but out of range for T, return the maximum representable value.
* Negative values for unsigned results are still treated as invalid.
* @return Length of string match, and parsed value. * @return Length of string match, and parsed value.
* @note The parser rejects leading whitespace and unary plus. * @note The parser rejects leading whitespace and unary plus.
*/ */
template <class T> template <class T>
[[nodiscard]] std::pair<size_type, T> PeekIntegerBase(int base) const [[nodiscard]] std::pair<size_type, T> PeekIntegerBase(int base, bool clamp = false) const
{ {
return ParseIntegerBase<T>(this->src.substr(this->position), base, false); return ParseIntegerBase<T>(this->src.substr(this->position), base, clamp, false);
} }
/** /**
* Try to read and parse an integer in number 'base', and then advance the reader. * Try to read and parse an integer in number 'base', and then advance the reader.
* If 'base == 0', then a prefix '0x' decides between base 16 or base 10. * If 'base == 0', then a prefix '0x' decides between base 16 or base 10.
* @param clamp If the value is a valid number, but out of range for T, return the maximum representable value.
* Negative values for unsigned results are still treated as invalid.
* @return Parsed value, if valid. * @return Parsed value, if valid.
* @note The parser rejects leading whitespace and unary plus. * @note The parser rejects leading whitespace and unary plus.
*/ */
template <class T> template <class T>
[[nodiscard]] std::optional<T> TryReadIntegerBase(int base) [[nodiscard]] std::optional<T> TryReadIntegerBase(int base, bool clamp = false)
{ {
auto [len, value] = this->PeekIntegerBase<T>(base); auto [len, value] = this->PeekIntegerBase<T>(base, clamp);
if (len == 0) return std::nullopt; if (len == 0) return std::nullopt;
this->SkipIntegerBase(base); this->SkipIntegerBase(base);
return value; return value;
@ -868,14 +881,16 @@ public:
/** /**
* Read and parse an integer in number 'base', and advance the reader. * Read and parse an integer in number 'base', and advance the reader.
* If 'base == 0', then a prefix '0x' decides between base 16 or base 10. * If 'base == 0', then a prefix '0x' decides between base 16 or base 10.
* @param clamp If the value is a valid number, but out of range for T, return the maximum representable value.
* Negative values for unsigned results are still treated as invalid.
* @return Parsed value, or 'def' if invalid. * @return Parsed value, or 'def' if invalid.
* @note The reader is advanced, even if no valid data was present. * @note The reader is advanced, even if no valid data was present.
* @note The parser rejects leading whitespace and unary plus. * @note The parser rejects leading whitespace and unary plus.
*/ */
template <class T> template <class T>
[[nodiscard]] T ReadIntegerBase(int base, T def = 0) [[nodiscard]] T ReadIntegerBase(int base, T def = 0, bool clamp = false)
{ {
auto [len, value] = ParseIntegerBase<T>(this->src.substr(this->position), base, true); auto [len, value] = ParseIntegerBase<T>(this->src.substr(this->position), base, clamp, true);
this->SkipIntegerBase(base); // always advance this->SkipIntegerBase(base); // always advance
return len > 0 ? value : def; return len > 0 ? value : def;
} }
@ -894,14 +909,16 @@ public:
* Accepts leading and trailing whitespace. Trailing junk is an error. * Accepts leading and trailing whitespace. Trailing junk is an error.
* @param arg The string to be converted. * @param arg The string to be converted.
* @param base The base for parsing the number, defaults to only decimal numbers. Use 0 to also allow hexadecimal. * @param base The base for parsing the number, defaults to only decimal numbers. Use 0 to also allow hexadecimal.
* @param clamp If the value is a valid number, but out of range for T, return the maximum representable value.
* Negative values for unsigned results are still treated as invalid.
* @return The number, or std::nullopt if it could not be parsed. * @return The number, or std::nullopt if it could not be parsed.
*/ */
template <class T = uint32_t> template <class T = uint32_t>
static inline std::optional<T> ParseInteger(std::string_view arg, int base = 10) static inline std::optional<T> ParseInteger(std::string_view arg, int base = 10, bool clamp = false)
{ {
StringConsumer consumer{arg}; StringConsumer consumer{arg};
consumer.SkipUntilCharNotIn(StringConsumer::WHITESPACE_NO_NEWLINE); consumer.SkipUntilCharNotIn(StringConsumer::WHITESPACE_NO_NEWLINE);
auto result = consumer.TryReadIntegerBase<T>(base); auto result = consumer.TryReadIntegerBase<T>(base, clamp);
consumer.SkipUntilCharNotIn(StringConsumer::WHITESPACE_NO_NEWLINE); consumer.SkipUntilCharNotIn(StringConsumer::WHITESPACE_NO_NEWLINE);
if (consumer.AnyBytesLeft()) return std::nullopt; if (consumer.AnyBytesLeft()) return std::nullopt;
return result; return result;

26
src/tests/string_consumer.cpp
View File

@ -321,6 +321,8 @@ TEST_CASE("StringConsumer - parse int")
CHECK(consumer.ReadUtf8() == ' '); CHECK(consumer.ReadUtf8() == ' ');
CHECK(consumer.PeekIntegerBase<uint32_t>(16) == std::pair<StringConsumer::size_type, uint32_t>(8, 0xffffffff)); CHECK(consumer.PeekIntegerBase<uint32_t>(16) == std::pair<StringConsumer::size_type, uint32_t>(8, 0xffffffff));
CHECK(consumer.PeekIntegerBase<int32_t>(16) == std::pair<StringConsumer::size_type, int32_t>(0, 0)); CHECK(consumer.PeekIntegerBase<int32_t>(16) == std::pair<StringConsumer::size_type, int32_t>(0, 0));
CHECK(consumer.PeekIntegerBase<uint32_t>(16, true) == std::pair<StringConsumer::size_type, uint32_t>(8, 0xffffffff));
CHECK(consumer.PeekIntegerBase<int32_t>(16, true) == std::pair<StringConsumer::size_type, int32_t>(8, 0x7fffffff));
CHECK(consumer.TryReadIntegerBase<int32_t>(16) == std::nullopt); CHECK(consumer.TryReadIntegerBase<int32_t>(16) == std::nullopt);
CHECK(consumer.ReadIntegerBase<int32_t>(16) == 0); CHECK(consumer.ReadIntegerBase<int32_t>(16) == 0);
CHECK(consumer.ReadUtf8() == ' '); CHECK(consumer.ReadUtf8() == ' ');
@ -328,6 +330,8 @@ TEST_CASE("StringConsumer - parse int")
CHECK(consumer.ReadUtf8() == ' '); CHECK(consumer.ReadUtf8() == ' ');
CHECK(consumer.PeekIntegerBase<uint32_t>(16) == std::pair<StringConsumer::size_type, uint32_t>(0, 0)); CHECK(consumer.PeekIntegerBase<uint32_t>(16) == std::pair<StringConsumer::size_type, uint32_t>(0, 0));
CHECK(consumer.PeekIntegerBase<int32_t>(16) == std::pair<StringConsumer::size_type, int32_t>(9, -0x1aaaaaaa)); CHECK(consumer.PeekIntegerBase<int32_t>(16) == std::pair<StringConsumer::size_type, int32_t>(9, -0x1aaaaaaa));
CHECK(consumer.PeekIntegerBase<uint32_t>(16, true) == std::pair<StringConsumer::size_type, uint32_t>(0, 0));
CHECK(consumer.PeekIntegerBase<int32_t>(16, true) == std::pair<StringConsumer::size_type, int32_t>(9, -0x1aaaaaaa));
CHECK(consumer.TryReadIntegerBase<uint32_t>(16) == std::nullopt); CHECK(consumer.TryReadIntegerBase<uint32_t>(16) == std::nullopt);
CHECK(consumer.ReadIntegerBase<uint32_t>(16) == 0); CHECK(consumer.ReadIntegerBase<uint32_t>(16) == 0);
CHECK(consumer.ReadUtf8() == ' '); CHECK(consumer.ReadUtf8() == ' ');
@ -350,6 +354,10 @@ TEST_CASE("StringConsumer - parse int")
CHECK(consumer.PeekIntegerBase<int32_t>(10) == std::pair<StringConsumer::size_type, int32_t>(0, 0)); CHECK(consumer.PeekIntegerBase<int32_t>(10) == std::pair<StringConsumer::size_type, int32_t>(0, 0));
CHECK(consumer.PeekIntegerBase<uint64_t>(10) == std::pair<StringConsumer::size_type, uint64_t>(13, 1234567890123)); CHECK(consumer.PeekIntegerBase<uint64_t>(10) == std::pair<StringConsumer::size_type, uint64_t>(13, 1234567890123));
CHECK(consumer.PeekIntegerBase<int64_t>(10) == std::pair<StringConsumer::size_type, int64_t>(13, 1234567890123)); CHECK(consumer.PeekIntegerBase<int64_t>(10) == std::pair<StringConsumer::size_type, int64_t>(13, 1234567890123));
CHECK(consumer.PeekIntegerBase<uint32_t>(10, true) == std::pair<StringConsumer::size_type, uint32_t>(13, 0xffffffff));
CHECK(consumer.PeekIntegerBase<int32_t>(10, true) == std::pair<StringConsumer::size_type, int32_t>(13, 0x7fffffff));
CHECK(consumer.PeekIntegerBase<uint64_t>(10, true) == std::pair<StringConsumer::size_type, uint64_t>(13, 1234567890123));
CHECK(consumer.PeekIntegerBase<int64_t>(10, true) == std::pair<StringConsumer::size_type, int64_t>(13, 1234567890123));
CHECK(consumer.TryReadIntegerBase<uint32_t>(10) == std::nullopt); CHECK(consumer.TryReadIntegerBase<uint32_t>(10) == std::nullopt);
CHECK(consumer.ReadIntegerBase<uint32_t>(10) == 0); CHECK(consumer.ReadIntegerBase<uint32_t>(10) == 0);
CHECK(consumer.ReadUtf8() == ' '); CHECK(consumer.ReadUtf8() == ' ');
@ -370,6 +378,10 @@ TEST_CASE("StringConsumer - parse int")
CHECK(consumer.PeekIntegerBase<int32_t>(16) == std::pair<StringConsumer::size_type, int32_t>(0, 0)); CHECK(consumer.PeekIntegerBase<int32_t>(16) == std::pair<StringConsumer::size_type, int32_t>(0, 0));
CHECK(consumer.PeekIntegerBase<uint64_t>(16) == std::pair<StringConsumer::size_type, uint64_t>(16, 0xffffffff'fffffffe)); CHECK(consumer.PeekIntegerBase<uint64_t>(16) == std::pair<StringConsumer::size_type, uint64_t>(16, 0xffffffff'fffffffe));
CHECK(consumer.PeekIntegerBase<int64_t>(16) == std::pair<StringConsumer::size_type, int64_t>(0, 0)); CHECK(consumer.PeekIntegerBase<int64_t>(16) == std::pair<StringConsumer::size_type, int64_t>(0, 0));
CHECK(consumer.PeekIntegerBase<uint32_t>(16, true) == std::pair<StringConsumer::size_type, uint32_t>(16, 0xffffffff));
CHECK(consumer.PeekIntegerBase<int32_t>(16, true) == std::pair<StringConsumer::size_type, int32_t>(16, 0x7fffffff));
CHECK(consumer.PeekIntegerBase<uint64_t>(16, true) == std::pair<StringConsumer::size_type, uint64_t>(16, 0xffffffff'fffffffe));
CHECK(consumer.PeekIntegerBase<int64_t>(16, true) == std::pair<StringConsumer::size_type, int64_t>(16, 0x7fffffff'ffffffff));
CHECK(consumer.ReadIntegerBase<uint32_t>(16) == 0); CHECK(consumer.ReadIntegerBase<uint32_t>(16) == 0);
CHECK(consumer.ReadUtf8() == ' '); CHECK(consumer.ReadUtf8() == ' ');
CHECK(consumer.PeekIntegerBase<uint32_t>(16) == std::pair<StringConsumer::size_type, uint32_t>(0, 0)); CHECK(consumer.PeekIntegerBase<uint32_t>(16) == std::pair<StringConsumer::size_type, uint32_t>(0, 0));
@ -500,9 +512,11 @@ TEST_CASE("StringConsumer - invalid int")
TEST_CASE("StringConsumer - most negative") TEST_CASE("StringConsumer - most negative")
{ {
StringConsumer consumer("-80000000 -0x80000000 -2147483648"sv); StringConsumer consumer("-80000000 -0x80000000 -2147483648 -9223372036854775808"sv);
CHECK(consumer.PeekIntegerBase<uint32_t>(16) == std::pair<StringConsumer::size_type, uint32_t>(0, 0)); CHECK(consumer.PeekIntegerBase<uint32_t>(16) == std::pair<StringConsumer::size_type, uint32_t>(0, 0));
CHECK(consumer.PeekIntegerBase<int32_t>(16) == std::pair<StringConsumer::size_type, int32_t>(9, 0x80000000)); CHECK(consumer.PeekIntegerBase<int32_t>(16) == std::pair<StringConsumer::size_type, int32_t>(9, 0x80000000));
CHECK(consumer.PeekIntegerBase<uint32_t>(16, true) == std::pair<StringConsumer::size_type, uint32_t>(0, 0));
CHECK(consumer.PeekIntegerBase<int32_t>(16, true) == std::pair<StringConsumer::size_type, int32_t>(9, 0x80000000));
consumer.SkipIntegerBase(16); consumer.SkipIntegerBase(16);
CHECK(consumer.ReadUtf8() == ' '); CHECK(consumer.ReadUtf8() == ' ');
CHECK(consumer.PeekIntegerBase<uint32_t>(0) == std::pair<StringConsumer::size_type, uint32_t>(0, 0)); CHECK(consumer.PeekIntegerBase<uint32_t>(0) == std::pair<StringConsumer::size_type, uint32_t>(0, 0));
@ -511,4 +525,14 @@ TEST_CASE("StringConsumer - most negative")
CHECK(consumer.ReadUtf8() == ' '); CHECK(consumer.ReadUtf8() == ' ');
CHECK(consumer.PeekIntegerBase<uint32_t>(10) == std::pair<StringConsumer::size_type, uint32_t>(0, 0)); CHECK(consumer.PeekIntegerBase<uint32_t>(10) == std::pair<StringConsumer::size_type, uint32_t>(0, 0));
CHECK(consumer.PeekIntegerBase<int32_t>(10) == std::pair<StringConsumer::size_type, int32_t>(11, 0x80000000)); CHECK(consumer.PeekIntegerBase<int32_t>(10) == std::pair<StringConsumer::size_type, int32_t>(11, 0x80000000));
consumer.SkipIntegerBase(0);
CHECK(consumer.ReadUtf8() == ' ');
CHECK(consumer.PeekIntegerBase<uint32_t>(10) == std::pair<StringConsumer::size_type, uint32_t>(0, 0));
CHECK(consumer.PeekIntegerBase<int32_t>(10) == std::pair<StringConsumer::size_type, int32_t>(0, 0));
CHECK(consumer.PeekIntegerBase<uint64_t>(10) == std::pair<StringConsumer::size_type, uint64_t>(0, 0));
CHECK(consumer.PeekIntegerBase<int64_t>(10) == std::pair<StringConsumer::size_type, int64_t>(20, 0x80000000'00000000));
CHECK(consumer.PeekIntegerBase<uint32_t>(10, true) == std::pair<StringConsumer::size_type, uint32_t>(0, 0));
CHECK(consumer.PeekIntegerBase<int32_t>(10, true) == std::pair<StringConsumer::size_type, int32_t>(20, 0x80000000));
CHECK(consumer.PeekIntegerBase<uint64_t>(10, true) == std::pair<StringConsumer::size_type, uint64_t>(0, 0));
CHECK(consumer.PeekIntegerBase<int64_t>(10, true) == std::pair<StringConsumer::size_type, int64_t>(20, 0x80000000'00000000));
} }