Problems with deleting stuff (Solved)

Started by
12 comments, last by Zeraan 18 years, 6 months ago
I keep getting "Debug Error" dialog window when I try to delete a pointer to array of DirectGraphic Texture files... I was able to locate this error to this:

if (m_NumButtons)
	{
		for (int i = 0; i < m_NumButtons; i++)
		{
			if (m_Buttons.texture != NULL)
			{
				m_Buttons.texture->Release();
				m_Buttons.texture = NULL;
			}
		}

		delete[] m_Buttons; //this is where it makes the error
		m_Buttons = NULL;
		m_NumButtons = 0;
	}



The initalization is like this

bool CUI::LoadButtons(char *file)
{
	std::ifstream fin;
	std::ofstream fout;
	const int size = 100;
	char temp[size];
	HRESULT hr;
	
	fin.open(file);
	fout.open("Error.dat");

	// load the images
	SKIP_WHITESPACE;	
	fin >> m_NumButtons;

	m_Buttons = new SImage[m_NumButtons];

	int i;
	for (i = 0; i < m_NumButtons; i++)
	{
		SKIP_WHITESPACE;
		fin >> m_Buttons.x;
		fin >> m_Buttons.y;
		fin >> m_Buttons.w;
		fin >> m_Buttons.h;
		fin >> m_Buttons.TexFileName;
		fin >> temp;
		m_Buttons.ColorKey = strtoul(temp, NULL, 0);
		m_Buttons.texture = NULL;

		hr = D3DXCreateTextureFromFileEx(m_d3dDevice, m_Buttons.TexFileName, 0, 0, 1, 0, D3DFMT_A8R8G8B8, 
										D3DPOOL_DEFAULT, D3DX_FILTER_NONE, D3DX_FILTER_NONE, 
										m_Buttons.ColorKey,	NULL, NULL, &m_Buttons.texture);
		if (hr)
		{
			fout << m_Buttons.TexFileName;
			fout.close();
			m_Buttons.texture = NULL;
			m_NumButtons = i+1;
			Shutdown();
			return false;
		}
	}

	fout.close();
	fin.close();

	return true;
}



Any ideas why this is happening? If you need more info, let me know. [Edited by - Zeraan on October 10, 2005 5:15:27 PM]
Advertisement
What is the value of the m_Buttons pointer before you delete it? Insert a breakpoint and check it out. Most probably it's uninitialized (0xcdcdcdcd or something).

The value is: 0x00a25648 It looks like it's initalized
In my experience, debug errors from deleting pointers usually stem from clobbering the "safety bits" that Dev Studio puts after any memory you allocate with new[]. I didn't see it specifically in your code, but I'd wager that somewhere, somehow, you overwrite more than what you allocated.

Check to make sure that you're never dereferencing m_Buttons beyond m_NumButtons. By the way, what's up with setting m_NumButtons to i+1 for the error code?

Best of luck,
jujumbura
I'm using MSVC++ 6.0

As for the i+1 this code isn't written by me, it's written by a friend, and I'm trying to add more flexibility to it. Will fix that.

Right now I don't use this Buttons anywhere in the game, was in phase of setting it up and loading it.
I found out if I didn't delete the m_Buttons, it doesn't return any errors. But I think there might be some memory leak...

What do I do?

Ok, when I click on retry, it points me to this block of code
if (!CheckBytes(pbData(pHead) + pHead->nDataSize, _bNoMansLandFill, nNoMansLandSize))                _RPT3(_CRT_ERROR, "DAMAGE: after %hs block (#%d) at 0x%08X.\n",                    szBlockUseName[_BLOCK_TYPE(pHead->nBlockUse)],                    pHead->lRequest,                    (BYTE *) pbData(pHead));


I don't completely understand this code, but I'm sure it indicates what kind of error I'm doing...
Yeah, that's what I'm talking about. That "nomanslandfill" stuff is data that your compiler pads your memory with when you call new[]. Basically, it fills it with some goofy sequence of bits ( sometimes 0x0BADF00D, or things that look very distinctive in memory watch ) that are not supposed to be overwritten. When you call delete[], you're saying you're done with the memory you allocated, so it goes in and checks the padding bits that it put in after your memory. If the padding bits have changed ( meaning you clobbered them somehow ), then the compiler will yell at you.

So somehow, some way, you're overflowing your buffer. If you're sure that you don't dereference past your allocated size, it might be that you're overflowing something within the struct that you're using. Perhaps you're reading data in from the file that overflows a member of the SImage struct? I'd take a good look at what that thing stores, and what you're putting into it.

jujumbura
It all looks right, I can't find the reason

this is the SImage
struct SImage{	int x, y, w, h;	int ColorKey;	IDirect3DTexture8* texture;	char TexFileName[20];};


this is the buttons.dat text file that I load in

2
0 0 220 35 font1/ButtonLight.bmp 0xff000000
0 0 220 35 font1/ButtonDark.bmp 0xff000000

This is the code that loads in the file
for (i = 0; i < m_NumButtons; i++)	{		SKIP_WHITESPACE;		fin >> m_Buttons.x;		fin >> m_Buttons.y;		fin >> m_Buttons.w;		fin >> m_Buttons.h;		fin >> m_Buttons.TexFileName;		fin >> temp;		m_Buttons.ColorKey = strtoul(temp, NULL, 0);		m_Buttons.texture = NULL;		hr = D3DXCreateTextureFromFileEx(m_d3dDevice, m_Buttons.TexFileName, 0, 0, 1, 0, D3DFMT_A8R8G8B8, 										D3DPOOL_DEFAULT, D3DX_FILTER_NONE, D3DX_FILTER_NONE, 										m_Buttons.ColorKey,	NULL, NULL, &m_Buttons.texture);		if (hr)		{			fout << m_Buttons.TexFileName;			fout.close();			m_Buttons.texture = NULL;			m_NumButtons = i;			Shutdown();			return false;		}	}//The whitespace stuff is:#define SKIP_WHITESPACE 	while (fin.peek() == '/' || fin.peek() == '\n' || fin.peek() == ' ' || fin.peek() == '\t') fin.getline(temp, size);


There's another SImage pointer array in the same class (UI class) that has the same code, could this be causing the problem?
Wohoho! Char[20] for a filename? Danger!

This filename: "font1/ButtonLight.bmp" is 21 characters homie. Which means that on your last SImage, if you read that string into the filename, you're gonna smash the next byte over if there aren't any checks to make sure you don't write beyond your maxlength. And it doesn't sound like there are.

First of all, that's a really small buffer size for a file name. If you really want to use a hardcoded max size like that, you need to be sure that you don't just assign into it without checking the size of what you're reading in. Instead of struct.filename = "somethingorother", you probably want to use a method that will write only as many bytes as your array can hold.

But honestly, why not just use a String class for your filename in the struct instead? Then you can just assign to it, and it will handle the buffer business for you.

In any case, that sounds like where your problem is.

jujumbura
Just to add something to jujumbura's answer : you are using C++, so don't be shy with the use of Standard Template Library. Like using string and vector in your example. You'll avoid those errors while having more robust code. Do not reinvent the wheel !

This topic is closed to new replies.

Advertisement