commit 1e7df19a137f290064bc098e9ccbf82232197285
author Lei Zhang <thestig@chromium.org> Fri Sep 04 02:12:42 2020 +0000
committer Chromium commit bot <commit-bot@chromium.org> Fri Sep 04 02:12:42 2020 +0000
tree 5c1222d3393258a199dfcf326f163ae4c40d455f
parent b01d300985266439e7112b03201d49dddd8f3ee5

Revert "Optimize a tight loop in FX_Number."

This reverts commit 67fd772899491dbe32306dfec48a5bbb005feb9b.

Reason for revert: Regressed PDF loading. See bug for details.

Original change's description:
> Optimize a tight loop in FX_Number.
> 
> While doing string to int conversion:
> 1) Remove a redundant std::isdigit() call.
> 2) Don't call CheckedNumeric::IsValid() in this case.
> - It's almost always valid.
> - Continuing in an invalid state does not cause any security issues.
> 
> Change-Id: I4473b1ec020cd56e241276c21e35c25b5af5a144
> Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/72250
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Commit-Queue: Lei Zhang <thestig@chromium.org>

Bug: chromium:1124998
TBR=tsepez@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: Ie616692ddbac3a2313c12a2563274ad19c7813c5
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/73336
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>

core/fxcrt/fx_number.cpp

diff --git a/core/fxcrt/fx_number.cpp b/core/fxcrt/fx_number.cpp
index 99e9f41..178ede9 100644
--- a/core/fxcrt/fx_number.cpp
+++ b/core/fxcrt/fx_number.cpp
@@ -50,10 +50,11 @@
     cc++;
   }
 
-  for (; cc < strc.GetLength() && std::isdigit(strc[cc]); ++cc) {
-    // Deliberately not using FXSYS_DecimalCharToInt() in a tight loop to avoid
-    // a duplicate std::isdigit() call.
-    unsigned_val = unsigned_val * 10 + strc[cc] - '0';
+  while (cc < strc.GetLength() && std::isdigit(strc[cc])) {
+    unsigned_val = unsigned_val * 10 + FXSYS_DecimalCharToInt(strc.CharAt(cc));
+    if (!unsigned_val.IsValid())
+      break;
+    cc++;
   }
 
   uint32_t uValue = unsigned_val.ValueOrDefault(0);