New new versions of Starts/EndsWith and SplitString in net

net/base/ip_pattern.cc

Index: net/base/ip_pattern.cc
diff --git a/net/base/ip_pattern.cc b/net/base/ip_pattern.cc
index 941dbf1a7e594851cf62bd32186f60170104b298..61c8c2e4df886888ac8c6a5e5adf6e5b2b3d829c 100644
--- a/net/base/ip_pattern.cc
+++ b/net/base/ip_pattern.cc
@@ -92,44 +92,45 @@ bool IPPattern::ParsePattern(const std::string& ip_pattern) {
   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);
@@ -142,18 +143,16 @@ 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()) {