Fix push_back() to shared fbstring bug.

Summary:
The following asserts:
fbstring str(1337, 'f');
fbstring cp = str;
cp.push_back('f');

This is problematic since ml_.capacity() != capacity() inside fbstring_core
for shared strings, which causes us not to un-share prior to push_back.

Test Plan: Existing tests, add unittest case.

Reviewed By: tudorb@fb.com

FB internal diff: D580267

diff --git a/folly/FBString.h b/folly/FBString.h
index 9280bf8b814e262b5646883b275fc41d55832eb5..9ac8918ce7e22d14af08bc79ebeb7f8650fb1740 100644 (file)
--- a/folly/FBString.h
+++ b/folly/FBString.h
@@ -626,7 +626,7 @@ public:
     } else {
       sz = ml_.size_;
       newSz = ml_.size_ + delta;
-      if (newSz > ml_.capacity()) {
+      if (newSz > capacity()) {
         reserve(newSz);
       }
     }
@@ -653,7 +653,7 @@ public:
       reserve(maxSmallSize * 2);
     } else {
       sz = ml_.size_;
-      cp = ml_.capacity();
+      cp = capacity();  // != ml_.capacity() for isShared()
       if (sz == cp) reserve(cp * 3 / 2);
     }
     assert(capacity() >= sz + 1);

diff --git a/folly/test/FBStringTest.cpp b/folly/test/FBStringTest.cpp
index 40dc4359722d5abf862adecfb1c67152bbbe599e..428bed28fd33d6e5d55493b918d43ba1375711a7 100644 (file)
--- a/folly/test/FBStringTest.cpp
+++ b/folly/test/FBStringTest.cpp
@@ -981,6 +981,18 @@ TEST(FBString, testFixedBugs) {
       cp.push_back('?');
     }
   }
+  { // D580267
+    {
+      fbstring str(1337, 'f');
+      fbstring cp = str;
+      cp.push_back('f');
+    }
+    {
+      fbstring str(1337, 'f');
+      fbstring cp = str;
+      cp += "bb";
+    }
+  }
 }
 
 int main(int argc, char** argv) {