Skip to main content
GameDev.net gamedev.net
🔒 Locked

copying dynamic pointers

Started by xDS4Lx Feb 18, 2003 at 10:38 AM 11 replies 2.3k views
Original Post
xDS4Lx
xDS4Lx
Ok, for some reason I dont know why this wont work. Everytime I try to copy a bitmap object it fails and crahes my prog (well the copying works fine but rendering fails) Heres the code: 
  
CBitmap::CBitmap(const CBitmap& obj)
: bitmapInfoHeader(obj.bitmapInfoHeader), pucBMP(NULL), bValid(false)
{	pucBMP = new unsigned char[obj.bitmapInfoHeader.biSizeImage];
	if(pucBMP)
	{	bValid =true;
		pucBMP = obj.pucBMP;
	}
}
"Computer games don't affect kids; I mean if Pac-Man affected us as kids, we'd all be running around in darkened rooms, munching magic pills and listening to repetitive electronic music." - Kristian Wilson, Nintendo, Inc, 1989
sphinx23
sphinx23
you are allocating memory, but then you only copy the address of obj.pucBMP. this is a memory leak, because the allocated memory is lost. you should use memcpy or something similar to copy the contents of the memory and not just the address.
Fruny
Fruny
What you''re doing is :
* allocate a chunk of memory
* throw the pointer away and make it point to the old data

In the end, you haven''t made a copy, but have two pointers pointing to the same data, and a memory leak.

You need to copy the data the pointer is pointing to, not the pointer itself.

Something like :

  
#include <algorithm>

using namespace std;

CBitmap::CBitmap(const CBitmap& obj)
: bitmapInfoHeader(obj.bitmapInfoHeader), 
  pucBMP(NULL), 
  bValid(false)
{
  pucBMP = new unsigned char[obj.bitmapInfoHeader.biSizeImage];
  if(pucBMP)
  {
     bValid =true;
     copy( obj.pucBMP,
           obj.pucBMP + obj.bitmapInfoHeader.biSizeImage,
           pucBMP );
  }
}


[ Start Here ! | How To Ask Smart Questions | Recommended C++ Books | C++ FAQ Lite | Function Ptrs | CppTips Archive ]
[ Header Files | File Format Docs | LNK2001 | C++ STL Doc | STLPort | Free C++ IDE | Boost C++ Lib | MSVC6 Lib Fixes ]
"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." — Brian W. Kernighan
xDS4Lx
xDS4Lx
Well thats not the origional code, origionally i tried going through and copying each element, I watched the copying in debug and it seemed to copy right but when I went to render it, it would fail.
[EDIT]
actually I tried using memcpy and it still didnt work.

[edited by - xds4lx on February 18, 2003 12:18:15 PM]
"Computer games don't affect kids; I mean if Pac-Man affected us as kids, we'd all be running around in darkened rooms, munching magic pills and listening to repetitive electronic music." - Kristian Wilson, Nintendo, Inc, 1989
Zipster
Zipster
memcpy is, like, soooo C. We''re in C++ now, use copy

Although since you''re just copying raw char data, memcpy is fine. It''s when you start using it for class objects that it becomes questionable

I just felt like saying that. Anyway, we really can''t help you unless we see real code. Otherwise, all we can do is speculate about what the problem might not might not be.
xDS4Lx
xDS4Lx
OK, before i added the copy constructor everything worked fine, it loads and it saves and it displays lol. Im useing copy now and everytime i run the program on exit i get a run time assertion about some out of bounds referencing or deleting but as you can see i make suer the pointer is not null before deleting it. I know all the other code is fine, its copying it that is not fine. Even if it copys right it still doesnt seem to work.

[EDIT]
ok well heres the loading code and the error messages that pop up:
Debug Assertion Failed!
Program......
File: dbgdel.cpp
Line: 51
Expression: _BLOCK_TYPE_IS_VALID(pHead->nBlockUse)

Debug Assertion Failed!
Program......
File: dbgheap.c
Line: 1132
Expression: _CrtIsValidHeapPointer(pUserData)

Debug Assertion Failed!
Program......
File: dbgheap.c
Line: 51
Expression: _BLOCK_TYPE_IS_VALID(pHead->nBlockUse)

all three are trigered in that order upon closing, like im deleting something that already was deleted, but It only occurs if the copy constructor is called.


    
CBitmap::CBitmap()
: pucBMP(NULL), bValid(false)
{
}

CBitmap::CBitmap(char* fileName)
: pucBMP(NULL), bValid(false)
{	LoadBitmap(fileName);
}

CBitmap::CBitmap(const CBitmap& obj)
: bitmapInfoHeader(obj.bitmapInfoHeader), pucBMP(NULL), bValid(false)
{	pucBMP = new unsigned char[bitmapInfoHeader.biSizeImage];
	if(pucBMP)
	{	bValid =true;
		std::copy(obj.pucBMP, (obj.pucBMP + bitmapInfoHeader.biSizeImage), pucBMP);
	}
}

CBitmap::~CBitmap()
{	this->Free();
}

void CBitmap::LoadBitmap(char* fileName)
{	if(VerifyFileName(fileName))
	{	ifstream inFile;
		inFile.open(fileName, ios::in | ios::binary);

		if(inFile.good()) // if the file opened

		{	::BITMAPFILEHEADER bitmapFileHeader;
			
			inFile.read(reinterpret_cast<char*>(&bitmapFileHeader),sizeof(::BITMAPFILEHEADER));

			if(!inFile.fail()) // reading file header did not fail

			{	if(bitmapFileHeader.bfType == BITMAP_ID) // file is a bitmap if the UNIVERSAL BITMAP ID 

				{	inFile.read(reinterpret_cast<char*>(&bitmapInfoHeader),sizeof(::BITMAPINFOHEADER));
					if(!inFile.fail())	// reading info header did not fail

					{	pucBMP = new unsigned char[bitmapInfoHeader.biSizeImage];
						
						if(pucBMP) // allocation was successful

						{	inFile.seekg(bitmapFileHeader.bfOffBits,ios::beg);

							if(inFile.good()) // seekg didnt fail

							{	inFile.read(reinterpret_cast<char*>(pucBMP),bitmapInfoHeader.biSizeImage);
								if(inFile.good()) // reading pucBMP did not fail

								{	unsigned char tempRGB;
									for(unsigned int iIndex = 0; iIndex < bitmapInfoHeader.biSizeImage; iIndex+=3)
									{	tempRGB = pucBMP[iIndex];
										pucBMP[iIndex] = pucBMP[iIndex+2];
										pucBMP[iIndex+2] = tempRGB;
									}
									bValid = true;  // the bmp has been loaded so now the obj is valid

								}
								else // reading pucBMP did fail

								{	this->Free();
								}
							}
							else // seekg failed

							{	this->Free();
							}
						} // allocating memory buffer for bitmap data failed

						else
						{	pucBMP = NULL;
						}							
					} // end of reading BITMAPINFOHEADER

				} // end of valid bitmapID

			} // end of reading FILE Header

		} // end of file open

		inFile.close();
	}  // end of valid file name

}

bool CBitmap::VerifyFileName(char* fileName) const
{	bool bValid = true;
	string strFile(fileName);
	if(!strFile.find(".bmp"))
		bValid = false;

	return(bValid);
}

void CBitmap::Free()
{	if(pucBMP)
	{	delete [] pucBMP;
		pucBMP = NULL;
	}
}


[edited by - xds4lx on February 18, 2003 3:05:07 PM]

[edited by - xds4lx on February 18, 2003 4:12:28 PM]
"Computer games don't affect kids; I mean if Pac-Man affected us as kids, we'd all be running around in darkened rooms, munching magic pills and listening to repetitive electronic music." - Kristian Wilson, Nintendo, Inc, 1989
xDS4Lx
xDS4Lx
Ok, std::copy is working, but im still geting the dubug errors i put in my above post. Its like its deleting the object right after its being created, and the other object. But i set the pointers to null if allocation fails and after deleteing them

[edited by - xds4lx on February 18, 2003 4:42:49 PM]
"Computer games don't affect kids; I mean if Pac-Man affected us as kids, we'd all be running around in darkened rooms, munching magic pills and listening to repetitive electronic music." - Kristian Wilson, Nintendo, Inc, 1989
xDS4Lx
xDS4Lx
Using a memory manager i found on flipcode it traced it down to it trying to dealocate memory that was never allocated, it only happens if the copy constructor is called. Does it automatically call the destructor for the const object?
"Computer games don't affect kids; I mean if Pac-Man affected us as kids, we'd all be running around in darkened rooms, munching magic pills and listening to repetitive electronic music." - Kristian Wilson, Nintendo, Inc, 1989
xDS4Lx
xDS4Lx
problem solved. just had to overload operator=
"Computer games don't affect kids; I mean if Pac-Man affected us as kids, we'd all be running around in darkened rooms, munching magic pills and listening to repetitive electronic music." - Kristian Wilson, Nintendo, Inc, 1989
DerekSaw
DerekSaw
Just to add
- you don''t need to check null-ness before delete.
- relying new returning a null when memory allocation failure isn''t really good.
"after many years of singularity, i'm still searching n the event horizon" o
xDS4Lx
xDS4Lx
quote:
Original post by DerekSaw
Just to add
- you don''t need to check null-ness before delete.
- relying new returning a null when memory allocation failure isn''t really good.


um deleting a null pointer is bad, and since the pointers are set to null before attempting memory allocation, they should still be null if allocation fails.
"Computer games don't affect kids; I mean if Pac-Man affected us as kids, we'd all be running around in darkened rooms, munching magic pills and listening to repetitive electronic music." - Kristian Wilson, Nintendo, Inc, 1989
Martee
Martee
No. Deleting a null pointer has no effect. Checking for null before calling delete is redundant.
ReactOS - an Open-source operating system compatible with Windows NT apps and drivers

Topic Locked

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

Sign in to reply to this topic.