Sign in to follow this  
theSecondt

[SDL] simple SDL_FreeSurface problem

Recommended Posts

why does this work(but memleaks):
Quote:
temp= rotozoomSurface(spriteOrig, angle, 1.f, 0); sprite= NULL; SDL_FreeSurface(sprite); sprite= temp; temp= NULL; SDL_FreeSurface(temp);
but this doesn't(crashes):
Quote:
temp= rotozoomSurface(spriteOrig, angle, 1.f, 0); SDL_FreeSurface(sprite); sprite= temp; SDL_FreeSurface(temp); temp=NULL;
-all the vars are SDL_Surface*, rotozoom returns SDL_Surface* -spriteOrig is already optimized my problem is with memleak and it lies within this procedure. it seems that if i free the temp surface the app crashes even though it isn't in use or w/e. thanks.

Share this post


Link to post
Share on other sites
Heres what you should be doing ( i think ):

SDL_FreeSurface( sprite ); // free the old sprite
sprite = rotozoomSurface(spriteOrig, angle, 1.f, 0);

The first fails because any surface you have stored in "sprite" is overwritten. You never free it. You should put the sprite = NULL after the SDL_FreeSurface( sprite ). But seeing as you assing a value to sprite 2 lines later its not neccessary to make it NULL, and there is no need for a temporary.

The second fails because you point sprite and temp at the same location. You then free that location, but sprite still points to it. You later on try to use this surface pointer (while drawing with it or something) but it points at invalid data.

Share this post


Link to post
Share on other sites

temp = rotozoomSurface(spriteOrig, angle, 1.f, 0);
sprite = NULL;


OK, 'sprite' now points at nothing.


SDL_FreeSurface(sprite);


Because 'sprite' points at nothing, this doesn't actually free anything.


sprite = temp;


Now 'sprite' points at whatever 'temp' used to point to.


temp = NULL;
SDL_FreeSurface(temp);


Similarly, 'temp' now points at nothing, and nothing got freed. 'sprite' still points at what 'temp' pointed at, but the thing that 'sprite' used to point at is now floating around in memory (i.e. leaked).

So we try again:


temp = rotozoomSurface(spriteOrig, angle, 1.f, 0);
SDL_FreeSurface(sprite);


OK, the thing 'sprite' pointed at gets cleaned up.


sprite = temp;


Now 'sprite' points at the 'temp'.


SDL_FreeSurface(temp);


Now the thing that 'temp' points to is cleaned up. But 'sprite' also points to it (because of 'sprite = temp'), so now 'sprite' points at a deallocated thing (i.e. crash because of dangling pointer).


temp = NULL;


No longer relevant at that point.

Think about it: How many things were allocated by this process? I count one: sprite and spriteOrig presumably already existed, but 'temp' is new. Therefore, how many should we expect to deallocate? Also one. In our case, we want to deallocate whatever 'sprite' was pointing at, and then re-point it at the newly created 'temp'.

If you're doing this in a function where 'temp' is a local variable (probably a good idea), then setting it to NULL (or anything else for that matter) afterwards won't make a difference - past the end of the function, the variable disappears anyway. So really, all we need to do is:


temp = rotozoomSurface(spriteOrig, angle, 1.f, 0);
SDL_FreeSurface(sprite);
sprite = temp;
// Ack, stop! (unless 'temp' has a wider scope and needs to be reset to NULL).
// We created one and destroyed one. That's good.


And when we stare at that a little more, we realize we don't really need the temporary at all: there's no reason the order of allocating and deallocating should matter (if anything, deallocating first would be better, so that we don't temporarily have both things on the heap and possibly increase the peak memory load), and if we deallocate first, we can write it more simply:


// Just like rip-off said:
SDL_FreeSurface( sprite ); // deallocate
sprite = rotozoomSurface(spriteOrig, angle, 1.f, 0); // then reallocate.


[smile]

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