strlen receives NULL instead of correct input

Question

I'm implementing a template vector-like class. I want my vector to operate on int and Product class, which is defined in main. It works good with int, but with Product class Valgrind reports the following:

==22629== Memcheck, a memory error detector
==22629== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==22629== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==22629== Command: ./a.out
==22629== v2 = ==22629== Invalid read of size 1
==22629==    at 0x4C30F62: strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22629==    by 0x4018DF: Product (main.cpp:33)
==22629==    by 0x4018DF: my_vector (my_vector.h:24)
==22629==    by 0x4018DF: void test_my_vector<Product>(Product, Product) (main.cpp:100)
==22629==    by 0x400E67: main (main.cpp:175)
==22629==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==22629== 
==22629== 
==22629== Process terminating with default action of signal 11 (SIGSEGV)
==22629==  Access not within mapped region at address 0x0
==22629==    at 0x4C30F62: strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22629==    by 0x4018DF: Product (main.cpp:33)
==22629==    by 0x4018DF: my_vector (my_vector.h:24)
==22629==    by 0x4018DF: void test_my_vector<Product>(Product, Product) (main.cpp:100)
==22629==    by 0x400E67: main (main.cpp:175)
==22629==  If you believe this happened as a result of a stack
==22629==  overflow in your program's main thread (unlikely but
==22629==  possible), you can try to increase the size of the
==22629==  main thread stack using the --main-stacksize= flag.
==22629==  The main thread stack size used in this run was 8388608.

Ok, it seems I pass NULL to strlen, but since I pass NULL and I don't have any Product constructors, which fill the name_ field with NULL, it implies I have already deleted object, which is being passed to copy constructor. At least these are my thoughts.

But I don't really understand where is my mistake. Maybe it is because of inappropriate use of placement new or some other memory-management fault.

I also have test_my_vector function, which tests my vector.

main.cpp

#include <iostream>
#include <fstream>
#include <cstring>
#include <assert.h>
#include "my_vector.h"
using namespace std;
class Product {
public:
    Product(const char* name, int quantity, double price){
        name_ = new char[strlen(name) + 1];
        strcpy(name_, name);
        quantity_ = quantity;
        price_ = price;
    }


    Product(){
        name_ = new char[strlen("") + 1];
        strcpy(name_, "");
        quantity_ = 0;
        price_ = 0;
    }

    ~Product(){
        delete [] name_;
    }

    Product(const Product& other){
        price_ = other.price_;
        quantity_ = other.quantity_;
        name_ = new char[strlen(other.name_) + 1];
        strcpy(name_, other.name_);
    };

    const Product& operator=(const Product& other){
        delete [] name_; 
        price_ = other.price_;
        quantity_ = other.quantity_;
        name_ = new char[strlen(other.name_) + 1];
        strcpy(name_, other.name_);
        return other;
    };


private:
    char* name_;
    int quantity_;
    double price_; 
};



template <typename T>  
void test_my_vector(T t1, T t2){
   //some asserts
}

int main() {
    test_my_vector<int>(5, 10);
    test_my_vector<Product>(Product("asdf", 4, 12.0), Product("qwe", -1, 7.5));
    return 0;
 }

And my_vector.h

template <class T>
class my_vector{
public:
my_vector(){
    capacity_ = 0;
    size_ = 0;
    array_ = NULL;
};

my_vector(const size_t n){
    size_ = n;
    capacity_ = (size_t) pow(2, ceil(log(n)/log(2)));
    array_ = (T*)(new char[sizeof(T) * capacity_]());
};

my_vector(const my_vector<T>& other){
    size_ = other.size_;
    capacity_ = other.capacity_;
    array_ = (T*)(new char[sizeof(T) * capacity_]);
    for(size_t i = 0; i < size_; i++)
        new (&array_[i]) T(other.array_[i]), std::cout << i;
};

my_vector<T>& operator=(const my_vector<T>& other){
    this -> ~my_vector();
    size_ = other.size_;
    capacity_ = other.capacity_;
    array_ = (T*)(new char[sizeof(T) * capacity_]);
    for(size_t i = 0; i < size_; i++)
        new (&array_[i]) T(other.array_[i]);;
    return *this;
};

~my_vector(){
    for(size_t i = 0; i < size_; i++)
        array_[i].~T();
    delete [] (char*) array_;
};



void resize(const size_t n){
    reserve(n);
    size_ = n;
};

void reserve(const size_t n){
    if (n > capacity_){
        if(!capacity_) capacity_ = 1;
        while(capacity_ < n)
            capacity_ *= 2;
        T* new_array = (T*)(new char[sizeof(T) * capacity_]);
        for (size_t i = 0; i < size_; i++)
            new (&new_array[i]) T(array_[i]);;
        this -> ~my_vector();
        array_ = new_array;
    }
};


void push_back(const T& t){
    if (!capacity_)
        reserve(2);
    else if(size_ == capacity_)
        reserve(2 * capacity_);
    new (&array_[size_]) T(t);
    size_++;
};

private:
   size_t capacity_;
   size_t size_;
   T* array_;
};

Answer 1

First error: Your my_vector constructor does not create objects. All it does is initialize the memory for the objects, but doesn't construct them. What winds up happening is that subsequent copy operations are working with invalid objects.

The correction is below:

my_vector(const size_t n)
{
    size_ = n;
    capacity_ = (size_t)pow(2, ceil(log(n) / log(2)));
    array_ = (T*)(new char[sizeof(T) * capacity_]());
    for (size_t i = 0; i < size_; i++)
        new (&array_[i]) T();  // default construct objects
};

In addition, I don't recommend using pow if you're going to use integer exponents. The link in the comments, plus this one shows what can happen when using pow for integer calculations.

Second error: Your assignment operators are incorrect in that they do not check for self assignment. Save yourself all of that trouble and use the copy / swap idiom for your assignment operator:

#include <algorithm>
//...
Product& operator=(const Product& other)
{
    Product temp(other);
    std::swap(temp.name_, name_);
    std::swap(temp.price, price);
    std::swap(temp.quantity_, quantity_);
    return *this;
}

The same thing can be done with my_vector:

my_vector<T>& operator=(const my_vector<T>& other)
{
    my_vector<T> temp(other);
    std::swap(temp.size_, size_);
    std::swap(temp.capacity_, capacity_);
    std::swap(temp.array_, array_);
    return *this;
}