Use copy constructor to push new non-pod element in the vector.

In the push_back method:
Previously we used the assignment operator for both pod and non pod types.

This was broken for non pod since the memory at the end of the vector where
we tried to assign to was not initialized (lhs cw not a valid instance).
Instead we use placement new to copy the element to the new location.

In the reserve method:
Fix line length.
Do not try to copy from the old memory area to the new one if the
area is non existent (mBegin is NULL).

2 files changed, 45 insertions, 10 deletions

diff --git a/include/vector b/include/vector
index b5f363a..a068bc3 100644
--- a/include/vector
+++ b/include/vector
@@ -205,7 +205,8 @@ bool vector<_T>::reserve(size_type new_size)
     if (is_pod<value_type>::value)
     {
         pointer oldBegin = mBegin;
-        mBegin = static_cast<pointer>(realloc(mBegin, new_size * sizeof(value_type)));
+        mBegin = static_cast<pointer>(
+            realloc(mBegin, new_size * sizeof(value_type)));
         if (!mBegin)
         {
             mBegin = oldBegin;
@@ -214,11 +215,14 @@ bool vector<_T>::reserve(size_type new_size)
     }
     else
     {
-        pointer newBegin =  static_cast<pointer>(malloc(new_size * sizeof(value_type)));
+        pointer newBegin =  static_cast<pointer>(
+            malloc(new_size * sizeof(value_type)));
         if (!newBegin) return false;
 
-        std::uninitialized_copy(mBegin, mBegin + mLength, newBegin);
-        if (mBegin) deallocate();
+        if (mBegin != NULL) {
+            std::uninitialized_copy(mBegin, mBegin + mLength, newBegin);
+            deallocate();
+        }
         mBegin = newBegin;
     }
     mCapacity = new_size;
@@ -235,7 +239,13 @@ void vector<_T>::push_back(const value_type& elt)
         if (0 == new_capacity || !reserve(new_capacity)) return;
     }
     // mLength < mCapacity
-    *(mBegin + mLength) = elt;
+    if (is_pod<value_type>::value) {
+        *(mBegin + mLength) = elt;
+    } else {
+        // The memory where the new element is added is uninitialized,
+        // we cannot use assigment (lhs is not valid).
+        new((void *)(mBegin + mLength)) _T(elt);
+    }
     ++mLength;
 }
 
diff --git a/tests/test_vector.cpp b/tests/test_vector.cpp
index 2412607..cf3b7c7 100644
--- a/tests/test_vector.cpp
+++ b/tests/test_vector.cpp
@@ -32,9 +32,11 @@
 #endif
 #include <climits>
 #include <cstring>
+#include <string>
 #include "common.h"
 
 namespace android {
+using std::string;
 using std::vector;
 static const size_t kExponentialFactor = 2;
 bool testConstructorInt()
@@ -68,6 +70,17 @@ bool testConstructorInt()
     return true;
 }
 
+bool testConstructorString()
+{
+    {
+        vector<string> vec1;
+        EXPECT_TRUE(vec1.empty());
+        EXPECT_TRUE(vec1.size() == 0);
+        EXPECT_TRUE(vec1.capacity() == 0);
+    }
+    return true;
+}
+
 typedef enum { ONE = 10, TWO} TestEnum;
 
 template<typename T> struct A { };
@@ -163,7 +176,7 @@ bool testReserve()
         c.mCount = 0;
         vector<CopyCounter> vec4(100, c);
         EXPECT_TRUE(c.mCount == 100);
-        // Resizing does not do any copy via the copy assignement op.
+        // Growing does not do any copy via the copy assignement op.
         vec4.reserve(1000);
         EXPECT_TRUE(c.mCount == 200);
         vec4.reserve(50); // reserving less than length is a nop.
@@ -192,10 +205,11 @@ bool testPushBack()
         }
         EXPECT_TRUE(vec1.capacity() == 1024);
         EXPECT_TRUE(vec1.size() == 1000);
-        EXPECT_TRUE(c.mAssignCount == 1000);
-        // Due to the multiple augmentation of the capacity, the copy
-        // constructor has been invoked.
-        EXPECT_TRUE(c.mCopyCtorCount > 0);
+        // Assignment should not be used, but the constructor should.
+        EXPECT_TRUE(c.mAssignCount == 0);
+        // Copy constructor was been invoked for each new element
+        // pushed and when the capacity was increased.
+        EXPECT_TRUE(c.mCopyCtorCount > 1000);
         EXPECT_TRUE(c.mCtorCount == 0);
     }
     {
@@ -210,6 +224,16 @@ bool testPushBack()
         EXPECT_TRUE(vec2.back() == 20);
         EXPECT_TRUE(vec2.size() == 2);
     }
+    // Push back an non-pod object.
+    {
+        string str = "a string";
+        vector<string> vec3;
+
+        vec3.push_back(str);
+        EXPECT_TRUE(vec3.size() == 1);
+        EXPECT_TRUE(vec3.front() == "a string");
+        EXPECT_TRUE(vec3.back() == "a string");
+    }
     return true;
 }
 
@@ -362,6 +386,7 @@ bool testCtorDtorForNonPod()
 int main(int argc, char **argv)
 {
     FAIL_UNLESS(testConstructorInt);
+    FAIL_UNLESS(testConstructorString);
     FAIL_UNLESS(testConstructorRepeat);
     FAIL_UNLESS(testReserve);
     FAIL_UNLESS(testPushBack);