Merge pull request #736 from zyearn/replace_kvpair_splitter

replace SplitStringIntoKeyValuePairs

src/brpc/policy/consistent_hashing_load_balancer.cpp

@@ -270,10 +270,11 @@ size_t ConsistentHashingLoadBalancer::RemoveServersInBatch(
     return n;
 }
 
-LoadBalancer *ConsistentHashingLoadBalancer::New(const butil::StringPiece& params) const {
+LoadBalancer *ConsistentHashingLoadBalancer::New(
+    const butil::StringPiece& params) const {
     ConsistentHashingLoadBalancer* lb = 
         new (std::nothrow) ConsistentHashingLoadBalancer(_type);
-    if (lb != nullptr && !lb->SetParameters(params)) {
+    if (lb && !lb->SetParameters(params)) {
         delete lb;
         lb = nullptr;
     }
@@ -377,20 +378,19 @@ void ConsistentHashingLoadBalancer::GetLoads(
 }
 
 bool ConsistentHashingLoadBalancer::SetParameters(const butil::StringPiece& params) {
-    for (butil::StringSplitter sp(params.begin(), params.end(), ' '); sp != nullptr; ++sp) {
-        butil::StringPiece key_value(sp.field(), sp.length());
-        size_t p = key_value.find('=');
-        if (p == key_value.npos || p == key_value.size() - 1) {
-            // No value configed.
+    for (butil::KeyValuePairsSplitter sp(params.begin(), params.end(), ' ', '=');
+            sp; ++sp) {
+        if (sp.value().empty()) {
+            LOG(ERROR) << "Empty value for " << sp.key() << " in lb parameter";
             return false;
         }
-        if (key_value.substr(0, p) == "replicas") {
-            if (!butil::StringToSizeT(key_value.substr(p + 1), &_num_replicas)) {
+        if (sp.key() == "replicas") {
+            if (!butil::StringToSizeT(sp.value(), &_num_replicas)) {
                 return false;
             }
             continue;
         }
-        LOG(ERROR) << "Failed to set this unknown parameters " << key_value;
+        LOG(ERROR) << "Failed to set this unknown parameters " << sp.key_and_value();
     }
     return true;
 }

src/brpc/uri.h

@@ -198,15 +198,15 @@ inline std::ostream& operator<<(std::ostream& os, const URI& uri) {
 class QuerySplitter : public butil::KeyValuePairsSplitter {
 public:
     inline QuerySplitter(const char* str_begin, const char* str_end)
-        : KeyValuePairsSplitter(str_begin, str_end, '=', '&')
+        : KeyValuePairsSplitter(str_begin, str_end, '&', '=')
     {}
 
     inline QuerySplitter(const char* str_begin)
-        : KeyValuePairsSplitter(str_begin, '=', '&')
+        : KeyValuePairsSplitter(str_begin, '&', '=')
     {}
 
     inline QuerySplitter(const butil::StringPiece &sp)
-        : KeyValuePairsSplitter(sp, '=', '&')
+        : KeyValuePairsSplitter(sp, '&', '=')
     {}
 };
 

src/butil/string_splitter.h

@@ -167,8 +167,8 @@ private:
 
 // Split query in the format according to the given delimiters.
 // This class can also handle some exceptional cases.
-// 1. consecutive key_value_pair_delimiter are omitted, for example,
-//    suppose key_value_delimiter is '=' and key_value_pair_delimiter
+// 1. consecutive pair_delimiter are omitted, for example,
+//    suppose key_value_delimiter is '=' and pair_delimiter
 //    is '&', then 'k1=v1&&&k2=v2' is normalized to 'k1=k2&k2=v2'.
 // 2. key or value can be empty or both can be empty.
 // 3. consecutive key_value_delimiter are not omitted, for example,
@@ -178,25 +178,25 @@ class KeyValuePairsSplitter {
 public:
     inline KeyValuePairsSplitter(const char* str_begin,
                                  const char* str_end,
-                                 char key_value_delimiter,
-                                 char key_value_pair_delimiter)
-        : _sp(str_begin, str_end, key_value_pair_delimiter)
+                                 char pair_delimiter,
+                                 char key_value_delimiter)
+        : _sp(str_begin, str_end, pair_delimiter)
         , _delim_pos(StringPiece::npos)
         , _key_value_delim(key_value_delimiter) {
         UpdateDelimiterPosition();
     }
 
     inline KeyValuePairsSplitter(const char* str_begin,
-                                 char key_value_delimiter,
-                                 char key_value_pair_delimiter)
+                                 char pair_delimiter,
+                                 char key_value_delimiter)
         : KeyValuePairsSplitter(str_begin, NULL,
-                key_value_delimiter, key_value_pair_delimiter) {}
+                pair_delimiter, key_value_delimiter) {}
 
     inline KeyValuePairsSplitter(const StringPiece &sp,
-                                 char key_value_delimiter,
-                                 char key_value_pair_delimiter)
+                                 char pair_delimiter,
+                                 char key_value_delimiter)
         : KeyValuePairsSplitter(sp.begin(), sp.end(),
-                key_value_delimiter, key_value_pair_delimiter) {}
+                pair_delimiter, key_value_delimiter) {}
 
     inline StringPiece key() {
         return key_and_value().substr(0, _delim_pos);

src/bvar/variable.cpp

@@ -24,7 +24,6 @@
 #include "butil/containers/flat_map.h"           // butil::FlatMap
 #include "butil/scoped_lock.h"                   // BAIDU_SCOPE_LOCK
 #include "butil/string_splitter.h"               // butil::StringSplitter
-#include "butil/strings/string_split.h"          // butil::SplitStringIntoKeyValuePairs
 #include "butil/errno.h"                         // berror
 #include "butil/time.h"                          // milliseconds_from_now
 #include "butil/file_util.h"                     // butil::FilePath
@@ -627,15 +626,13 @@ public:
             // .data will be appended later
             path = path.RemoveFinalExtension();
         }
-        butil::StringPairs pairs;
-        pairs.reserve(8);
-        butil::SplitStringIntoKeyValuePairs(tabs, '=', ';', &pairs);
-        dumpers.reserve(pairs.size() + 1);
-        //matchers.reserve(pairs.size());
-        for (size_t i = 0; i < pairs.size(); ++i) {
+
+        for (butil::KeyValuePairsSplitter sp(tabs, ';', '='); sp; ++sp) {
+            std::string key = sp.key().as_string();
+            std::string value = sp.value().as_string();
             FileDumper *f = new FileDumper(
-                    path.AddExtension(pairs[i].first).AddExtension("data").value(), s);
-            WildcardMatcher *m = new WildcardMatcher(pairs[i].second, '?', true);
+                    path.AddExtension(key).AddExtension("data").value(), s);
+            WildcardMatcher *m = new WildcardMatcher(value, '?', true);
             dumpers.push_back(std::make_pair(f, m));
         }
         dumpers.push_back(std::make_pair(

test/string_splitter_unittest.cpp

@@ -343,12 +343,12 @@ TEST_F(StringSplitterTest, key_value_pairs_splitter_sanity) {
         // Test three constructors
         butil::KeyValuePairsSplitter* psplitter = NULL;
         if (i == 0) {
-            psplitter = new butil::KeyValuePairsSplitter(kvstr, '=', '&');
+            psplitter = new butil::KeyValuePairsSplitter(kvstr, '&', '=');
         } else if (i == 1) {
             psplitter = new butil::KeyValuePairsSplitter(
-                    kvstr.data(), kvstr.data() + kvstr.size(), '=', '&');
+                    kvstr.data(), kvstr.data() + kvstr.size(), '&', '=');
         } else if (i == 2) {
-            psplitter = new butil::KeyValuePairsSplitter(kvstr.c_str(), '=', '&');
+            psplitter = new butil::KeyValuePairsSplitter(kvstr.c_str(), '&', '=');
         }
         butil::KeyValuePairsSplitter& splitter = *psplitter;