From e421d527131fbdd50a451360549b47eb12b49e3f Mon Sep 17 00:00:00 2001 From: Victor Zverovich Date: Mon, 14 Jun 2021 15:33:35 -0700 Subject: [PATCH] Simplify error handling in parse_nonnegative_int --- include/fmt/core.h | 31 +++++++++++++++++++------------ include/fmt/printf.h | 13 ++++++------- test/format-test.cc | 26 +++++++++++--------------- test/printf-test.cc | 8 ++++---- 4 files changed, 40 insertions(+), 38 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index fd0782d33aa1..9e86afc45aa3 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -2099,9 +2099,9 @@ inline auto find(const char* first, const char* last, char value, // Parses the range [begin, end) as an unsigned integer. This function assumes // that the range is non-empty and the first character is a digit. -template +template FMT_CONSTEXPR auto parse_nonnegative_int(const Char*& begin, const Char* end, - ErrorHandler&& eh) -> int { + int error_value) noexcept -> int { FMT_ASSERT(begin != end && '0' <= *begin && *begin <= '9', ""); unsigned value = 0, prev = 0; auto p = begin; @@ -2115,13 +2115,11 @@ FMT_CONSTEXPR auto parse_nonnegative_int(const Char*& begin, const Char* end, if (num_digits <= std::numeric_limits::digits10) return static_cast(value); // Check for overflow. - const unsigned big = to_unsigned((std::numeric_limits::max)()); - if (num_digits == std::numeric_limits::digits10 + 1 && - prev * 10ull + unsigned(p[-1] - '0') <= big) { - return static_cast(value); - } - eh.on_error("number is too big"); - return -1; + const unsigned max = to_unsigned((std::numeric_limits::max)()); + return num_digits == std::numeric_limits::digits10 + 1 && + prev * 10ull + unsigned(p[-1] - '0') <= max + ? static_cast(value) + : error_value; } // Parses fill and alignment. @@ -2177,7 +2175,8 @@ FMT_CONSTEXPR auto do_parse_arg_id(const Char* begin, const Char* end, if (c >= '0' && c <= '9') { int index = 0; if (c != '0') - index = parse_nonnegative_int(begin, end, handler); + index = + parse_nonnegative_int(begin, end, (std::numeric_limits::max)()); else ++begin; if (begin == end || (*begin != '}' && *begin != ':')) @@ -2226,7 +2225,11 @@ FMT_CONSTEXPR auto parse_width(const Char* begin, const Char* end, FMT_ASSERT(begin != end, ""); if ('0' <= *begin && *begin <= '9') { - handler.on_width(parse_nonnegative_int(begin, end, handler)); + int width = parse_nonnegative_int(begin, end, -1); + if (width != -1) + handler.on_width(width); + else + handler.on_error("number is too big"); } else if (*begin == '{') { ++begin; if (begin != end) begin = parse_arg_id(begin, end, width_adapter{handler}); @@ -2257,7 +2260,11 @@ FMT_CONSTEXPR auto parse_precision(const Char* begin, const Char* end, ++begin; auto c = begin != end ? *begin : Char(); if ('0' <= c && c <= '9') { - handler.on_precision(parse_nonnegative_int(begin, end, handler)); + auto precision = parse_nonnegative_int(begin, end, -1); + if (precision != -1) + handler.on_precision(precision); + else + handler.on_error("number is too big"); } else if (c == '{') { ++begin; if (begin != end) diff --git a/include/fmt/printf.h b/include/fmt/printf.h index 7194b81d4838..3a3cd152830b 100644 --- a/include/fmt/printf.h +++ b/include/fmt/printf.h @@ -334,16 +334,16 @@ int parse_header(const Char*& it, const Char* end, if (c >= '0' && c <= '9') { // Parse an argument index (if followed by '$') or a width possibly // preceded with '0' flag(s). - detail::error_handler eh; - int value = parse_nonnegative_int(it, end, eh); + int value = parse_nonnegative_int(it, end, -1); if (it != end && *it == '$') { // value is an argument index ++it; - arg_index = value; + arg_index = value != -1 ? value : max_value(); } else { if (c == '0') specs.fill[0] = '0'; if (value != 0) { // Nonzero value means that we parsed width and don't need to // parse it or flags again, so return now. + if (value == -1) FMT_THROW(format_error("number is too big")); specs.width = value; return arg_index; } @@ -353,8 +353,8 @@ int parse_header(const Char*& it, const Char* end, // Parse width. if (it != end) { if (*it >= '0' && *it <= '9') { - detail::error_handler eh; - specs.width = parse_nonnegative_int(it, end, eh); + specs.width = parse_nonnegative_int(it, end, -1); + if (specs.width == -1) FMT_THROW(format_error("number is too big")); } else if (*it == '*') { ++it; specs.width = static_cast(visit_format_arg( @@ -412,8 +412,7 @@ void vprintf(buffer& buf, basic_string_view format, ++it; c = it != end ? *it : 0; if ('0' <= c && c <= '9') { - detail::error_handler eh; - specs.precision = parse_nonnegative_int(it, end, eh); + specs.precision = parse_nonnegative_int(it, end, 0); } else if (c == '*') { ++it; specs.precision = static_cast( diff --git a/test/format-test.cc b/test/format-test.cc index 19597a12c367..892556f18d30 100644 --- a/test/format-test.cc +++ b/test/format-test.cc @@ -78,15 +78,11 @@ TEST(util_test, increment) { TEST(util_test, parse_nonnegative_int) { auto s = fmt::string_view("10000000000"); auto begin = s.begin(), end = s.end(); - EXPECT_THROW_MSG( - parse_nonnegative_int(begin, end, fmt::detail::error_handler()), - fmt::format_error, "number is too big"); + EXPECT_EQ(fmt::detail::parse_nonnegative_int(begin, end, -1), -1); s = "2147483649"; begin = s.begin(); end = s.end(); - EXPECT_THROW_MSG( - parse_nonnegative_int(begin, end, fmt::detail::error_handler()), - fmt::format_error, "number is too big"); + EXPECT_EQ(fmt::detail::parse_nonnegative_int(begin, end, -1), -1); } TEST(util_test, utf8_to_utf16) { @@ -416,10 +412,10 @@ TEST(format_test, arg_errors) { safe_sprintf(format_str, "{%u", INT_MAX + 1u); EXPECT_THROW_MSG(fmt::format(runtime(format_str)), format_error, - "number is too big"); + "invalid format string"); safe_sprintf(format_str, "{%u}", INT_MAX + 1u); EXPECT_THROW_MSG(fmt::format(runtime(format_str)), format_error, - "number is too big"); + "argument not found"); } template struct test_format { @@ -747,16 +743,16 @@ TEST(format_test, runtime_width) { safe_sprintf(format_str, "{0:{%u", UINT_MAX); increment(format_str + 4); EXPECT_THROW_MSG(fmt::format(runtime(format_str), 0), format_error, - "number is too big"); + "invalid format string"); size_t size = std::strlen(format_str); format_str[size] = '}'; format_str[size + 1] = 0; EXPECT_THROW_MSG(fmt::format(runtime(format_str), 0), format_error, - "number is too big"); + "argument not found"); format_str[size + 1] = '}'; format_str[size + 2] = 0; EXPECT_THROW_MSG(fmt::format(runtime(format_str), 0), format_error, - "number is too big"); + "argument not found"); EXPECT_THROW_MSG(fmt::format(runtime("{0:{"), 0), format_error, "invalid format string"); @@ -924,16 +920,16 @@ TEST(format_test, runtime_precision) { safe_sprintf(format_str, "{0:.{%u", UINT_MAX); increment(format_str + 5); EXPECT_THROW_MSG(fmt::format(runtime(format_str), 0), format_error, - "number is too big"); + "invalid format string"); size_t size = std::strlen(format_str); format_str[size] = '}'; format_str[size + 1] = 0; EXPECT_THROW_MSG(fmt::format(runtime(format_str), 0), format_error, - "number is too big"); + "argument not found"); format_str[size + 1] = '}'; format_str[size + 2] = 0; EXPECT_THROW_MSG(fmt::format(runtime(format_str), 0), format_error, - "number is too big"); + "argument not found"); EXPECT_THROW_MSG(fmt::format(runtime("{0:.{"), 0), format_error, "invalid format string"); @@ -2029,7 +2025,7 @@ TEST(format_test, format_string_errors) { "compile-time checks for named arguments require C++20 support", int); # endif - EXPECT_ERROR_NOARGS("{10000000000}", "number is too big"); + EXPECT_ERROR_NOARGS("{10000000000}", "argument not found"); EXPECT_ERROR_NOARGS("{0x}", "invalid format string"); EXPECT_ERROR_NOARGS("{-}", "invalid format string"); EXPECT_ERROR("{:{0x}}", "invalid format string", int); diff --git a/test/printf-test.cc b/test/printf-test.cc index 295c2f421417..e22e0b9d7ad4 100644 --- a/test/printf-test.cc +++ b/test/printf-test.cc @@ -86,9 +86,9 @@ TEST(printf_test, automatic_arg_indexing) { TEST(printf_test, number_is_too_big_in_arg_index) { EXPECT_THROW_MSG(test_sprintf(format("%{}$", big_num)), format_error, - "number is too big"); + "argument not found"); EXPECT_THROW_MSG(test_sprintf(format("%{}$d", big_num)), format_error, - "number is too big"); + "argument not found"); } TEST(printf_test, switch_arg_indexing) { @@ -102,7 +102,7 @@ TEST(printf_test, switch_arg_indexing) { EXPECT_THROW_MSG(test_sprintf("%d%1$", 1, 2), format_error, "cannot switch from automatic to manual argument indexing"); EXPECT_THROW_MSG(test_sprintf(format("%d%{}$d", big_num), 1, 2), format_error, - "number is too big"); + "cannot switch from automatic to manual argument indexing"); EXPECT_THROW_MSG(test_sprintf("%d%1$d", 1, 2), format_error, "cannot switch from automatic to manual argument indexing"); @@ -123,7 +123,7 @@ TEST(printf_test, invalid_arg_index) { EXPECT_THROW_MSG(test_sprintf("%2$", 42), format_error, "argument not found"); EXPECT_THROW_MSG(test_sprintf(format("%{}$d", big_num), 42), format_error, - "number is too big"); + "argument not found"); } TEST(printf_test, default_align_right) {