L.S. Roth
L.S. Roth

Reputation: 447

Why do I get a memory error only for std::string?

I am trying to create my own vector class. It seems to work fine for the built-in types, but when using std::string as a parameter, the program shows me the error below after entering two strings. I have no experience with debuggers yet. Can someone point me in the right direction?

a.out(21360,0x109f1de00) malloc: *** error for object 0x7fe4f8405b68: pointer being freed was not allocated
a.out(21360,0x109f1de00) malloc: *** set a breakpoint in malloc_error_break to debug

This is my code:

#include <cstddef>
#include <iostream>
#include <stdexcept>
#include <string>

template<class T>
class vector {
    std::size_t sz{0};
    T* ptr{nullptr};
public:
    vector<T>() { }
    vector<T>(std::size_t m_sz) : sz(m_sz) {
        ptr = new T[m_sz];
        sz = m_sz;
    }

    ~vector() {
        delete[] ptr;
    }
public:
    T operator[](std::size_t n) {
        if (n < sz)
            return ptr[n];
        throw std::out_of_range(std::string("element [") + std::to_string(n) + "] does not exist");
    }
public:
    T* begin() { return sz ? ptr : nullptr; }
    T* end() { return sz ? ptr + sz : nullptr; }
    bool empty() { return !sz; }
    std::size_t size() { return sz; }
    inline void push_back(T);
};

template<class T>
void vector<T>::push_back(T x) {
    T* tmp_ptr = ptr;
    ptr = new T[sz + 1];
    for (std::size_t i(0); sz && i != sz; ++i)
            ptr[i] = tmp_ptr[i];
    if (tmp_ptr)
        delete tmp_ptr;
    ptr[sz] = x;
    ++sz;
}

int main() {
    vector<std::string> vect;
    for (std::string str; std::cin >> str;)
        vect.push_back(str);
    for (std::string& str : vect)
        std::cout << str << ' ';
    std::cout << '\n';
    return 0;
}

Upvotes: 0

Views: 252

Answers (2)

tai
tai

Reputation: 528

Your push_back function has a small typo:

if (tmp_ptr)
    delete tmp_ptr;

should be

delete[] tmp_ptr;

Upvotes: 1

Ted Lyngmo
Ted Lyngmo

Reputation: 117168

You should delete[] what you new[] (everywhere).

Both your destructor and push_back causes undefined behavior.

Your push_back should look something like this:

template<class T>
void vector<T>::push_back(T x) {
    T* old_ptr = ptr;
    ptr = new T[sz + 1];
    for (std::size_t i = 0; i < sz; ++i) // corrected loop condition
        ptr[i] = old_ptr[i];  
    delete[] old_ptr;   // no need to check if it's `nullptr`, but delete[]
    ptr[sz] = x;
    ++sz;
}

Upvotes: 2

Related Questions