0

Basically I am trying to implement my own data type similar to vector. I'm doing this just for fun. Thing is that I constantly get Segmentation fault when trying to put a new element of type string (tried to create a struct named person and it had the same problem). Piece of code which is I think most important in this case (table.h):

template<typename type>
class table
{
    private:
        type* elements = new type[0];
        unsigned int length_ = 0;

        void resize(int change)
        {
            length_ = length_ + change;
            type* elements_ = new type[length_];

            std::memcpy(elements_, elements, length_ * sizeof(type));

            delete[] elements;
            elements = elements_;
        }

    public:
        /* ... */

        void put(type&& element)
        {
            resize(1);
            elements[length_ - 1] = element;
        }
}

Then (main.cpp):

/* ... */

int main()
{
    table<string> t;
    t.append("Hello World"); // segfault
    for(string s : t) {cout << s << endl;}

    /* But this works:
    table<char*> t;
    t.append((char*)"Hello World");
    for(string s : t) {cout << s << endl;} */
}
Kazafka
  • 17
  • 5
  • Please give a [mre]. – Passerby Nov 28 '21 at 14:41
  • 3
    The code provided does not reproduce the problem described. And `std::memcpy` is being used on a non-memcpy-able object. – Eljay Nov 28 '21 at 14:43
  • Don't use `new`: use `std::vector`. I've never seen `new` used in an initialization statement. It may work, it may not. – Victor Eijkhout Nov 28 '21 at 14:50
  • You can't use `memcpy` on general types like `string`. Roughly, a `string` contains a pointer to char where the actual string data resides. If we `memcpy` that, we get two strings with the same pointer, then when both are destructed we perform a double free on the same pointer which is undefined behavior. `memcpy` is only safe on very basic data types. – chi Nov 28 '21 at 16:18
  • See this related [question](https://stackoverflow.com/questions/29777492/why-would-the-behavior-of-stdmemcpy-be-undefined-for-objects-that-are-not-triv). – 1201ProgramAlarm Nov 28 '21 at 16:57
  • Does this answer your question? [Why would the behavior of std::memcpy be undefined for objects that are not TriviallyCopyable?](https://stackoverflow.com/questions/29777492/why-would-the-behavior-of-stdmemcpy-be-undefined-for-objects-that-are-not-triv) – BartoszKP Nov 28 '21 at 21:07

1 Answers1

3

In this code

length_ = length_ + change;
type* elements_ = new type[length_];

std::memcpy(elements_, elements, length_ * sizeof(type));

you're incrementing length_ and then copying that many elements from your elements array into the new one. But elements only has length_ - change elements in it. So it's attempting to copy data from outside of the bounds of the array, which is undefined behavior.

Also as someone mentioned in the comments, you can't use std::memcpy for types like std::string. It can only be used for trivially copyable types. You can do something like this instead:

unsigned int newLength = length_ + change;
type* elements_ = new type[newLength];
std::copy(elements, elements + length_, elements_);
length_ = newLength;

or better yet just use a std::vector<type>

template<typename type>
class table
{
    private:
        std::vector<type> elements;

    public:
        /* ... */

        void put(type&& element)
        {
            elements.emplace_back(std::move(element));
        }
}
Kevin
  • 6,993
  • 1
  • 15
  • 24