From 9778206732296e949ef22a79362c6bce34a56052 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Fri, 7 Aug 2015 15:28:21 -0700 Subject: Fix FXSYS_itoa() implementation. I thought about removing it, but decided to fix it instead until c++11 hits and there may be better alternatives. Remove unused variants. BUG=517854 R=brucedawson@chromium.org, thestig@chromium.org Review URL: https://codereview.chromium.org/1274423003 . --- BUILD.gn | 1 + core/include/fxcrt/fx_system.h | 1 - core/src/fxcrt/fx_basic_gcc.cpp | 38 ++++---- core/src/fxcrt/fx_system_unittest.cpp | 158 ++++++++++++++++++++++++++++++++++ pdfium.gyp | 1 + 5 files changed, 183 insertions(+), 16 deletions(-) create mode 100644 core/src/fxcrt/fx_system_unittest.cpp diff --git a/BUILD.gn b/BUILD.gn index 6581943163..4741941684 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -732,6 +732,7 @@ test("pdfium_unittests") { "core/src/fxcrt/fx_basic_bstring_unittest.cpp", "core/src/fxcrt/fx_basic_memmgr_unittest.cpp", "core/src/fxcrt/fx_basic_wstring_unittest.cpp", + "core/src/fxcrt/fx_system_unittest.cpp", "testing/fx_string_testhelpers.cpp", "testing/fx_string_testhelpers.h", ] diff --git a/core/include/fxcrt/fx_system.h b/core/include/fxcrt/fx_system.h index f0d403501c..ad63d56829 100644 --- a/core/include/fxcrt/fx_system.h +++ b/core/include/fxcrt/fx_system.h @@ -268,7 +268,6 @@ int32_t FXSYS_wtoi(const FX_WCHAR* str); int64_t FXSYS_atoi64(const FX_CHAR* str); int64_t FXSYS_wtoi64(const FX_WCHAR* str); const FX_CHAR* FXSYS_i64toa(int64_t value, FX_CHAR* str, int radix); -const FX_WCHAR* FXSYS_i64tow(int64_t value, FX_WCHAR* str, int radix); int FXSYS_round(FX_FLOAT f); #define FXSYS_Mul(a, b) ((a) * (b)) #define FXSYS_Div(a, b) ((a) / (b)) diff --git a/core/src/fxcrt/fx_basic_gcc.cpp b/core/src/fxcrt/fx_basic_gcc.cpp index bf3d61524e..6f17482156 100644 --- a/core/src/fxcrt/fx_basic_gcc.cpp +++ b/core/src/fxcrt/fx_basic_gcc.cpp @@ -32,30 +32,41 @@ T FXSYS_StrToInt(STR_T str) { } return neg ? -num : num; } -template + +template STR_T FXSYS_IntToStr(T value, STR_T string, int radix) { - int i = 0; - if (value < 0) { - string[i++] = '-'; - value = -value; - } else if (value == 0) { + if (radix < 2 || radix > 16) { + string[0] = 0; + return string; + } + if (value == 0) { string[0] = '0'; string[1] = 0; return string; } + int i = 0; + UT uvalue; + if (value < 0) { + string[i++] = '-'; + // Standard trick to avoid undefined behaviour when negating INT_MIN. + uvalue = static_cast(-(value + 1)) + 1; + } else { + uvalue = value; + } int digits = 1; - T order = value / 10; + T order = uvalue / radix; while (order > 0) { digits++; - order = order / 10; + order = order / radix; } for (int d = digits - 1; d > -1; d--) { - string[d + i] = "0123456789abcdef"[value % 10]; - value /= 10; + string[d + i] = "0123456789abcdef"[uvalue % radix]; + uvalue /= radix; } string[digits + i] = 0; return string; } + #ifdef __cplusplus extern "C" { #endif @@ -72,10 +83,7 @@ int64_t FXSYS_wtoi64(const FX_WCHAR* str) { return FXSYS_StrToInt(str); } const FX_CHAR* FXSYS_i64toa(int64_t value, FX_CHAR* str, int radix) { - return FXSYS_IntToStr(value, str, radix); -} -const FX_WCHAR* FXSYS_i64tow(int64_t value, FX_WCHAR* str, int radix) { - return FXSYS_IntToStr(value, str, radix); + return FXSYS_IntToStr(value, str, radix); } #ifdef __cplusplus } @@ -182,7 +190,7 @@ int FXSYS_wcsicmp(const FX_WCHAR* dst, const FX_WCHAR* src) { return (f - l); } char* FXSYS_itoa(int value, char* string, int radix) { - return FXSYS_IntToStr(value, string, radix); + return FXSYS_IntToStr(value, string, radix); } #ifdef __cplusplus } diff --git a/core/src/fxcrt/fx_system_unittest.cpp b/core/src/fxcrt/fx_system_unittest.cpp new file mode 100644 index 0000000000..ae1e41c77e --- /dev/null +++ b/core/src/fxcrt/fx_system_unittest.cpp @@ -0,0 +1,158 @@ +// Copyright 2015 PDFium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include + +#include "testing/gtest/include/gtest/gtest.h" +#include "../../../testing/fx_string_testhelpers.h" +#include "../../include/fxcrt/fx_system.h" + +// Unit test covering cases where PDFium replaces well-known library +// functionality on any given platformn. +namespace { + +const FX_CHAR kSentinel = 0x7f; + +void Check32BitBase16Itoa(int32_t input, const char* expected_output) { + const size_t kBufLen = 11; // "-" + 8 digits + NUL + sentinel. + FX_CHAR buf[kBufLen]; + buf[kBufLen - 1] = kSentinel; + FXSYS_itoa(input, buf, 16); + EXPECT_EQ(std::string(expected_output), buf); + EXPECT_EQ(kSentinel, buf[kBufLen - 1]); +} + +void Check32BitBase10Itoa(int32_t input, const char* expected_output) { + const size_t kBufLen = 13; // "-" + 10 digits + NUL + sentinel. + FX_CHAR buf[kBufLen]; + buf[kBufLen - 1] = kSentinel; + FXSYS_itoa(input, buf, 10); + EXPECT_EQ(std::string(expected_output), buf); + EXPECT_EQ(kSentinel, buf[kBufLen - 1]); +} + +void Check32BitBase2Itoa(int32_t input, const char* expected_output) { + const size_t kBufLen = 35; // "-" + 32 digits + NUL + sentinel. + FX_CHAR buf[kBufLen]; + buf[kBufLen - 1] = kSentinel; + FXSYS_itoa(input, buf, 2); + EXPECT_EQ(std::string(expected_output), buf); + EXPECT_EQ(kSentinel, buf[kBufLen - 1]); +} + +void Check64BitBase16Itoa(int64_t input, const char* expected_output) { + const size_t kBufLen = 19; // "-" + 16 digits + NUL + sentinel. + FX_CHAR buf[kBufLen]; + buf[kBufLen - 1] = kSentinel; + FXSYS_i64toa(input, buf, 16); + EXPECT_EQ(std::string(expected_output), buf); + EXPECT_EQ(kSentinel, buf[kBufLen - 1]); +} + +void Check64BitBase10Itoa(int64_t input, const char* expected_output) { + const size_t kBufLen = 22; // "-" + 19 digits + NUL + sentinel. + FX_CHAR buf[kBufLen]; + buf[kBufLen - 1] = kSentinel; + FXSYS_i64toa(input, buf, 10); + EXPECT_EQ(std::string(expected_output), buf); + EXPECT_EQ(kSentinel, buf[kBufLen - 1]); +} + +void Check64BitBase2Itoa(int64_t input, const char* expected_output) { + const size_t kBufLen = 67; // "-" + 64 digits + NUL + sentinel. + FX_CHAR buf[kBufLen]; + buf[kBufLen - 1] = kSentinel; + FXSYS_i64toa(input, buf, 2); + EXPECT_EQ(std::string(expected_output), buf); + EXPECT_EQ(kSentinel, buf[kBufLen - 1]); +} + +} // namespace + +TEST(fxcrt, FXSYS_itoa_InvalidRadix) { + FX_CHAR buf[32]; + + FXSYS_itoa(42, buf, 17); + EXPECT_EQ(std::string(""), buf); + + FXSYS_itoa(42, buf, 1); + EXPECT_EQ(std::string(""), buf); + + FXSYS_itoa(42, buf, 0); + EXPECT_EQ(std::string(""), buf); + + FXSYS_itoa(42, buf, -1); + EXPECT_EQ(std::string(""), buf); +} + +TEST(fxcrt, FXSYS_itoa) { + Check32BitBase16Itoa(std::numeric_limits::min(), "-80000000"); + Check32BitBase10Itoa(std::numeric_limits::min(), "-2147483648"); + Check32BitBase2Itoa(std::numeric_limits::min(), + "-10000000000000000000000000000000"); + + Check32BitBase16Itoa(-1, "-1"); + Check32BitBase10Itoa(-1, "-1"); + Check32BitBase2Itoa(-1, "-1"); + + Check32BitBase16Itoa(0, "0"); + Check32BitBase10Itoa(0, "0"); + Check32BitBase2Itoa(0, "0"); + + Check32BitBase16Itoa(42, "2a"); + Check32BitBase10Itoa(42, "42"); + Check32BitBase2Itoa(42, "101010"); + + Check32BitBase16Itoa(std::numeric_limits::max(), "7fffffff"); + Check32BitBase10Itoa(std::numeric_limits::max(), "2147483647"); + Check32BitBase2Itoa(std::numeric_limits::max(), + "1111111111111111111111111111111"); +} + + +TEST(fxcrt, FXSYS_i64toa_InvalidRadix) { + FX_CHAR buf[32]; + + FXSYS_i64toa(42, buf, 17); + EXPECT_EQ(std::string(""), buf); + + FXSYS_i64toa(42, buf, 1); + EXPECT_EQ(std::string(""), buf); + + FXSYS_i64toa(42, buf, 0); + EXPECT_EQ(std::string(""), buf); + + FXSYS_i64toa(42, buf, -1); + EXPECT_EQ(std::string(""), buf); +}; + +TEST(fxcrt, FXSYS_i64toa) { + Check64BitBase16Itoa( + std::numeric_limits::min(), "-8000000000000000"); + Check64BitBase10Itoa( + std::numeric_limits::min(), "-9223372036854775808"); + Check64BitBase2Itoa( + std::numeric_limits::min(), + "-1000000000000000000000000000000000000000000000000000000000000000"); + + Check64BitBase16Itoa(-1, "-1"); + Check64BitBase10Itoa(-1, "-1"); + Check64BitBase2Itoa(-1, "-1"); + + Check64BitBase16Itoa(0, "0"); + Check64BitBase10Itoa(0, "0"); + Check64BitBase2Itoa(0, "0"); + + Check64BitBase16Itoa(42, "2a"); + Check64BitBase10Itoa(42, "42"); + Check64BitBase2Itoa(42, "101010"); + + Check64BitBase16Itoa( + std::numeric_limits::max(), "7fffffffffffffff"); + Check64BitBase10Itoa( + std::numeric_limits::max(), "9223372036854775807"); + Check64BitBase2Itoa( + std::numeric_limits::max(), + "111111111111111111111111111111111111111111111111111111111111111"); +} diff --git a/pdfium.gyp b/pdfium.gyp index 16d55d30d8..31f57da1b6 100644 --- a/pdfium.gyp +++ b/pdfium.gyp @@ -719,6 +719,7 @@ 'core/src/fxcrt/fx_basic_bstring_unittest.cpp', 'core/src/fxcrt/fx_basic_memmgr_unittest.cpp', 'core/src/fxcrt/fx_basic_wstring_unittest.cpp', + 'core/src/fxcrt/fx_system_unittest.cpp', 'testing/fx_string_testhelpers.h', 'testing/fx_string_testhelpers.cpp', ], -- cgit v1.2.3