Remove some uses of isdigit in net/, as it can be locale depedent.

net/spdy/spdy_alt_svc_wire_format.cc

 // Copyright (c) 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/spdy/spdy_alt_svc_wire_format.h"
 
 #include <algorithm>
 #include <limits>
 #include <string>
 
 #include "base/logging.h"
+#include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 
 namespace net {
 
 using base::StringPiece;
 
 namespace {
 
 template <class T>
 bool ParsePositiveIntegerImpl(StringPiece::const_iterator c,
                               StringPiece::const_iterator end,
                               T* value) {
   *value = 0;
-  for (; c != end && isdigit(*c); ++c) {
+  // TODO(mmenke):  This really should be using methods in parse_number.h.
+  for (; c != end && '0' <= *c && *c <= '9'; ++c) {
     if (*value > std::numeric_limits<T>::max() / 10) {
       return false;
     }
     *value *= 10;
     if (*value > std::numeric_limits<T>::max() - (*c - '0')) {
       return false;
     }
     *value += *c - '0';
   }
   return (c == end && *value > 0);
@@ skipping 223 matching lines @@
 }
 
 // static
 void SpdyAltSvcWireFormat::SkipWhiteSpace(StringPiece::const_iterator* c,
                                           StringPiece::const_iterator end) {
   for (; *c != end && (**c == ' ' || **c == '\t'); ++*c) {
   }
 }
 
 // static
 bool SpdyAltSvcWireFormat::PercentDecode(StringPiece::const_iterator c,

[eroman, 2016/12/07 18:40:11]

Hmm, weird.
When I think percent escaping I think URL style (with decimal digits).

[mmenke, 2016/12/07 19:02:50]

decimal digits?  I'm not following.  URLs use hex escaping, too.

[eroman, 2016/12/07 19:11:53]

My mistake, realized right after sending :)

I assumed there was a difference or this would be using a shared url-unescaping
routine.
But perhaps the only difference is lack of support for wide strings?

                                          StringPiece::const_iterator end,
                                          std::string* output) {
   output->clear();
   for (; c != end; ++c) {
     if (*c != '%') {
       output->push_back(*c);
       continue;
     }
     DCHECK_EQ('%', *c);
     ++c;
-    if (c == end || !isxdigit(*c)) {
+    if (c == end || !base::IsHexDigit(*c)) {
       return false;
     }
-    char decoded = tolower(*c);
-    // '0' is 0, 'a' is 10.
-    decoded += isdigit(*c) ? (0 - '0') : (10 - 'a');
     // Network byte order is big-endian.
-    decoded <<= 4;
+    char decoded = base::HexDigitToInt(*c) << 4;
+
     ++c;
-    if (c == end || !isxdigit(*c)) {
+    if (c == end || !base::IsHexDigit(*c)) {
       return false;
     }
-    decoded += tolower(*c);
-    // '0' is 0, 'a' is 10.
-    decoded += isdigit(*c) ? (0 - '0') : (10 - 'a');
+    // Network byte order is big-endian.
+    decoded += base::HexDigitToInt(*c);

[eroman, 2016/12/07 18:40:11]

Hmm.

Signed overflow is undefined, so this addition is potentially undefined behavior
(as is the initialization of decoded).

Not sure why C makes this stuff so unbelievably difficult.

 * char is not guaranteed to be either signed or unsigned
 * the expression (myuint8 << 4) returns an int (SIGNED int), because hey, why
not? (making narrowing and signed/unsigned compilation warnings all the more
noisy)

Probably the most sane thing to do is just make decoded be an |int|.

[mmenke, 2016/12/07 19:02:50]

SGTM, done, though still have to cast the int to char.

+
     output->push_back(decoded);
   }
   return true;
 }
 
 // static
 bool SpdyAltSvcWireFormat::ParseAltAuthority(StringPiece::const_iterator c,
                                              StringPiece::const_iterator end,
                                              std::string* host,
                                              uint16_t* port) {
@@ skipping 44 matching lines @@
     StringPiece::const_iterator end,
     uint16_t* value) {
   return ParsePositiveIntegerImpl<uint16_t>(c, end, value);
 }
 
 // static
 bool SpdyAltSvcWireFormat::ParsePositiveInteger32(
     StringPiece::const_iterator c,
     StringPiece::const_iterator end,
     uint32_t* value) {
   return ParsePositiveIntegerImpl<uint32_t>(c, end, value);

[mmenke, 2016/12/07 16:24:36]

Could use the parse_number method here, but we'd have to create a StringPiece
using the knowledge that StringPiece::const_iterators are just pointers to a C++
character array.

[eroman, 2016/12/07 18:40:11]

Could also do something like this:

StringPiece MakeStringPiece(StringPiece::const_iterator begin,
StringPiece::const_iterator end) {
  auto length = std::distance(begin, end);
  return length <= 0 ? StringPiece() : StringPiece(&(*begin),
static_cast<size_t>(length));
}


....

return ParseUint32(MakeStringPiece(...), value) && value > 0;


And for the 16-bit version I would be fine with adding a 16-bit flavor to
parse_number.h (there are at least 1 or 2 places that could use that too,
dealing with port parsing IIRC).

 }
 
 }  // namespace net