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 .

5 files changed, 183 insertions, 16 deletions

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 <typename T, typename STR_T>
+
+template <typename T, typename UT, typename STR_T>
 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<UT>(-(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<int64_t, const FX_WCHAR*>(str);
 }
 const FX_CHAR* FXSYS_i64toa(int64_t value, FX_CHAR* str, int radix) {
-  return FXSYS_IntToStr<int64_t, FX_CHAR*>(value, str, radix);
-}
-const FX_WCHAR* FXSYS_i64tow(int64_t value, FX_WCHAR* str, int radix) {
-  return FXSYS_IntToStr<int64_t, FX_WCHAR*>(value, str, radix);
+  return FXSYS_IntToStr<int64_t, uint64_t, FX_CHAR*>(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<int32_t, FX_CHAR*>(value, string, radix);
+  return FXSYS_IntToStr<int32_t, uint32_t, FX_CHAR*>(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 <string>
+
+#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<int32_t>::min(), "-80000000");
+  Check32BitBase10Itoa(std::numeric_limits<int32_t>::min(), "-2147483648");
+  Check32BitBase2Itoa(std::numeric_limits<int32_t>::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<int32_t>::max(), "7fffffff");
+  Check32BitBase10Itoa(std::numeric_limits<int32_t>::max(), "2147483647");
+  Check32BitBase2Itoa(std::numeric_limits<int32_t>::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<int64_t>::min(), "-8000000000000000");
+  Check64BitBase10Itoa(
+      std::numeric_limits<int64_t>::min(), "-9223372036854775808");
+  Check64BitBase2Itoa(
+      std::numeric_limits<int64_t>::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<int64_t>::max(), "7fffffffffffffff");
+  Check64BitBase10Itoa(
+      std::numeric_limits<int64_t>::max(), "9223372036854775807");
+  Check64BitBase2Itoa(
+      std::numeric_limits<int64_t>::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',
       ],