Fix FXSYS_StrToInt()

core/fxcrt/fx_basic_gcc.cpp

Index: core/fxcrt/fx_basic_gcc.cpp
diff --git a/core/fxcrt/fx_basic_gcc.cpp b/core/fxcrt/fx_basic_gcc.cpp
index 2c9c54c8b60bd26cf6d6772517c6ec2f7df007e9..91aa689cd0ec8844001a3d0bc55d7f45f7e3cf9c 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<IntType>::is_signed && *str == '-';
-  if (neg)
+  // Process the sign.
+  bool neg = *str == '-';

[dsinclair, 2016/03/24 02:41:06]

This loses the std::numeric_limits<IntType>::is_signed which we need below to
determine if we can negate or not. Otherwise, calling with a uint8_t and passing
-20 would do the wrong thing.

[Wei Li, 2016/03/24 17:31:25]

I think eventually we may want to use std:strto? to replace all these functions.
It is ok if we would want to use a custom function to treat all negative values
for unsigned values as 0, but the common way is to preserve the binary
representation so that it would the same value after converting to an integer.

For pdfium, str to unsigned are exclusively used for reading object nums and
generation numbers, both of them are non-negative. 

Ditto for all the following comments.

[dsinclair, 2016/03/24 18:10:11]

Fair enough, think the conditional on 28 can still be simplifed a bit:

if (neg && std::numeric_limits<IntType>::is_signed) {
  return std::numeric_limits<IntType>::min();
else
  return std::numeric_limits<IntType>::max();

[Wei Li, 2016/03/24 23:25:46]

Done.

+  if (neg || *str == '+')
     str++;
 
   IntType num = 0;
   while (*str && FXSYS_isDecimalDigit(*str)) {
     IntType val = FXSYS_toDecimalDigit(*str);
-    if (num > (std::numeric_limits<IntType>::max() - val) / 10)
-      break;
+    if (num > (std::numeric_limits<IntType>::max() - val) / 10) {
+      if (std::numeric_limits<IntType>::is_signed) {

[dsinclair, 2016/03/24 01:40:44]

I didn't make this change originally because I wasn't sure if the code currently
depended on the weird truncation behaviour.

[Wei Li, 2016/03/24 02:27:42]

I checked callers which mainly use this to process fields or offsets of pdf
file. That is why this has not causing any problem so far.

+        // Return MAX or MIN when the represented number is larger than the max
+        // value or smaller than the min value.
+        return neg ? std::numeric_limits<IntType>::min()
+                   : std::numeric_limits<IntType>::max();
+      } else {
+        // Return MAX when the represented number is out of range.
+        return std::numeric_limits<IntType>::max();

[dsinclair, 2016/03/24 02:41:06]

If you put the is_signed check back at line 20 this if {} else can go away and
just have the return on 31.

+      }
+    }
 
     num = num * 10 + val;
     str++;