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

core/src/fxcrt/fx_basic_util.cpp

 // Copyright 2014 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.
 
 // 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();
 }
 void FX_PRIVATEDATA::FreeData() {
   if (m_pData == NULL) {
     return;
   }
   if (m_bSelfDestruct) {
     delete (CFX_DestructObject*)m_pData;
   } else if (m_pCallback) {
@@ skipping 69 matching lines @@
     int cc = 0, integer = 0;
     const FX_CHAR* str = strc.GetCStr();
     int len = strc.GetLength();
     FX_BOOL bNegative = FALSE;
     if (str[0] == '+') {
       cc++;
     } else if (str[0] == '-') {
       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) {
       integer = -integer;
     }
     *(int*)pData = integer;
   } else {
     bInteger = FALSE;
     *(FX_FLOAT*)pData = FX_atof(strc);
   }
@@ skipping 16 matching lines @@
     if (str[cc] != '+' && str[cc] != '-') {
       break;
     }
     cc++;
   }
   FX_FLOAT value = 0;
   while (cc < len) {
     if (str[cc] == '.') {
       break;
     }
-    value = value * 10 + str[cc] - '0';
+    value = value * 10 + FXSYS_toDecimalDigit(str[cc]);
     cc++;
   }
   static const FX_FLOAT fraction_scales[] = {
       0.1f,         0.01f,         0.001f,        0.0001f,
       0.00001f,     0.000001f,     0.0000001f,    0.00000001f,
       0.000000001f, 0.0000000001f, 0.00000000001f};
   int scale = 0;
   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;
       }
       cc++;
     }
   }
   return bNegative ? -value : value;
 }
 
@@ skipping 195 matching lines @@
       d * m.b + e * m.e + f * m.h, d * m.c + e * m.f + f * m.i,
       g * m.a + h * m.d + i * m.g, g * m.b + h * m.e + i * m.h,
       g * m.c + h * m.f + i * m.i);
 }
 
 CFX_Vector_3by1 CFX_Matrix_3by3::TransformVector(const CFX_Vector_3by1& v) {
   return CFX_Vector_3by1(a * v.a + b * v.b + c * v.c,
                          d * v.a + e * v.b + f * v.c,
                          g * v.a + h * v.b + i * v.c);
 }