Error returning a list of user-defined objects from a function in C++

Question

Background

I have written a class Str that mimics string operations, for instructional purposes. The class Str is an array of characters in essence.

I created a function makeList that receives a Str object, creates a list of Str objects using push_back and returns the list.

I am using allocators, but I have tightened the memory management to allocate only enough memory for the characters created.

Problem

• Compilation, linking, and run-time produce no errors.

• The list object content is OK when viewed within MakeList using cout << ret.back().

• The problem is that the objects within the list have garbage input overwriting the first ~10 characters when viewed in the main body.

• I have also verified that the Str I create are being destroyed via the default destructor.

I have included the minimum amount of code to replicate the problem.

Non-class code:

list<Str> makeList(const Str s)
    {
        list<Str> ret;
        ret.push_back(s);
        cout << ret.back() << endl;

        return ret;
    }

    int main() {

        Str greeting = "Hello world";
        list<Str> result;
        result = makeList(greeting);
        result.pop_front();
        cout << "The result is " << result.front() << endl;

        return 0;
    }

Class code:

class Str {
    friend std::istream& operator>>(istream&, Str&);
public:
    typedef char* iterator;
    typedef const char* const_iterator;
    typedef size_t size_type;
    typedef char value_type;

    Str() { create(); } // default constructor

    Str(const_iterator cp, const_iterator cp2) {
        create(cp, cp2);
    }
    Str(const_iterator cp) {
        create(cp, cp + strlen(cp));
    }
    ~Str() {uncreate(); }

    char& operator[](size_type i) { return data[i]; }
    const char& operator[](size_type i) const { return data[i]; }

    Str& operator=(const Str& s) {
        uncreate();
        create(s.begin(), s.end());
    };

    Str substr(size_type i, size_type d) const {
        const_iterator cStart, cTerm;
        cStart = this->begin() + i;
        cTerm  = this->begin() + i + d;
        Str sub(cStart, cTerm);
        cout << "Pointer diff = " << sub.limit - sub.data << endl;
        return sub;
    }

    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:
    iterator data;
    iterator avail;
    iterator limit;

    allocator<char> alloc;

    void create();
    void create(const_iterator, const_iterator);

    void uncreate();

};

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

void Str::create(const_iterator i, const_iterator j)
{
    data = alloc.allocate(j - i);
    limit = avail = uninitialized_copy(i, j, data);
}

void Str::uncreate()
{
    if (data) {
        iterator it = avail;
        while (it != data)
            alloc.destroy(--it);

        alloc.deallocate(data, limit - data);
    }

    data = limit = avail = 0;
    cout << "UNCREATED" << endl;
}

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

}

Answer 1

Among other things, you are missing a copy constructor for Str, but you do call it here :

 result2 = makeList(greeting); // Pass by value -> copy

And here :

 ret.push_back(s);  // Pass by value -> copy

Without it, the default copy constructor is used, but it's not doing what you want. Possible implementation :

Str(const Str& other) 
{
    create(other.begin(), other.end());
}

Always respect the rule of three