Skip to content

Commit

Permalink
[strings] Improve overflow checks in to_integer
Browse files Browse the repository at this point in the history
  • Loading branch information
@tobiashienzsch
tobiashienzsch committed Apr 23, 2024
1 parent 7136409 commit a940c60
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 21 deletions.
90 changes: 74 additions & 16 deletions include/etl/_strings/to_integer.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,71 @@
#include <etl/_cctype/isdigit.hpp>
#include <etl/_cctype/isspace.hpp>
#include <etl/_cctype/tolower.hpp>
#include <etl/_concepts/integral.hpp>
#include <etl/_concepts/signed_integral.hpp>
#include <etl/_cstddef/size_t.hpp>
#include <etl/_iterator/next.hpp>
#include <etl/_limits/numeric_limits.hpp>
#include <etl/_numeric/abs.hpp>
#include <etl/_string_view/basic_string_view.hpp>
#include <etl/_type_traits/conditional.hpp>
#include <etl/_type_traits/is_signed.hpp>

namespace etl::strings {

namespace detail {

template <etl::integral Int>
struct nop_overflow_checker {
explicit constexpr nop_overflow_checker(Int /*base*/) noexcept { }

[[nodiscard]] constexpr auto operator()(Int /*value*/, Int /*digit*/) const noexcept -> bool { return false; }
};

template <etl::integral Int>
struct unsigned_overflow_checker {
explicit constexpr unsigned_overflow_checker(Int base) noexcept
: _base{base}
{
}

[[nodiscard]] constexpr auto operator()(Int value, Int digit) const noexcept -> bool
{
return value > _maxDivBase or (value == _maxDivBase and digit > _maxModBase);
}

private:
Int _base;
Int _maxDivBase{static_cast<Int>(etl::numeric_limits<Int>::max() / _base)};
Int _maxModBase{static_cast<Int>(etl::numeric_limits<Int>::max() % _base)};
};

template <etl::integral Int>
struct signed_overflow_checker {
explicit constexpr signed_overflow_checker(Int base) noexcept
: _base{base}
{
}

[[nodiscard]] constexpr auto operator()(Int value, Int digit) const noexcept -> bool
{
return value < _minDivBase or (value == _minDivBase and digit > _minModBase);
}

private:
Int _base;
Int _minDivBase{static_cast<Int>(etl::numeric_limits<Int>::min() / _base)};
Int _minModBase{etl::abs(static_cast<Int>(etl::numeric_limits<Int>::min() % _base))};
};

template <etl::integral Int, bool Check>
using overflow_checker = etl::conditional_t<
Check,
etl::conditional_t<signed_integral<Int>, signed_overflow_checker<Int>, unsigned_overflow_checker<Int>>,
nop_overflow_checker<Int>>;

} // namespace detail

struct to_integer_options {
bool skip_whitespace = true;
bool check_overflow = true;
Expand All @@ -26,25 +83,16 @@ enum struct to_integer_error : unsigned char {
overflow,
};

template <typename Int>
template <etl::integral Int>
struct to_integer_result {
char const* end{nullptr};
to_integer_error error{to_integer_error::none};
Int value;
};

template <typename Int, to_integer_options Options = to_integer_options{}>
template <etl::integral Int, to_integer_options Options = to_integer_options{}>
[[nodiscard]] constexpr auto to_integer(etl::string_view str, Int base = Int(10)) noexcept -> to_integer_result<Int>
{
constexpr auto const max = etl::numeric_limits<Int>::max();
auto const wouldOverflow = [maxDivBase = max / base, maxModBase = max % base](Int val, Int digit) -> bool {
if constexpr (Options.check_overflow) {
return val > maxDivBase or (val == maxDivBase and digit > maxModBase);
} else {
return false;
}
};

auto const len = str.size();
auto pos = size_t{};

Expand All @@ -59,15 +107,16 @@ template <typename Int, to_integer_options Options = to_integer_options{}>
}

// optional minus for signed types
[[maybe_unused]] auto sign = Int(1);
[[maybe_unused]] auto positive = true;
if constexpr (is_signed_v<Int>) {
if (str[pos] == '-') {
sign = Int(-1);
positive = false;
++pos;
}
}

auto const firstDigit = pos;
auto const firstDigit = pos;
auto const wouldOverflow = detail::overflow_checker<Int, Options.check_overflow>{base};

// loop over digits
auto value = Int{};
Expand Down Expand Up @@ -97,11 +146,20 @@ template <typename Int, to_integer_options Options = to_integer_options{}>
return {.end = str.data(), .error = to_integer_error::overflow, .value = Int{}};
}

value = static_cast<Int>(value * base + digit);
if constexpr (is_signed_v<Int>) {
value = static_cast<Int>(value * base - digit);
} else {
value = static_cast<Int>(value * base + digit);
}
}

if constexpr (is_signed_v<Int>) {
value *= sign;
if (positive) {
if (value == etl::numeric_limits<Int>::min()) {
return {.end = str.data(), .error = to_integer_error::overflow, .value = Int{}};
}
value *= Int(-1);
}
}

auto const end = etl::next(str.data(), static_cast<etl::ptrdiff_t>(pos));
Expand Down
54 changes: 49 additions & 5 deletions tests/strings/to_integer.t.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <etl/strings.hpp>

#include <etl/array.hpp>
#include <etl/cstdint.hpp>
#include <etl/string_view.hpp>
#include <etl/type_traits.hpp>

Expand All @@ -17,14 +18,27 @@ constexpr auto test() -> bool
using namespace etl::string_view_literals;
using namespace etl::strings;

constexpr auto noOverflowChecks = to_integer_options{
.skip_whitespace = true,
.check_overflow = false,
};

CHECK(to_integer<Int>({}, 10).end == nullptr);
CHECK(to_integer<Int>({}, 10).error == to_integer_error::invalid_input);

CHECK(to_integer<Int, noOverflowChecks>({}, 10).end == nullptr);
CHECK(to_integer<Int, noOverflowChecks>({}, 10).error == to_integer_error::invalid_input);

{
auto str = ""_sv;
auto str = ""_sv;

auto result = to_integer<Int>(str, 10);
CHECK(result.end == str.begin());
CHECK(result.error == to_integer_error::invalid_input);

auto resultNoChecks = to_integer<Int, noOverflowChecks>(str, 10);
CHECK(resultNoChecks.end == str.begin());
CHECK(resultNoChecks.error == to_integer_error::invalid_input);
}
{
auto str = "$"_sv;
Expand Down Expand Up @@ -53,6 +67,11 @@ constexpr auto test() -> bool
CHECK(result.error == to_integer_error::none);
CHECK(result.end == str.end());
CHECK(result.value == Int(10));

auto resultNoChecks = to_integer<Int, noOverflowChecks>(str, 16);
CHECK(resultNoChecks.error == to_integer_error::none);
CHECK(resultNoChecks.end == str.end());
CHECK(resultNoChecks.value == Int(10));
}

{
Expand Down Expand Up @@ -94,14 +113,39 @@ constexpr auto test() -> bool
CHECK(result.end == str.data());
}

if constexpr (etl::is_same_v<Int, signed char>) {
auto legal = "127"_sv;
CHECK(to_integer<Int>(legal, 10).value == Int(127));
CHECK(to_integer<Int>(legal, 10).error == to_integer_error::none);
if constexpr (etl::is_same_v<Int, etl::int8_t>) {
auto legalMin = "-128"_sv;
CHECK(to_integer<Int>(legalMin, 10).value == Int(-128));
CHECK(to_integer<Int>(legalMin, 10).error == to_integer_error::none);

auto legalMax = "127"_sv;
CHECK(to_integer<Int>(legalMax, 10).value == Int(127));
CHECK(to_integer<Int>(legalMax, 10).error == to_integer_error::none);

auto overflow = "128"_sv;
CHECK(to_integer<Int>(overflow, 10).end == overflow.begin());
CHECK(to_integer<Int>(overflow, 10).error == to_integer_error::overflow);

auto moreOverflow = "999"_sv;
CHECK(to_integer<Int>(moreOverflow, 10).end == moreOverflow.begin());
CHECK(to_integer<Int>(moreOverflow, 10).error == to_integer_error::overflow);
}

if constexpr (etl::is_same_v<Int, etl::uint8_t>) {
auto illegalMinus = "-1"_sv;
CHECK(to_integer<Int>(illegalMinus, 10).error == to_integer_error::invalid_input);

auto legalMax = "255"_sv;
CHECK(to_integer<Int>(legalMax, 10).value == Int(255));
CHECK(to_integer<Int>(legalMax, 10).error == to_integer_error::none);

auto overflow = "256"_sv;
CHECK(to_integer<Int>(overflow, 10).end == overflow.begin());
CHECK(to_integer<Int>(overflow, 10).error == to_integer_error::overflow);

auto moreOverflow = "999"_sv;
CHECK(to_integer<Int>(moreOverflow, 10).end == moreOverflow.begin());
CHECK(to_integer<Int>(moreOverflow, 10).error == to_integer_error::overflow);
}

if constexpr (sizeof(Int) < 4) {
Expand Down

0 comments on commit a940c60

Please sign in to comment.