Memory leak help - Resolved

Started by
11 comments, last by Enigma 15 years, 9 months ago
Ive narrowed down the memory leak to this line of code and cant figure out whats causing it: unscaledData = (unsigned char*)malloc((thisTexture->width*thisTexture->height)*4); The codes used in loading a PCX texture file and the code leading upto that is this:
modelTexture_t *CUtility::LoadPCXTexture(char *filename)
{
	PCXHEADER texInfo;					// header of texture
	modelTexture_t *thisTexture;		// the texture
	unsigned char *unscaledData = 0;	// used to calculate pcx
	int i;								// index counter
	int j;								// index counter
	int width;							// width of texture
	int height;							// height of texture

	// allocate memory for texture struct
	thisTexture = (modelTexture_t*)malloc(sizeof(modelTexture_t));
	if (thisTexture == NULL)
		return NULL;

	// load the PCX file into the texture struct
	thisTexture->data = LoadPCXFile(filename, &texInfo);
	if (thisTexture->data == NULL)
	{
		free(thisTexture->data);
		return NULL;
	}

	// store the texture information
	thisTexture->palette = texInfo.palette;
	thisTexture->width = texInfo.xMax - texInfo.xMin ;
	thisTexture->height = texInfo.yMax - texInfo.yMin ;

	// allocate memory for the unscaled data
	unscaledData = (unsigned char*)malloc((thisTexture->width*thisTexture->height)*4);


Im not sure if this is part of the problem but this is the code to load the actual file:
unsigned char* CUtility::LoadPCXFile(char *filename, PCXHEADER *pcxHeader)
{
	int idx = 0;				// counter index
	int c;						// used to retrieve a char from the file
	int i;						// counter index
	int numRepeat;		
	FILE *filePtr;				// file handle
	int width;					// pcx width
	int height;					// pcx height
	unsigned char *pixelData;	// pcx image data
	unsigned char *paletteData;	// pcx palette data

	// open PCX file
	filePtr = fopen(filename, "rb");
	if (filePtr == NULL)
		return NULL;

	// retrieve first character; should be equal to 10
	c = getc(filePtr);
	if (c != 10)
	{
		fclose(filePtr);
		return NULL;
	}

	// retrieve next character; should be equal to 5
	c = getc(filePtr);
	if (c != 5)
	{
		fclose(filePtr);
		return NULL;
	}

	// reposition file pointer to beginning of file
	rewind(filePtr);

	// read 4 characters of data to skip
	fgetc(filePtr);
	fgetc(filePtr);
	fgetc(filePtr);
	fgetc(filePtr);

	// retrieve leftmost x value of PCX
	pcxHeader->xMin = fgetc(filePtr);		// loword
	pcxHeader->xMin |= fgetc(filePtr) << 8; // hiword

	// retrieve bottom-most y value of PCX
	pcxHeader->yMin = fgetc(filePtr);		// loword
	pcxHeader->yMin |= fgetc(filePtr) << 8;	// hiword

	// retrieve rightmost x value of PCX
	pcxHeader->xMax = fgetc(filePtr);		// loword
	pcxHeader->xMax |= fgetc(filePtr) << 8;	// hiword

	// retrieve topmost y value of PCX
	pcxHeader->yMax = fgetc(filePtr);		// loword
	pcxHeader->yMax |= fgetc(filePtr) << 8;	// hiword

	// calculate the width and height of the PCX
	width = pcxHeader->xMax - pcxHeader->xMin + 1;
	height = pcxHeader->yMax - pcxHeader->yMin + 1;

	// allocate memory for PCX image data
	pixelData = (unsigned char*)malloc(width*height);

	// set file pointer to 128th byte of file, where the PCX image data starts
	fseek(filePtr, 128, SEEK_SET);
	
	// decode the pixel data and store
	while (idx < (width*height))
	{
		c = getc(filePtr);
		if (c > 0xbf)
		{
			numRepeat = 0x3f & c;
			c = getc(filePtr);

			for (i = 0; i < numRepeat; i++)
			{
				pixelData[idx++] = c;
			}
		}
		else
			pixelData[idx++] = c;

		fflush(stdout);
	}

	// allocate memory for the PCX image palette
	paletteData = (unsigned char*)malloc(768);

	// palette is the last 769 bytes of the PCX file
	fseek(filePtr, -769, SEEK_END);

	// verify palette; first character should be 12
	c = getc(filePtr);
	if (c != 12)
	{
		fclose(filePtr);
		return NULL;
	}

	// read and store all of palette
	for (i = 0; i < 768; i++)
	{
		c = getc(filePtr);
		paletteData = c;
	}

	// close file and store palette in header
	fclose(filePtr);
	pcxHeader->palette = paletteData;

	// return the pixel image data
	return pixelData;
}

Any help to get rid of this would be greatly appreciated. [Edited by - Infinite_Daremo on July 1, 2008 5:23:00 AM]
Advertisement
Why are you using malloc in what is clearly C++ code?
modelTexture_t *CUtility::LoadPCXTexture(char *filename){	// allocate memory for texture struct	std::auto_ptr< modelTexture_t > thisTexture(new modelTexture_t());	if (thisTexture == NULL)		return NULL;	// load the PCX file into the texture struct	thisTexture->data = LoadPCXFile(filename, &texInfo);	if (thisTexture->data == NULL)	{		return NULL;	}	// store the texture information	thisTexture->palette = texInfo.palette;	thisTexture->width = texInfo.xMax - texInfo.xMin ;	thisTexture->height = texInfo.yMax - texInfo.yMin ;	// allocate memory for the unscaled data	std::vector< unsigned char > unscaledData((thisTexture->width*thisTexture->height)*4);

There, all memory leaks eliminated.

Σnigma
Quote:Original post by Enigma
Why are you using malloc in what is clearly C++ code?
*** Source Snippet Removed ***
There, all memory leaks eliminated.

Σnigma


If I may add to your fine solution :) ...

Since operator new never returns zero, checks for zero are redundant.

In the event of an error, operator new throws an exception of type std::bad_alloc.
Quite right, I missed that. (I also inadvertantly deleted one too many lines at the top - the declaration of texInfo is still required, albeit not until one line later).

Σnigma
Thanks for the help Enigma but its throwing these errors at me now:

Error 4 error C2440: 'return' : cannot convert from 'std::auto_ptr<_Ty> *__w64 ' to 'modelTexture_t *' c:\users\gavin\documents\visual studio 2008\projects\opengl_game\opengl_game\utility.cpp 728

Error 3 error C2678: binary '==' : no operator found which takes a left-hand operand of type 'std::auto_ptr<_Ty>' (or there is no acceptable conversion) c:\users\gavin\documents\visual studio 2008\projects\opengl_game\opengl_game\utility.cpp 663
Quote:Original post by Infinite_Daremo
Error 4 error C2440: 'return' : cannot convert from 'std::auto_ptr<_Ty> *__w64 ' to 'modelTexture_t *' c:\users\gavin\documents\visual studio 2008\projects\opengl_game\opengl_game\utility.cpp 728


This above error is basically saying that it cannot convert std::auto_ptr<modelTexture_t> to modelTexture*.

This (I suspect) is because you wrote "return thisTexture" at the bottom of CUtility::LoadPCXTexture.

You instead want to write "return thisTexture.release()"

Quote:Original post by Infinite_Daremo
Error 3 error C2678: binary '==' : no operator found which takes a left-hand operand of type 'std::auto_ptr<_Ty>' (or there is no acceptable conversion) c:\users\gavin\documents\visual studio 2008\projects\opengl_game\opengl_game\utility.cpp 663


This has to do with comparisons between std::auto_ptr<modelTexture_t> and zero. std::auto_ptr doesn't define comparison operators. Since those checks are redundant anyways (as I explained earlier), you can just remove them.
Thanks, but im getting an unhandled exception when using .release() and the memory leak is still there at the exact same line.
Quote:Original post by Infinite_Daremo
Thanks, but im getting an unhandled exception when using .release() and the memory leak is still there at the exact same line.


It would help if you would re-post your function in its current state.
It wasnt that i changed my code back to what i originally posted and its still throwing the exception even though no other code has changed. Gonna have to find out whats causing that before i go on.
Finally sorted, thanks for the help thats been driving me insane for 8 hours.

This topic is closed to new replies.

Advertisement