Pointer issue in custom string class

[jamesa917]

I'm working through Accelerated C++ and one of the chapters involves making our own string class 'Str'. One of the end of chapter questions asks us to have the class manage its own storage rather than using a vector or similar.

The previous chapter involved making our own Vec class, so I've based the memory management on the techniques used for that, but I have a problem somewhere with my assignment operator.

Header file:

#ifndef GUARD_Str_h
#define GUARD_Str_h

#include <iostream>
#include <algorithm>
#include <iterator>
#include <cstdlib>

#include <memory>

class Str {
public:
    typedef std::size_t size_type;
    typedef char* iterator;
    typedef const char* const_iterator;
    typedef char value_type;
    typedef char& reference;
    typedef const char& const_reference;

    Str() { create(); }
    Str(const char* cp) { create(cp); }
    template <class In> Str(In b, In e) { create(b, e); }
    ~Str() { uncreate(); }
    Str& operator=(const Str&);

    void push_back(const char);
    size_type size() const { return avail - data; }

    iterator begin() { return data; }
    const_iterator begin() const { return data; }
    iterator end() { return avail; }
    const_iterator end() const { return avail; }

private:
    char* data;     // first character in the Str
    char* avail;    // (one past) the last character in the Str
    char* limit;    // (one past) the allocated memory

    std::allocator<char> alloc;     // object to handle allocated memory

    // allocate and initialize the underlying array
    void create();
    void create (const char*);
    template <class In> void create(In, In);

    // destroy the elements in the array and free the memory
    void uncreate();

    // support functions for push_back
    void grow();
    void unchecked_append(const char);

};

template <class In> void Str::create(In b, In e)
{
    data = avail = limit = 0;
    std::copy(b, e, std::back_inserter(*this));
}

std::ostream& operator<<(std::ostream&, const Str&);

#endif 

Source file:

#include "Str.h"

using std::ostream;
using std::back_inserter;
using std::copy;
using std::size_t;
using std::uninitialized_fill;
using std::strlen;
using std::uninitialized_copy;
using std::max;
using std::ptrdiff_t;

ostream& operator<<(ostream& os, const Str& s)
{
    for (Str::size_type i = 0; i != s.size(); ++i)
        os << s[i];
    return os;
}

void Str::create()
{
    data = avail = limit = 0;
}

void Str::create(const char* cp)
{
    size_t slen = strlen(cp);
    data = alloc.allocate(slen);
    limit = avail = uninitialized_copy(cp, cp + slen, data);
}

void Str::uncreate()
{
    if (data) {
        iterator it = avail;
        while (it != data)
            alloc.destroy(--it); // destroy everything in the array
        alloc.deallocate(data, limit - data); // return space that was allocated
    }
    // reset pointers
    data = limit = avail = 0;
}

Str& Str::operator=(const Str& rhs)
{
    if (&rhs != this) {
        uncreate();
        create(rhs.begin(), rhs.end());
    }
    return *this;
}

void Str::push_back(const char c) {
    if (avail == limit)
        grow();             // if the array is full, make a bigger one
    unchecked_append(c);    // append the new char
}

void Str::grow()
{
    // when growing, allocate twice as much space as currently in use
    size_type new_size = max(2 * (limit - data), ptrdiff_t(1));

    // allocate new space and copy existing elements to the new space
    char* new_data = alloc.allocate(new_size);
    char* new_avail = uninitialized_copy(data, avail, new_data);

    // return the old space
    uncreate();

    // reset pointers to point to the newly allocated space
    data = new_data;
    avail = new_avail;
    limit = data + new_size;
}

void Str::unchecked_append(const char c)
{
    alloc.construct(avail++, c);    // copy new char to end
}

My issue is that if I run the following code, I get a "pointer being freed was not allocated" error which I've narrowed down to the assignment of s1 to s2. But if instead I use Str s2(s1.begin(), s1.end()) I don't get this error, even though using Str s2(s1) should use the same create function!

#include <iostream>

#include "Str2.h"

using std::cout;
using std::cin;
using std::endl;

int main()
{
    Str s1 = "Hello";
    Str s2(s1);
    cout << s1 << endl << s2;
    return 0;
}

Help would be much appreciated as I can't track down where I've gone wrong! Another observation is that if I change s1 (e.g. s1 = "Goodbye") then s2 no longer prints "Hello", so I'm pretty sure my assignment operator is somehow only keeping a pointer from s2 to s1 rather than copying every char.

[MiiNiPaa]

Str s2(s1);

It calls copy constructor which you did not define. So it uses compiler generated one which does a memberwise copy. As a result s1 and s2 contains same pointers.
When their destructors are executed at the end of main() said pointers are deleted twice which leads to your error.

[jamesa917]

Brilliant, thanks for the reply!