No problem. Here are the memory leaks I found so far:
- call SDL_FreeSurface() after drawing the splash screen.
- call Mix_CloseAudio() after freeing the sounds.
- free dispimg in Unit::~Unit()
- call SDL_FreeSurface(bg) in:
AskToQuit(), YesNoBox(), ExitToMenuBox(), GameOverBox(),
AskToQuitGame(), InGameMenu()
(perhaps the background should be part of the GUI?)
Hey I sent a reply to this from the wrong email address, so it is being
screened. It should post soon though. Basically I implemented all this
and uploaded the changes to CVS and the zips I mentioned, and though I
found the cause for the crashes (libpng) I am still leaking on a zoom
cycle (about 2MB per cycle, zoom out twice, then in twice). Further
details on the libpng problem are in the reply I sent the first time.
Looking forward to receiving that.
I didn’t use any of the .dll files you supplied in the first .zip, since you
didn’t supply the required, matching, header files. Maybe that’s why I
couldn’t reproduce the fault.
Another surface leak (these matter if you are using hardware surfaces which
are often a severely limited resource):
void StaticAnim::LoadFrames(std::string sanim)
{
int i;
animbase=sanim;
SDL_Surface *temp;
char send[60];
for(i=1; i<13; i++)
{
sprintf(send,"%s%02d.png",sanim.c_str(), i);
temp=LoadIMAGE(send);
if (anim[i-1]) SDL_FreeSurface(anim[i-1]);
anim[i-1]=rotozoomSurface(temp, 0, zoom, SMOOTH);
SDL_FreeSurface(temp); // <--------------------
if (i==1)
{
w=anim[i-1]->w;
h=anim[i-1]->h;
}
}
//SDL_FreeSurface(temp);
}
BTW: should really be like all the other loops:
void StaticAnim::LoadFrames(std::string sanim)
{
int i;
SDL_Surface *temp;
char send[60]; // shiver … could try using snprintf()?
animbase=sanim;
for(i=0; i<12; i++) // this ‘12’ should be a #define everywhere
{
sprintf(send,"%s%02d.png",sanim.c_str(), i+1); // +1 here only
temp=LoadIMAGE(send);
if (anim[i]) SDL_FreeSurface(anim[i]);
anim[i]=rotozoomSurface(temp, 0, zoom, SMOOTH);
SDL_FreeSurface(temp);
if (i==0)
{
w=anim[i]->w;
h=anim[i]->h;
}
}
}
There are lots more leaks, but I think that’s the last of the easy ones.
Sorry, but the memory allocation schemes in ‘units.cpp’ is too confused for me
to work out what’s wrong with it If I were you, I would consider a quick
re-write, now that you know what you want it to do.
Where the internals of objects are concerned, everytime you allocate a block
of memory or a resource, code up the one and only place where it is freed
(hint: the destructor can call the same function).
You can get rid of the booleans indicating that a resource is loaded by
testing the pointer to the resource directly and making sure that it is
initialised to 0 and set to 0 after it has been freed:
#define UNITS_NUM_FRAMES 72
Unit::Unit()
{
Reset();
}
Unit::~Unit()
{
Delete();
}
void Unit::Reset() // probably should be private
{
int i;
for (i=0; i<UNITS_NUM_FRAMES; i++)
{
mask[i]=0; smask[i]=0; image[i]=0; shield[i]=0;
}
dispimg=0;
…etc…
}
void Unit::Delete() // probably not a great choice of name, unless private
{
int i;
for (i=0; i<UNITS_NUM_FRAMES; i++)
{
if (mask[i]) { bitmask_free(mask[i]); mask[i]=0; }
if (smask[i]) { bitmask_free(smask[i]); smask[i]=0; }
if (image[i]) { SDL_FreeSurface(image[i]); image[i]=0; }
if (shield[i]) { SDL_FreeSurface(shield[i]); shield[i]=0; }
}
if ( dispimg ) { SDL_FreeSurface(dispimg); dispimg=0; }
…etc…
}
bool Unit::IsLoaded(int pic)
{
return bool(image[pic]!=0);
}
bool Unit::ShieldIsLoaded(int pic)
{
return bool(shield[pic]!=0);
}
This (widely-used) technique handles tricky cases where e.g. object are
part-constructed and the clean-up routines are called more than once (see
some Windows GUI programming …),
Good Luck with it,
cheers,
John.On Sunday 28 March 2004 06:18, TomT64 wrote:
-TomT64
SDL mailing list
SDL at libsdl.org
http://www.libsdl.org/mailman/listinfo/sdl