Sign in to follow this  
minipie8

assertion error - deleting a struct that contains heap reference

Recommended Posts

The program I'm making has a struct that contains arrays from the heap. When I allocated an array of these struct, there is no way to delete that array without an assertion error. I tried making a more general program with the same thing, but that did not work either (that simpler program is listed below). When I debugged the program, I noticed that the destructor was only called once, instead of 10 times, like I expected. The specific error is when the delete operator calls _ASSERTE(_BLOCK_TYPE_IS_VALID(pHead->nBlockUse)); Does anyone know of a way to delete the array?
#include <stdio.h>
#include <string.h>

struct thingy
{
protected:
	char * name;
public:
	thingy()
	{
		name = new char[101]; //100 char and a /0
		for (unsigned int i; i < 101; i++)
		{
			name[i] = 0;
		}
	}
	~thingy()
	{
		delete name;
	}
	void setname(char * newname)
	{
		for (unsigned int i; i < 101; i++)
		{
			name[i] = 0;
		}
		for (i = 0; ((i < 100) && (newname[i] != 0)); i++)
		{
			name[i] = newname[i];
		}
	}
	void getname(char * sendname)
	{
		strcpy(name, sendname);
	}
};

int main()
{
	thingy * a;
	a = new thingy[10];
	delete a; //this causes an assertion error
	return 0;
}

[Edited by - Washu on August 5, 2004 8:44:49 PM]

Share this post


Link to post
Share on other sites
As a general rule, calls to new [] should match delete [], and new should match delete.

ie. delete [] a;

That's not to say that there are not other errors, but that'll fix the present one.

Share this post


Link to post
Share on other sites
also just for fun

this:

for (unsigned int i; i < 101; i++)
{
name[i] = 0;
}





can be replaced with:

memset( name, 0, sizeof(char)*101 );





much faster and just one line of code. :)

-me

Share this post


Link to post
Share on other sites
Quote:
Original post by Palidine
also just for fun

this:
*** Source Snippet Removed ***

can be replaced with:
*** Source Snippet Removed ***

much faster and just one line of code. :)

-me


Or better yet:


name = new char[101]();



To the OP:
You should always use delete[] with new[] and delete with new. Also, you should be including cstdio and cstring, not stdio.h and string.h

Secondly, you shouldn't need to reset the string to 0 when you set it, as the other string should be null terminated. Also, it would be better to return a const pointer to the string for your get method. Unless you know for a fact that you're going to be modifying the string it returns.

And finally: Magic numbers suck. use a named constant that makes sense.

Share this post


Link to post
Share on other sites
Um, just in case no-one picked this up, there's something horribly wrong with:

for (unsigned int i; i < 101; i++)

Like nowhere is i even assigned zero!!! This would probably crap out in release mode. Better to write this:

for (unsigned int i=0; i < 101; i++)

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this