C++: Deallocating dynamic memory when derived class object is destructed

Question

I seem to have an issue with dynamic memory allocation.

Below you'll see a derived class that contains a pointer for a name variable that will be dynamically allocated using the void name(const char* name) method. The function is run by the Product constructor, which sets a name for the product class when the object is created. Here's the class:

namespace sict {
  class Product :public Streamable {

    char* name_;

public:

    Product(const char* name);
    virtual ~Product();

    void name(const char* name);

}

And here's the name function itself, along with the one argument constructor:

void sict::Product::name(const char * name) {
    int g = strlen(name);
    name_ = new char[g];
    strncpy(name_, name, g);
    name_[g] = 0;
}

Product::~Product() {
    delete [] name_;
    name_ = nullptr;
}

To me this code seems capable enough of created the object then destroying it peacefully whenever it exits the scope of the function it's running in. However, when the function ends and the destructor runs, the program freezes/crashes at delete [] name_. Running the program on Visual Studio's compiler seems to yield no particular errors (other than the program freezing), but the gcc compiler detects some sort of heap corruption. Would anybody know why this is happening?

`name_[g] = 0;` writes off the end of the allocated array, corrupting the heap. Try changing `g` to `strlen(name) + 1` to confirm. Since this is C++, you'd be much better off using [`std::string`](http://en.cppreference.com/w/cpp/string/basic_string) and let it handle the memory management for you. — Sean Cline, Mar 26 '16 at 12:24
You are not adhering to the [Rule of 3](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). — PaulMcKenzie, Mar 26 '16 at 12:25
Oh, this one's easy, just use `std::string`. You know, it's C++. Not C. — Cheers and hth. - Alf, Mar 26 '16 at 12:26
It's hard to say without seeing [a Complete, Minimal, and Verifiable example](http://stackoverflow.com/help/mcve). Perhaps it's an issue related to the [Rule of 3](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). — James Adkison, Mar 26 '16 at 12:26
Your class has multiple issues. See [this question](http://stackoverflow.com/questions/36020149/how-to-design-class-that-has-char-pointer-as-class-member-variable) — PaulMcKenzie, Mar 26 '16 at 12:31
You should manage the char array with a class that does the memory management correctly already. Then declare a member of that class in `Product` and call it `name`. — M.M, Mar 26 '16 at 13:13

score 3 · Accepted Answer · edited May 23 '17 at 12:15

I'm not sure why Sean Cline didn't post his comment as an answer, but Sean is correct.

name_ is given g elements and then name_[g] is set to zero, but name_[g] is one past the end of the array. Either use name_ = new char[g+1]; or name_[g-1] = 0; so that you don't go past the end of the array.

Also, as several comments point out, any time you have a class that allocates memory dynamically, make sure you define the copy constructor, the assignment operator, and the destructor. If you miss one, the default implementation will perform a shallow copy, which can cause headaches for you.

A shallow copy is when the pointer is copied instead of the data it points to. In your case, if an object of this class is ever copied or assigned, you end up with two objects pointing to the same data on the heap, and they would both try to delete it when they run their destructors.

For more information on those functions, see the Rule of Three

C++: Deallocating dynamic memory when derived class object is destructed

1 Answers1