CODEWITHSUNDEEP
c++memory-management
Ian Ling
Ian Ling

Reputation: 334

Necessary to delete dynamic struct contents, and then the struct itself?

#include <cstring>
using namespace std;

struct Product {
   char *     name;
   float      price;
};

int main() {
    Product * bread = new Product;
    bread->name = new char[6];
    bread->name = "bread";

    delete[] bread->name; //!!!THE ERROR OCCURS ON THIS LINE!!!
    delete bread;
}

Gives me the following error:

*** Error in `./out': munmap_chunk(): invalid pointer: 0x0000000000400824 ***

My question is whether or not it's even necessary to delete bread->name, or if deleting bread will take care of that for me. If it is necessary to delete bread->name, why does the program crash when I try to do that?

Upvotes: 1

Views: 454

Answers (2)

nonsensickle
nonsensickle

Reputation: 4528

int main() {
    Product * bread = new Product;
    bread->name = new char[6];
    bread->name = "bread"; // <- Error! Overwriting the pointer value!

    delete[] bread->name; // <- Error! Trying to free read-only memory where "bread" is stored...
    delete bread;
}

Basically you're overwriting your dynamically allocated char array new char[6] with a constant char array of "bread". Remove the dynamic allocation and you won't have to delete it.

When you write "bread" the compiler takes the letters and stores them in read-only memory. Every time you write "bread" and try and assign it you are effectively assigning a pointer to read-only memory, or a const char*.

When you allocated your dynamic array you stored the address of the dynamically allocated memory in the bread->name pointer but then you overwrote it with the address of the read-only memory which you are not allowed to free. Hence the compiler is complaining about it.

In your code since you no longer have the pointer to the dynamically allocated memory, new char[6], you can no longer free it and you also have a memory leak.

I would do it like this (given your constraints on using strings) and assuming you really want the dynamic allocation:

int main() {
    const char* breadStr = "bread";
    int len = strlen(breadStr);

    Product * bread = new Product;
    bread->name = new char[len + 1];
    strncpy(bread->name, breadStr, len + 1); // Copy the string and the \0 (hence the +1)

    delete[] bread->name; // no more error!
    delete bread;
}

If dynamic allocation is not required then you could do something like this:

int main() {
    Product * bread = new Product;
    bread->name = "bread";

    // You don't have to delete the bread->name since it is NOT dynamically allocated but is in your read-only memory.
    delete bread;
}

But I would really prefer to do all of this in C++ using strings like @Barry suggested.

Upvotes: 3

Barry
Barry

Reputation: 302663

The issue actually stems from here:

bread->name = "bread";

After allocating a new array for name, you are assigning that pointer to a completely different value - one that happens to live in read-only memory. Hence the error when you delete it: you're trying to delete [] an array that you didn't allocate.

The key issue is that you don't want to assign the pointer name, you want to populate the contents of the array you just allocated - you want to populate what name points to. For that, strcpy:

strcpy(bread->name, "bread");

Or really, since this is C++:

struct Product {
   std::string  name;
   float        price;
};

Product bread;
bread.name = "bread";

Upvotes: 8

Related Questions