From 6d18bd3b8ec82ae3c24a439f5c7925786a0e2d8b Mon Sep 17 00:00:00 2001 From: Wei Li Date: Fri, 25 Mar 2016 15:02:07 -0700 Subject: Fix FXSYS_StrToInt() Correctly handle sign and out of range values. R=dsinclair@chromium.org, tsepez@chromium.org Review URL: https://codereview.chromium.org/1828873002 . --- core/fxcrt/fx_basic_gcc.cpp | 18 ++++++-- core/fxcrt/fx_basic_gcc_unittest.cpp | 87 +++++++++++++++++++++++------------- 2 files changed, 69 insertions(+), 36 deletions(-) diff --git a/core/fxcrt/fx_basic_gcc.cpp b/core/fxcrt/fx_basic_gcc.cpp index 440f924795..c29c9956e2 100644 --- a/core/fxcrt/fx_basic_gcc.cpp +++ b/core/fxcrt/fx_basic_gcc.cpp @@ -16,15 +16,25 @@ IntType FXSYS_StrToInt(const CharType* str) { if (!str) return 0; - bool neg = std::numeric_limits::is_signed && *str == '-'; - if (neg) + // Process the sign. + bool neg = *str == '-'; + if (neg || *str == '+') str++; IntType num = 0; while (*str && FXSYS_isDecimalDigit(*str)) { IntType val = FXSYS_toDecimalDigit(*str); - if (num > (std::numeric_limits::max() - val) / 10) - break; + if (num > (std::numeric_limits::max() - val) / 10) { + if (neg && std::numeric_limits::is_signed) { + // Return MIN when the represented number is signed type and is smaller + // than the min value. + return std::numeric_limits::min(); + } else { + // Return MAX when the represented number is signed type and is larger + // than the max value, or the number is unsigned type and out of range. + return std::numeric_limits::max(); + } + } num = num * 10 + val; str++; diff --git a/core/fxcrt/fx_basic_gcc_unittest.cpp b/core/fxcrt/fx_basic_gcc_unittest.cpp index 1eac9e301a..8d8d2f5deb 100644 --- a/core/fxcrt/fx_basic_gcc_unittest.cpp +++ b/core/fxcrt/fx_basic_gcc_unittest.cpp @@ -10,16 +10,21 @@ TEST(fxcrt, FXSYS_atoi) { EXPECT_EQ(0, FXSYS_atoi("0")); EXPECT_EQ(-1, FXSYS_atoi("-1")); EXPECT_EQ(2345, FXSYS_atoi("2345")); - EXPECT_EQ(2147483647, FXSYS_atoi("2147483647")); EXPECT_EQ(-2147483647, FXSYS_atoi("-2147483647")); + // Handle the sign. + EXPECT_EQ(-2345, FXSYS_atoi("-2345")); + EXPECT_EQ(2345, FXSYS_atoi("+2345")); + // The max value. + EXPECT_EQ(2147483647, FXSYS_atoi("2147483647")); + // The min value. + EXPECT_EQ(-2147483648, FXSYS_atoi("-2147483648")); + // With invalid char. EXPECT_EQ(9, FXSYS_atoi("9x9")); - // TODO(dsinclair): These are all wacky ..... - EXPECT_EQ(2147483623, FXSYS_atoi("2147483623423412348")); - EXPECT_EQ(214748364, FXSYS_atoi("2147483648")); - // The digit is parsed as a positive value, so we end up not being able to - // handle the largest possible negative value. - EXPECT_EQ(-214748364, FXSYS_atoi("-2147483648")); + // Out of range values. + EXPECT_EQ(2147483647, FXSYS_atoi("2147483623423412348")); + EXPECT_EQ(2147483647, FXSYS_atoi("2147483648")); + EXPECT_EQ(-2147483648, FXSYS_atoi("-2147483650")); } TEST(fxcrt, FXSYS_atoi64) { @@ -27,16 +32,21 @@ TEST(fxcrt, FXSYS_atoi64) { EXPECT_EQ(0, FXSYS_atoi64("0")); EXPECT_EQ(-1, FXSYS_atoi64("-1")); EXPECT_EQ(2345, FXSYS_atoi64("2345")); - EXPECT_EQ(9223372036854775807LL, FXSYS_atoi64("9223372036854775807")); EXPECT_EQ(-9223372036854775807LL, FXSYS_atoi64("-9223372036854775807")); + // Handle the sign. + EXPECT_EQ(-2345, FXSYS_atoi64("-2345")); + EXPECT_EQ(2345, FXSYS_atoi64("+2345")); + // The max value. + EXPECT_EQ(9223372036854775807LL, FXSYS_atoi64("9223372036854775807")); + // The min value. Written in -1 format to avoid implicit unsigned warning. + EXPECT_EQ(-9223372036854775807LL - 1LL, FXSYS_atoi64("-9223372036854775808")); + // With invalid char. EXPECT_EQ(9, FXSYS_atoi64("9x9")); - // TODO(dsinclair): These are all wacky ..... - EXPECT_EQ(9223372036854712341LL, FXSYS_atoi64("922337203685471234123475807")); - EXPECT_EQ(922337203685477580LL, FXSYS_atoi64("9223372036854775808")); - // The digit is parsed as a positive value, so we end up not being able to - // handle the largest possible negative value. - EXPECT_EQ(-922337203685477580LL, FXSYS_atoi64("-9223372036854775808")); + // Out of range values. + EXPECT_EQ(9223372036854775807LL, FXSYS_atoi64("922337203685471234123475807")); + EXPECT_EQ(9223372036854775807LL, FXSYS_atoi64("9223372036854775808")); + EXPECT_EQ(-9223372036854775807LL - 1LL, FXSYS_atoi64("-9223372036854775810")); } TEST(fxcrt, FXSYS_wtoi) { @@ -44,16 +54,17 @@ TEST(fxcrt, FXSYS_wtoi) { EXPECT_EQ(0, FXSYS_wtoi(L"0")); EXPECT_EQ(-1, FXSYS_wtoi(L"-1")); EXPECT_EQ(2345, FXSYS_wtoi(L"2345")); - EXPECT_EQ(2147483647, FXSYS_wtoi(L"2147483647")); EXPECT_EQ(-2147483647, FXSYS_wtoi(L"-2147483647")); + // The max value. + EXPECT_EQ(2147483647, FXSYS_wtoi(L"2147483647")); + // The min value. + EXPECT_EQ(-2147483648, FXSYS_wtoi(L"-2147483648")); EXPECT_EQ(9, FXSYS_wtoi64(L"9x9")); - // TODO(dsinclair): These are all wacky ..... - EXPECT_EQ(2147483623, FXSYS_wtoi(L"2147483623423412348")); - EXPECT_EQ(214748364, FXSYS_wtoi(L"2147483648")); - // The digit is parsed as a positive value, so we end up not being able to - // handle the largest possible negative value. - EXPECT_EQ(-214748364, FXSYS_wtoi(L"-2147483648")); + // Out of range values. + EXPECT_EQ(2147483647, FXSYS_wtoi(L"2147483623423412348")); + EXPECT_EQ(2147483647, FXSYS_wtoi(L"2147483648")); + EXPECT_EQ(-2147483648, FXSYS_wtoi(L"-2147483652")); } TEST(fxcrt, FXSYS_wtoi64) { @@ -61,28 +72,40 @@ TEST(fxcrt, FXSYS_wtoi64) { EXPECT_EQ(0, FXSYS_wtoi64(L"0")); EXPECT_EQ(-1, FXSYS_wtoi64(L"-1")); EXPECT_EQ(2345, FXSYS_wtoi64(L"2345")); - EXPECT_EQ(9223372036854775807LL, FXSYS_wtoi64(L"9223372036854775807")); EXPECT_EQ(-9223372036854775807LL, FXSYS_wtoi64(L"-9223372036854775807")); + // Handle the sign. + EXPECT_EQ(-2345, FXSYS_wtoi64(L"-2345")); + EXPECT_EQ(2345, FXSYS_wtoi64(L"+2345")); + // The max value. + EXPECT_EQ(9223372036854775807LL, FXSYS_wtoi64(L"9223372036854775807")); + // The min value. + EXPECT_EQ(-9223372036854775807LL - 1LL, + FXSYS_wtoi64(L"-9223372036854775808")); + // With invalid char. EXPECT_EQ(9, FXSYS_wtoi64(L"9x9")); - // TODO(dsinclair): These are all wacky ..... - EXPECT_EQ(9223372036854712341LL, + // Out of range values. + EXPECT_EQ(9223372036854775807LL, FXSYS_wtoi64(L"922337203685471234123475807")); - EXPECT_EQ(922337203685477580LL, FXSYS_wtoi64(L"9223372036854775808")); - // The digit is parsed as a positive value, so we end up not being able to - // handle the largest possible negative value. - EXPECT_EQ(-922337203685477580LL, FXSYS_wtoi64(L"-9223372036854775808")); + EXPECT_EQ(9223372036854775807LL, FXSYS_wtoi64(L"9223372036854775808")); + EXPECT_EQ(-9223372036854775807LL - 1LL, + FXSYS_wtoi64(L"-9223372036854775810")); } TEST(fxcrt, FXSYS_atoui) { EXPECT_EQ(0, FXSYS_atoui("")); EXPECT_EQ(0, FXSYS_atoui("0")); - EXPECT_EQ(0, FXSYS_atoui("-1")); + EXPECT_EQ(4294967295, FXSYS_atoui("-1")); EXPECT_EQ(2345, FXSYS_atoui("2345")); + // Handle the sign. + EXPECT_EQ(4294964951, FXSYS_atoui("-2345")); + EXPECT_EQ(2345, FXSYS_atoui("+2345")); + // The max value. EXPECT_EQ(4294967295, FXSYS_atoui("4294967295")); EXPECT_EQ(9, FXSYS_atoui("9x9")); - // TODO(dsinclair): These are all wacky ..... - EXPECT_EQ(2147483623, FXSYS_atoi("2147483623423412348")); - EXPECT_EQ(429496729, FXSYS_atoi("4294967296")); + // Out of range values. + EXPECT_EQ(4294967295, FXSYS_atoui("2147483623423412348")); + EXPECT_EQ(4294967295, FXSYS_atoui("4294967296")); + EXPECT_EQ(4294967295, FXSYS_atoui("-4294967345")); } -- cgit v1.2.3