[SDL] simple SDL_FreeSurface problem

Started by
1 comment, last by Zahlman 17 years, 6 months ago
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.
Advertisement
Heres what you should be doing ( i think ):
SDL_FreeSurface( sprite ); // free the old spritesprite = 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.
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 ); // deallocatesprite = rotozoomSurface(spriteOrig, angle, 1.f, 0); // then reallocate.


[smile]

This topic is closed to new replies.

Advertisement