From: Giuseppe Ottaviano <ott@fb.com>
Date: Fri, 8 Jan 2016 01:34:42 +0000 (-0800)
Subject: Fix two fbstring operator+ overloads
X-Git-Tag: deprecate-dynamic-initializer~164
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=81295897a3f7ae5b15195ede2a531226472a9ecc;p=folly.git

Fix two fbstring operator+ overloads

Summary: `insert()` returns `fbstring` in most cases, but `iterator` (that is, `value_type*`) when the first argument is an iterator. Two overloads of `operator+` used `insert` as if it returned `fbstring`, which by chance works anyway unless the resulting string contains a `'\0'` (plus it does an extra string copy). This diff fixes the bug.

Reviewed By: philippv, luciang, Gownta

Differential Revision: D2813713

fb-gh-sync-id: 015188b72813da2dabe23980f50f00832d62aa14
---

diff --git a/folly/FBString.h b/folly/FBString.h
index 06b8b86e..d26b961d 100644
--- a/folly/FBString.h
+++ b/folly/FBString.h
@@ -2121,7 +2121,8 @@ basic_fbstring<E, T, A, S> operator+(
   const auto len = basic_fbstring<E, T, A, S>::traits_type::length(lhs);
   if (rhs.capacity() >= len + rhs.size()) {
     // Good, at least we don't need to reallocate
-    return std::move(rhs.insert(rhs.begin(), lhs, lhs + len));
+    rhs.insert(rhs.begin(), lhs, lhs + len);
+    return rhs;
   }
   // Meh, no go. Do it by hand since we have len already.
   basic_fbstring<E, T, A, S> result;
@@ -2153,7 +2154,8 @@ basic_fbstring<E, T, A, S> operator+(
   //
   if (rhs.capacity() > rhs.size()) {
     // Good, at least we don't need to reallocate
-    return std::move(rhs.insert(rhs.begin(), lhs));
+    rhs.insert(rhs.begin(), lhs);
+    return rhs;
   }
   // Meh, no go. Forward to operator+(E, const&).
   auto const& rhsC = rhs;
diff --git a/folly/test/FBStringTest.cpp b/folly/test/FBStringTest.cpp
index 15783ea1..94bbac99 100644
--- a/folly/test/FBStringTest.cpp
+++ b/folly/test/FBStringTest.cpp
@@ -1259,6 +1259,17 @@ TEST(FBString, testFixedBugs) {
     copy.push_back('b');
     EXPECT_GE(copy.capacity(), 1);
   }
+  { // D2813713
+    fbstring s1("a");
+    s1.reserve(8); // Trigger the optimized code path.
+    auto test1 = '\0' + std::move(s1);
+    EXPECT_EQ(2, test1.size());
+
+    fbstring s2(1, '\0');
+    s2.reserve(8);
+    auto test2 = "a" + std::move(s2);
+    EXPECT_EQ(2, test2.size());
+  }
 }
 
 TEST(FBString, findWithNpos) {