Debug Assertion Error - delete call on char pointer

Question

So I decided to dwelve a bit within the pesty C++.

When I call the delete function on a pointer to a simple class that I created I'm greeted by a Debug Assertion Failure -Expression:_BLOCK_TYPE_IS_VALID(pHead->nBlockUse). I assume this is because I've handled the string manipulation wrong and thus causing memory corruption.

I created a basic class, [I]animal[/I], that has a string defined that can be set through a function.

// name
char * ptrName;

animal::animal(char * name)
{
 this->SetName(name);
};

animal::~animal()
{
 delete [] ptrName;
}

void animal::SetName(char * name)
{
 ptrName = name;
};

When using the above class as shown below the error occurs. I've tried both delete ptrName and delete [] ptrName but to no avail.

animal * cat = new animal("Optimus Prime");
delete cat;

What am I missing?

Answer 1

Who has ownership of your string?

For example, when you construct your new animal, you're passing in a string literal - that's not yours to free.

You should consider avoiding char* and just using std::string instead.

If you have to use char*, think about ownership. For example, one option is for you to take a copy of the string (using strdup) and own that. This way you can't be stuck with strange bugs like this

char* szFoo = strdup("my string");
{
  animal a(szFoo);
}
// At this point szFoo has been deleted by the destructor of a
// and bad things will start to happen here.
printf("The value of my string %s",szFoo);

Answer 2

The string "Optimus Prime" was not dynamically allocated, and thus it is not correct to call delete on it.

Answer 3

The problem comes from deleting a pointer that you don't own. You haven't allocated the string, so you must not delete it. The C string you are using is allocated statically by the compiler.

Answer 4

The problem is that in the setName function you are merely assigning the name to ptrName. In the example, the name is a const char string pointer which you can't delete (it is not allocated on the heap). To avoid this error, you can either use a std::string in the class or allocate a new char arry in the constructor of the animal class and assign the pointer to it. Then, in the destructor you can delete the array.

Answer 5

So I decided to dwelve a bit within the pesty C++.

Then do yourself a favor and use C++ right. That would be to use std::string:

// name
std::string name_;

animal::animal(const std::string& name)
  : name_(name)
{
}

//animal::~animal() // not needed any longer

//note: copying also automatically taken care of by std::string
//animal(const animal&)
//animal& operator=(const animal&)

void animal::SetName(const std::string& name)
{
 name_ = name;
}

Have a look at The Definitive C++ Book Guide and List. I'd recommend Accelerated C++. It comes with a steep learning curve, but since you already know a bit of C++, it's the 250 pages that might set you on the right track.

As a rule of thumb: Whenever you release a resource (memory or other), and it's not in the destructor of a class whose solely purpose is to manage this one resource, something is wrong with your design. Personally, I become suspicious whenever I feel the need to write a destructor, copy constructor, or assignment operator.