Fix FXSYS_StrToInt()

core/fxcrt/fx_basic_gcc.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 <cctype>
 #include <cwctype>
 #include <limits>
 
 #include "core/include/fxcrt/fx_ext.h"
 #include "core/include/fxcrt/fx_string.h"
 
 template <typename IntType, typename CharType>
 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++;
   }
   return neg ? -num : num;
 }
 
 template <typename T, typename UT, typename STR_T>
 STR_T FXSYS_IntToStr(T value, STR_T str, int radix) {
   if (radix < 2 || radix > 16) {
@@ skipping 195 matching lines @@
       buf[wlen] = bstr[i];
     }
     wlen++;
   }
   return wlen;
 }
 #ifdef __cplusplus
 }
 #endif
 #endif