1

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?

Kasper Holdum
  • 12,993
  • 6
  • 45
  • 74

5 Answers5

2

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

JaredReisinger
  • 6,955
  • 1
  • 22
  • 21
2

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.

David
  • 9,635
  • 5
  • 62
  • 68
2

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.

Gangadhar
  • 1,893
  • 9
  • 9
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);
Jeff Foster
  • 43,770
  • 11
  • 86
  • 103
1

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.

Community
  • 1
  • 1
sbi
  • 219,715
  • 46
  • 258
  • 445
  • I ordered Accelerated C++ from Amazon just now. What is the reason for not needing to deallocate the name_ variable when you set it to a new value? – Kasper Holdum Aug 07 '10 at 18:23
  • All the logic of allocating and deallocating is hidden behind std::string. – Jeff Foster Aug 07 '10 at 18:38
  • @Qua: When it's `std::string`, then that class takes care of memory management for you. When it's C strings (pointers to the first character of a dynamically allocated array), you have to do it manually. But then you also have to keep track of which pointer really points to a dynamically allocated array.A string literal isn't dynamically allocated, so it doesn't need to be `delete[]`'d. But inside of your class you don't know whether a pointer handed in points to a dynamically allocated array. So you have to always copy it into your own array and leave... – sbi Aug 07 '10 at 18:43
  • ...managing the pointer handed in to the caller. The fact that this is error-prone is one of the main reasons C++ with its abstraction mechanisms was invented in the first place. I suppose a string class was among the first dozen of classes ever written for C++. Now that we have `std::string` just use that. – sbi Aug 07 '10 at 18:46
  • Excellent insight into topics that aren't that dominant in the world of .NET. – Kasper Holdum Aug 07 '10 at 19:06