CODEWITHSUNDEEP
c++memoryvectormemory-leaksscoping
Gucu112
Gucu112

Reputation: 967

Dealing with memory, when using std::vector

I'm using vector with my own class type:

std::vector<CItem> m_vItems;

In my class I'm initializing SFML types like texture and sprite:

class CItem
{
    (...)
    sf::Texture m_Texture;
    sf::Sprite m_Sprite;
    sf::IntRect* m_pRect;
    (...)
}

I'm trying to pass object to my vector declared as member of other class CLevel and I'm doing it inside method of that class like this:

CItem *temp = new CItem(x, y, kind);
m_vItems.push_back(*temp);

As you see, I'm not deleteing temp pointer with delete, but in destructor of class CLevel I've got a simple line:

std::vector<CItem>().swap(m_vItems);

And I'm little confused about memory leaks. Is my program has got some of these or the line above solving problem and my example has been correctly written?

Upvotes: 1

Views: 89

Answers (5)

user2460683
user2460683

Reputation:

A memory leak is when space is allocated on the heap (in your case by calling new) and the reference to that memory is lost. In other words, you have no way to reclaim the memory by calling delete. If you use swap to move pointers from one vector to another then technically that's not a leak because you still have a reference to the memory in the other vector and you can still call delete.

Remember to call delete eventually. It may be tempting in some cases depending on the use to just let the system cleanup later and not delete the memory ever, e.g. if the code is a CGI. However, that can cause problems later on when the code is used in a use case not initially anticipated, e.g. a unit test that's called from a long-running framework. It's generally worth the time to code the delete right away, even if doing so has no bearing on an immediate goal, rather than put yourself in the position where you might have to circle back and try to fix it later.

Upvotes: 0

gbjbaanb
gbjbaanb

Reputation: 52689

Assuming you need to keep a vector of pointers, rather than a vector of CItems, I'd use a smart pointer to manage the object lifetime. A shared_ptr is easy to use:

shared_ptr<CItem*> temp(new CItem(x,y,z));
m_vItems.push_back(temp);

When the vector goes, the CItem will be cleaned up correctly. When the items are passed around, they will be handled correctly too - no memory leaks.

Upvotes: 0

4pie0
4pie0

Reputation: 29744

CItem *temp = new CItem(x, y, kind);
m_vItems.push_back(*temp); // here a copy of *temp is pushed into vector

You should call delete somewhere to delete what you allocated with temp:

delete temp;

to avoid memory leak. Any call to new must have matching call to delete somewhere. This doesn't influence a copy of temp that was pushed into vector. It still exist as long as vector exists.

The best is to use just:

m_vItems.push_back(CItem(x, y, kind)); // implement this constructor correctly
                                       // to avoid uninitialized variables

Always when leaks are concern you can profile your program with tool: Valgrind or Visual Leak Detector.

Upvotes: 1

mbadawi23
mbadawi23

Reputation: 1071

if your are on Windows platform, you could use the CRT library to detect if you have any leaks in a specific code section. This link explains how to do it in VS 2012 it is also available for earlier versions.

Upvotes: 0

Mark B
Mark B

Reputation: 96291

Your program calls new without a matching delete, and it didn't pass the result of new to some other class that will manage it for you. Therefore your program has a memory leak.

Do you have a problem with using m_vItems.push_back(CItem(x, y, kind)); instead of the two line example you gave?

Upvotes: 2

Related Questions