New new versions of Starts/EndsWith and SplitString in net

net/base/ip_pattern.cc

 // Copyright 2014 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/base/ip_pattern.h"
 
 #include <string>
 
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 74 matching lines @@
     ++pattern_it;
   }
   return true;
 }
 
 bool IPPattern::ParsePattern(const std::string& ip_pattern) {
   DCHECK(ip_mask_.empty());
   if (ip_pattern.find(':') != std::string::npos) {
     is_ipv4_ = false;
   }
-  Strings components;
-  base::SplitString(ip_pattern, is_ipv4_ ? '.' : ':', &components);
+
+  std::vector<base::StringPiece> components =
+      base::SplitStringPiece(ip_pattern, is_ipv4_ ? "." : ":",
+                             base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
   if (components.size() != (is_ipv4_ ? 4u : 8u)) {
     DVLOG(1) << "Invalid component count: " << ip_pattern;
     return false;
   }
-  for (Strings::iterator component_it = components.begin();
-       component_it != components.end(); ++component_it) {
-    if (component_it->empty()) {
+  for (base::StringPiece component : components) {

[Ryan Sleevi, 2015/07/06 08:52:01]

EXTREME PEDANTRY:

Wouldn't
for (const auto& component : components) {
or
for (const base::StringPiece& component : components) {

make more sense here? So that it can bind to the const form in compilers that
implemented the C++14 optimization in C++11 mode?

[The debate about whether tis better to copy a base::StringPiece vs refer to it
by reference seems unclear; I've seen arguments about performance go both ways,
and Google allows both, but Chromium tends to prefer the by-ref rather than
treating StringPiece as POD]

[You also tend to prefer the latter elsewhere in this CL]

[brettw, 2015/07/06 16:52:34]

This particular loop is weird because it actually modifies the loop variable
(!). This is why the previous iterator wasn't const.

My opinion is that const& is better for loops for StringPiece, but I often pass
by value to functions. This makes some intuitive sense: the compiler can't
normally optimize across function boundaries, and putting the two words on the
stack next to the rest of the parameters is faster than putting a pointer and
then dereferencing it twice to an area the function isn't touching otherwise. In
this case, this compiler has full visibility to optimize dereferences and the
vector variables are sure to be in L1 cache anyway.

+    if (component.empty()) {
       DVLOG(1) << "Empty component: " << ip_pattern;
       return false;
     }
-    if (*component_it == "*") {
+    if (component == "*") {
       // Let standard code handle this below.
-      *component_it = is_ipv4_ ? "[0-255]" : "[0-FFFF]";
-    } else if ((*component_it)[0] != '[') {
+      component = is_ipv4_ ? "[0-255]" : "[0-FFFF]";
+    } else if (component[0] != '[') {
       // This value will just have a specific integer to match.
       uint32_t value;
-      if (!ValueTextToInt(*component_it, &value))
+      if (!ValueTextToInt(component, &value))
         return false;
       ip_mask_.push_back(true);
       component_values_.push_back(value);
       continue;
     }
-    if ((*component_it)[component_it->size() - 1] != ']') {
+    if (component[component.size() - 1] != ']') {
       DVLOG(1) << "Missing close bracket: " << ip_pattern;
       return false;
     }
     // Now we know the size() is at least 2.
-    if (component_it->size() == 2) {
+    if (component.size() == 2) {
       DVLOG(1) << "Empty bracket: " << ip_pattern;
       return false;
     }
     // We'll need a pattern to match this bracketed component.
     scoped_ptr<ComponentPattern> component_pattern(new ComponentPattern);
     // Trim leading and trailing bracket before calling for parsing.
-    if (!ParseComponentPattern(base::StringPiece(component_it->data() + 1,
-        component_it->size() - 2), component_pattern.get())) {
+    if (!ParseComponentPattern(component.substr(1, component.size() - 2),
+                               component_pattern.get())) {
       return false;
     }
     ip_mask_.push_back(false);
     component_patterns_.push_back(component_pattern.release());
   }
   return true;
 }
 
 bool IPPattern::ParseComponentPattern(const base::StringPiece& text,
                                       ComponentPattern* pattern) const {
   // We're given a comma separated set of ranges, some of which may be simple
   // constants.
-  Strings ranges;
-  base::SplitString(text.as_string(), ',', &ranges);
-  for (Strings::iterator range_it = ranges.begin();
-        range_it != ranges.end(); ++range_it) {
-    base::StringTokenizer range_pair(*range_it, "-");
+  for (const std::string& range : base::SplitString(
+           text, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL)) {
+    base::StringTokenizer range_pair(range, "-");
     uint32_t min = 0;
     range_pair.GetNext();
-    if (!ValueTextToInt(range_pair.token(), &min))
+    if (!ValueTextToInt(range_pair.token_piece(), &min))
       return false;
     uint32_t max = min;  // Sometimes we have no distinct max.
     if (range_pair.GetNext()) {
-      if (!ValueTextToInt(range_pair.token(), &max))
+      if (!ValueTextToInt(range_pair.token_piece(), &max))
         return false;
     }
     if (range_pair.GetNext()) {
       // Too many "-" in this range specifier.
       DVLOG(1) << "Too many hyphens in range: ";
       return false;
     }
     pattern->AppendRange(min, max);
   }
   return true;
@@ skipping 12 matching lines @@
     return false;
   }
   if (!is_ipv4_ && *output > 0xFFFFu) {
     DVLOG(1) << "IPv6 component greater than 0xFFFF";
     return false;
   }
   return ok;
 }
 
 }  // namespace net