Sign in to follow this  
thejoshinator

Memory leak?

Recommended Posts

Hey guys, This is my first attempt at a copy constructor, but I'm not sure how exactly to go about copying the pointers. For example, I know I have to create a new HDC via the 'new' pointer, but do I set the new HDC equal to the hdc of the object being copied, or do I create a HDC that is compatible with it? Here's the code:
CMap::CMap(CMap &map)
{
	// This function is the copy constructor for the CMap class

	int index = 0; // looping variable

	// copy the shallow members
	this->map_height = map.map_height;
	this->map_width = map.map_width;
	this->scale = map.scale;
	this->scrn_height = map.scrn_height;
	this->scrn_width = map.scrn_width;
	this->start_tile = map.start_tile;
	strcpy(this->mapname, map.mapname);
	strcpy(this->tileset, map.tileset);

	// Make new bmps and hdcs
	this->hdc = *new HDC;
	this->bmp = *new HBITMAP;
	this->old_bmp = *new HBITMAP;

	// Copy the deep members
	this->hdc = CreateCompatibleDC(map.hdc); // is this right? or does it need to be "this->hdc = map.hdc"?
	this->bmp = map.bmp; // Same question here?
	this->old_bmp = map.old_bmp;
} // end CMap(CMap &map);

Thanks, The Joshinator

Share this post


Link to post
Share on other sites
If you have the desire and need for the most accurate answer, download MMGR and add it to your project. Follow the directions in the .h and .cpp file which are #include the mmgr.h after system header files but before yours so it will compile correctly. Simply run once and look at the log generated. It will tell you if you have any memory leaks. I can personally vouch for this free code. It is the best thing I have ever seen. Not only that, Fluid Studios has some more great code, such as a logger for use.

Now if you do verify that you have memory leaks, someone else will have to exaplain the whole logics behind it. I am not that well versed with the whole Win32 drawing contexts. Hope this helps.

- Drew

Share this post


Link to post
Share on other sites
I think you're just creating a new pointer to point to the original data. Try memcpy and use some profiler to check for leakage like the above poster suggest.

Also try creat an instance of the map and then copy it then delete the original and see what happen to the copy data.

Share this post


Link to post
Share on other sites
You dont' need to create a new pointer to an hdc, just using createcompatableDC will work.

You should also modify your decalaration to take a const CMap& not a CMap&. Same for the Bitmaps, generally a deep copy cstr would create a clone of each object not another reference to each object. If all you need is a reference then a shallow copy cstr should work as none of your types is reference counted.

Cheers
Chris

Share this post


Link to post
Share on other sites
Quote:
...generally a deep copy cstr would create a clone of each object not another reference to each object.

That is what I want to do - create a deep copy with clones instead of references. How do i clone a HBITMAP, data and all?

Share this post


Link to post
Share on other sites
You should never have a '*' in front of new like that. Not only are you not ever storing the pointer ANYWHERE, but since the fields themselves are not pointers to begin with then there would never be any need to allocate at all, so yes you definately have 3 leaks there.
You most certainly do mean to have pointers however, and want to allocate new new bitmaps etc and copy memory from one to the other.

Can you show us the declaration of CMap please?

Share this post


Link to post
Share on other sites
Here it is:
// CMap class
class CMap
{
public:
// public functions
CMap(); // Constructor: Sets everything to 0 or NULL
CMap(const CMap &map); // Copy Constructor: makes deep copies of a CMap object

void Set_Mapname(char *name); // Set the mapname
void Set_Tileset(char *filename); // The tileset to use
void Set_Start_Tile(int index) {start_tile = index;} // Set the starting tile
void Set_Map_Width(int width) {map_width = width+2;} // Set the map width
void Set_Map_Height(int height) {map_height = height+2;} // Set the map height
void Set_Screen_Width(int width) {scrn_width = width+2;} // Set the screen width
void Set_Screen_Height(int height) {scrn_height = height+2;} // Set the screen height
void Set_Tile_Width(int width) {tile_width = width;} // Set the tile width
void Set_Tile_Height(int height) {tile_height = height;} // Set the tile height
void Set_Scale(float percent_change) {scale = percent_change/100;} // Set the scale of the map

char *Get_Mapname() {return mapname;} // Returns the mapname
int Get_Tile_Width() {return tile_width;} // Returns the width of the tiles
int Get_Tile_Height() {return tile_height;} // Returns the height of the tiles
int Get_Map_Width() {return map_width;} // Returns the width of the map in tiles
int Get_Map_Height() {return map_height;} // Returns the height of the map in tiles

int Add_Layer(CWindraw *windraw); // Adds one layer to the map
void Add_Tileset(int tile_index, // Adds one tileset to the map
char *filename);
int Draw_Tiles(HDC dest_hdc); // Draw all the tiles
int Load(char *filename); // Loads a map from disk

void Reset(); // Resets all the variables to clear the way for the new data
int Save(char *filename); // Save the current map

~CMap(); // Frees up the memory

protected:
// protected members
char mapname[80], // Name of the map
tileset[80]; // Name of the tileset to use

int start_tile, // Index of the starting tile
x,y, // Absolute coords of the map for scrolling purposes
map_width, // Width of the map in tiles
map_height, // Height of the map in tiles
scrn_width, // Width of the screen in tiles
scrn_height, // Height of the screen in tiles
tile_width, // Width of each tile
tile_height; // Height of each tile

float scale; // scale to use for zooming

vector<CLayer*> layers; // Vector of layers

HDC hdc; // HDC to use for tilesets
HBITMAP bmp; // bmp to use for tilesets
HBITMAP old_bmp; // used to save old bmp
};

Share this post


Link to post
Share on other sites
Sidenote
Storing HDC like you do is a no-no. HDC are global resources, and you should release them when you don't use them anymore. Moreover, it seems you are trying to duplicate them - because you are storing them. Using this you may create a system wide HDC shortage which could result in undefined behaviors (unpaint buttons or control are the first symptomes).

Are you sure you really need it? And, if you need it, are you sure you really want to clone it?

Regards,

Share this post


Link to post
Share on other sites
Well I use this class in a tilemap editor, and I do release the hdc in the deconstructor. This isn't the only class that does this though. I also have CLayer and CTile, each of which have their own HBITMAPs and HDCs. A CMap is an array of CLayers, which in turn are arrays of CTiles. But like I said, all three classes release their resources when they go out of scope. I couldn't find any other way to have an array of bitmaps that I could blit to a backbuffer. Before I started the project I suspected that this wasn't exactly the best way, but the response from a few people at GT.com gave me the impression that it was a good way to go. This was a looooong time ago, so don't ask me for the thread or anything cause I won't remember lol. But is there a better way to do this?

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