🔒 Locked

deleting with std::vectors

Started by Laroche Dec 30, 2002 at 7:51 PM 13 replies 300+ views

Original Post

Dec 30, 2002 at 7:51 PM

Im just wondering if this is safe:

    
void CManager::Release()
{
	for (std::vector<CAsset*>::iterator i = AssetVector.begin(); i != AssetVector.end(); ++i)
	{
		CAsset* p = (*i);
		delete p;
	}
}

Because I'm getting a strange error i've never really had before during debug: DAMAGE: After normal block blah blah .... I'm not quite certain what it is, but it happens when I try and quit the app. Any help would be appreciated! EDIT: Just in case this is important, CAsset is a Base class pointer, And whatever is in manager is derived from it. [edited by - laroche on December 30, 2002 8:52:50 PM]

Alan Kemp

Dec 30, 2002 at 8:38 PM

Although you are deleting the objects you have ''newed'', you are not removing them from the vector.

To remove from a vector call erase(..) passing an iterator to the element you want to remove. However, calling erase also invalidates any iterators you have to the vector, messing up your for loop.

I usually do something like this:

  
while(AssetVector.size() > 0)
{
    delete *AssetVector.begin();
    AssetVector.erase(AssetVector.begin());
}

Alan

Fruny

Dec 30, 2002 at 8:54 PM

It is only safe if CAsset has a virtual destructor, otherwise the effects are undefined.

The erase() call is incorrect ( you should delete at the back() and call pop_back() )

[edited by - Fruny on December 30, 2002 9:56:45 PM]

Stoffel

Dec 31, 2002 at 12:04 AM

"Damage After Normal Block" means you have overwritten allocated memory. The compiler inserts code to let you know when you do this, but it can only make the check when you try to free memory. You caused the problem some time before this routine; it''s just detected at this time.

Your routine will work as intended--releasing all dynamically allocated assets in AssetVector. It could be a little more efficient (not checking end each loop iteration), but it''s mostly fine. Your vector will be full of pointers to deleted objects at the end of the loop; all you need to do to empty the vector of these pointers is put AssetVector.clear () at the end of the function.

Now, as to your real problem, we can''t tell from this code. If it comes in the middle of the loop, it means that you somehow stomped over the memory after one of your CAssets. Make sure your base class destructor is virtual. If you can identify which asset it is, set up a memory breakpoint right after the asset (your debug build should fill 4 bytes of 0xFE or something after the memory block of 0xCDCDCDCD--this is your guard area).

Alan: that code is O(N^2). Each iteration through the loop copies the entire vector back onto itself (minus the first element).

Fruny: I wouldn''t delete at the back and call pop_back, because that''s also less efficient than doing this loop and following with clear.

T h i n k L i b e r a l l y .

Thunder_Hawk

Dec 31, 2002 at 12:14 AM

quote:
Original post by Stoffel
T h i n k L i b e r a l l y .

I like the period.

Laroche

Dec 31, 2002 at 7:04 PM

Can you give me an example of overwriting allocated memory? I''m looking through my code like nuts but I can''t find anything like that.

Martee

Dec 31, 2002 at 7:14 PM

  
char* p = new char[10];

for(int n = 0; n <= 10; ++n)  // Oops! Off by one

   p[n] = 0;

delete[] p;

[edited by - Martee on December 31, 2002 8:15:09 PM]

Laroche

Dec 31, 2002 at 11:55 PM

Could it be because of the way I store my assets? Here is an example:

  
// Storing assets

void CManager::PushAsset(CAsset *pAsset)
{
	if (std::find(	AssetVector.begin(), 
					AssetVector.end(), pAsset) == AssetVector.end())
	AssetVector.push_back(pAsset);
}

// Requesting Assets

CAsset * CManager::RequestAsset(int nID)
{
	for (std::vector<CAsset*>::iterator i = AssetVector.begin(); i != AssetVector.end(); ++i)
	{
		if ((*i)->GetID() == nID)
		{
			return (*i);
		}
	}

	return 0;
}

// Called once in initialization code inside CGame::Initialize()

	CSprite * pSprite = new CSprite();
	
	pSprite->SetID(0);
	pSprite->Initialize();
	pSprite->CreateSize(400);


	pSpriteManager->PushAsset(pSprite);

	PushManager(pSpriteManager);

// requesting asset

	ManagerPtr pManager = RequestManager(0);
	CSprite * pSprite = static_cast<CSprite*>(pManager->RequestAsset(0));

ManagerPtr is a typedef for boost::shared_ptr. I though it would have helped fix the problem but thats not it at all

Fruny

Jan 1, 2003 at 12:00 AM

I ask one last time. Is the destructor for CAsset, or whatever base class, declared virtual ?

Laroche

Jan 1, 2003 at 12:04 AM

yes, it is, in CAsset and in all derived classes

Stoffel

Jan 1, 2003 at 2:59 PM

I don''t see anything obviously incorrect in the code you''ve posted. Looks like you have to do this the hard way--figure out which block is damaged, run the program again and set a breakpoint when that memory changes (you can break on a memory change instead of a line # in MSVC). When you hit that breakpoint, you''ve hit the naughty section.

T h i n k L i b e r a l l y .

Laroche

Jan 9, 2003 at 12:14 PM

How do I set a breakpoint for a memory location?

Qw3r7yU10p!

Jan 9, 2003 at 1:03 PM

If you''re using msvc++ then it comes with documentation about how to set a breakpoint at a memory location

Seriema

Jan 23, 2003 at 7:09 PM

Maybe this is a little late, but it''s about your deleting every element. Why don''t you just call .clear() which calls .erase(begin(), end()) ? It pops an element, and destroys it.

Come to think of it, does it release the memor too? If you have a vector Will it release everything when the destructor is called, or do we have to delete every single element?..

"No lies of sugar can sweeten the sournes of reality"

}+TITANIUM+{ A.K.A. DXnewbie[onMIRC]

Fruny

Jan 23, 2003 at 7:48 PM

quote:
Original post by Seriema
Maybe this is a little late, but it''s about your deleting every element. Why don''t you just call .clear() which calls .erase(begin(), end()) ? It pops an element, and destroys it.

Come to think of it, does it release the memor too? If you have a vector Will it release everything when the destructor is called, or do we have to delete every single element?..

Pointers don''t have destructors. The objects won''t be destroyed, the memory won''t be released. Use a smart pointer class.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.