CODEWITHSUNDEEP
c++listtemplatesstl
Someone
Someone

Reputation: 35

template class with std string argument gives segmentation fault when using local array as storage

Here is the program


using namespace std;

template <typename T>
class Test
{
public:
   struct Node{
      T data;
      Node *next;
   };

   void setData(T data)
   {
      head = reinterpret_cast<Node*>(storage);
      head->data = data;
   }
private:
   unsigned char storage[2048];
   Node* head;
};

int main()
{
   Test<std::string> test;
   test.setData("Hello");
   return 0;
}

The above program compiles and also works fine when argument is of int type. But when I set argument as std::string it crashes on - head->data = data;

Is it something I have to take care of std::string explicitly.

Thanks in advance.

Upvotes: 1

Views: 256

Answers (2)

Some programmer dude
Some programmer dude

Reputation: 409356

You tell the compiler to treat the array storage as a Node structure. But the data in storage isn't initialized in any way, it's contents is indeterminate and using it in any way leads to undefined behavior.

More specifically, the members of the "structure" are not constructed or initialized, so trying to use the member data for anything complex will (as in your case) very likely result in crashes.

The simple solution is to use placement new

head = new (storage) Node;

Note that after placement new you can't delete the object, you need to explicitly call the destructor

head->~Node();

Upvotes: 1

skratchi.at
skratchi.at

Reputation: 1151

You operate on uninitialized memory. Especially you try to implement your own memory management, which will fail in 99% of implementations.

You can simply use new and delete in your created class and don't worry about the memory any longer.

#include <string>

template <typename T>
struct Node {
    T data;
    Node *next;
};

template <typename T>
class Test
{
public:
    Test() {
        head = new Node<T>();
    }

    ~Test() {
        delete head; head = nullptr;
    }

    void setData(T data) {
        if (head = nullptr) {
            head->data = data;
        }
    }
private:
    Node<T>* head = nullptr;
};

int main() {
    Test<std::string> test_string;
    Test<int> test_int;
    test_string.setData("Hello");
    test_int.setData(1);
    return 0;
}

Important notes:

  • in the constructor of Test, the memory will be allocated
  • for every new, there must be a delete. So please note the destructor
  • for a good defensive implementation, always check, if pointers are a nullptr
  • And for a even much better code style, PLEASE DO NOT use using namespace std;. If you do so, you open an enormously large namespace, which could collide with some of your class names or function names. This error nearly impossible to find.

disclaimer: this works on MSVC2017.

BONUS

For a more modern c++, you could use a std::unique_ptr.

#include <string>
#include <memory>

template <typename T>
struct Node {
    T data;
    Node *next;
};

template <typename T>
class Test
{
public:

    void setData(T data)
    {
        if (head = nullptr)
        {
            head->data = data;
        }
    }
private:
    std::unique_ptr<Node<T>> head = std::make_unique<Node<T>>();
};

int main()
{
    Test<std::string> test_string;
    Test<int> test_int;
    test_string.setData("Hello");
    test_int.setData(1);
    return 0;
}

Upvotes: 1

Related Questions