Revert "Revert "Revert "Revert "Cleanup some numeric code.""""

core/src/fxcrt/fx_basic_util.cpp

Index: core/src/fxcrt/fx_basic_util.cpp
diff --git a/core/src/fxcrt/fx_basic_util.cpp b/core/src/fxcrt/fx_basic_util.cpp
index 46a0dec1e538884370b92f7865aa84d1fde13722..78bdeee6939fbbe3006942cc3fe156c38d788dac 100644
--- a/core/src/fxcrt/fx_basic_util.cpp
+++ b/core/src/fxcrt/fx_basic_util.cpp
@@ -5,12 +5,17 @@
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "../../include/fxcrt/fx_basic.h"
+#include "../../include/fxcrt/fx_ext.h"
+
 #if _FXM_PLATFORM_ != _FXM_PLATFORM_WINDOWS_
 #include <sys/types.h>
 #include <dirent.h>
 #else
 #include <direct.h>

[dsinclair, 2015/11/04 20:16:24]

Should these also be moved up?

 #endif
+
+#include <cctype>

[Tom Sepez, 2015/11/04 18:42:46]

nit: this goes at line 6.

[dsinclair, 2015/11/04 20:16:24]

Done.

+
 CFX_PrivateData::~CFX_PrivateData() {
   ClearAll();
 }
@@ -100,14 +105,11 @@ void FX_atonum(const CFX_ByteStringC& strc, FX_BOOL& bInteger, void* pData) {
       bNegative = TRUE;
       cc++;
     }
-    while (cc < len) {
-      if (str[cc] < '0' || str[cc] > '9') {
+    while (cc < len && std::isdigit(str[cc])) {
+      integer = integer * 10 + FXSYS_toDecimalDigit(str[cc]);
+      if (integer < 0)

[Tom Sepez, 2015/11/04 18:42:46]

argh. You can't detect overflow this way. Signed int overflow is undefined
behaviour and the compiler will aggressively remove this check because it knows
that undefined behaviour can't happen.

If we really want this, then we first have to check if integer >
std::numeric_limit<int> / 10 before the multiply and that if integer >
std::numeric_limit<int> - 9 before the add.  There's a way to combine these if
you're clever.

[dsinclair, 2015/11/04 20:16:24]

I'm assuming you meant std::numeric_limits<int>::max() ?


Kept the checks, went with non-clever. I don't know why it checks for overflow
here, but I'm going to assume it's needed until we can prove otherwise. Added a
TODO.

         break;
-      }
-      integer = integer * 10 + str[cc] - '0';
-      if (integer < 0) {
-        break;
-      }
+
       cc++;
     }
     if (bNegative) {
@@ -144,7 +146,7 @@ FX_FLOAT FX_atof(const CFX_ByteStringC& strc) {
     if (str[cc] == '.') {
       break;
     }
-    value = value * 10 + str[cc] - '0';
+    value = value * 10 + FXSYS_toDecimalDigit(str[cc]);
     cc++;
   }
   static const FX_FLOAT fraction_scales[] = {
@@ -155,7 +157,7 @@ FX_FLOAT FX_atof(const CFX_ByteStringC& strc) {
   if (cc < len && str[cc] == '.') {
     cc++;
     while (cc < len) {
-      value += fraction_scales[scale] * (str[cc] - '0');
+      value += fraction_scales[scale] * FXSYS_toDecimalDigit(str[cc]);
       scale++;
       if (scale == sizeof fraction_scales / sizeof(FX_FLOAT)) {
         break;