pointer to a vector: getting segmentation fault

Question

I'm getting the segmentation fault in the following code. Could you please help me to figure it out? In the following code, it prints "ok here". Once it deallocates the memory, it shows the segmentation fault. But why ? any solution ?

Any help on this will be appreciated.

#include <iostream>
#include <cstring>
#include <vector>
using namespace std;

class Cube
{
public:
    char *str;

    Cube(int len)
    {
        str = new char[len+1];
    }
    Cube(const Cube &c)
    {
        str = new char[strlen(c.str) + 1];
        strcpy(str, c.str);
    }   
    ~Cube()
    {
        delete [] str;
    }
};

void foo(vector <Cube> *vec)
{
    for (int i = 0; i < 10; i++)
    {
        char in [] = "hello !!";
        Cube *c = new Cube(strlen(in)+1);
        strcpy(c->str, in);
        vec->push_back(*c);
        cout << "ok here" << endl;
        delete [] c;
    }
}

int main()
{
    vector <Cube> vec;

    foo(&vec);  
    return 0;    
}

Have you tried using gdb to find the line that is failing? Seems like the problem is that you're delete[]-ing the str variable when it is a char *. — Suroot, Jun 22 '11 at 01:54

score 4 · Answer 1 · edited May 23 '17 at 12:27

You have not followed the rule of three: Cube does not implement a correct copy assignment operator. As others have mentioned, you have also mismatched new[] with delete, which can only end in tears.

That said, you don't need to use pointers or explicit dynamic allocation for any of this. You should replace your usage of new char[] with std::string and don't allocate any Cube objects on the heap. In a well-written C++ program, new should be used rarely and delete should almost never need to be used.

This program does no explicit dynamic allocation, has the same result as you expect from your program, but is correct. Note how much cleaner code is when you don't worry about dynamically allocating or explicitly destroying things yourself!

#include <string>
#include <vector>

struct Cube {
    std::string str;
    explicit Cube(std::string const& s) : str(s) { }
};

void foo(std::vector<Cube>& vec) {
    for (int i = 0; i < 10; ++i) {
        vec.push_back(Cube("hello !!"));
    }
}

int main() {
    std::vector<Cube> vec;
    foo(vec);
}

Make sure that you have a good introductory C++ book.

score 1 · Answer 2 · answered Jun 22 '11 at 01:52

1

You are deleteing an array of Cubes here:

delete [] c;

But you didn't allocate an array here:

Cube *c = new Cube(strlen(in)+1);

It should be just:

delete c;

answered Jun 22 '11 at 01:52

Jim Buck

20,482
11
57
74

score 0 · Answer 3 · answered Jun 22 '11 at 01:52

0

delete [] c;

should be

delete c;

Aside from that, the Cube class itself is dubious. Better use a std::string instead of a c-style string.

answered Jun 22 '11 at 01:52

Xeo

129,499
52
291
397

score 0 · Answer 4 · answered Jun 27 '11 at 20:41

0

You used

delete [] c;

If c was allocated like this:

Cube * c = new Cube[3]; //3 for example

THEN delete [] c would be appropriate. In this case, you should omit the [].

answered Jun 27 '11 at 20:41

MGZero

5,812
5
29
46

pointer to a vector: getting segmentation fault

4 Answers4