See the included patch which fixes the following three functions:
CreateRGBSurfaceFrom (possible segfault)
This function didn't create a private copy of the pixel data,
but rather used whatever pixel data was passed in. This causes
problems when the scalar that is used to store that pixel data
goes out of scope (such as heavy use of 'my' variables under
'use strict'), which could then cause invalid memory accesses
when the pixel data is next accessed. It could also cause
problems if that scalar is changed after a surface is created
from it. This change causes the pixel data to be copied to a
freshly created area and creates the surface from that. It
calculates the size by multiplying the pitch by the height.
Presumably, the pixel data was created with a call to
SurfacePixels (->pixels), which should be considered the only
way to create data to pass to CreateRGBSurfaceFrom.
SurfaceCopy (memcpy possible segfault, reduce memory usage)
This function improperly calculated the size of the pixel data
to copy, using the following formula:
pitch * BytesPerPixel * height
According to the libsdl docs, the pitch is the bytes per scan
line, and includes the BytesPerPixel. So this formula was a
factor of BytesPerPixel too large, and could cause segfaults
when trying to copy areas of memory out of the allowed range. I
believe this formula should just be:
pitch * height
which matches pixel memory calculations in the libsdl source and
in SurfacePixels.
FreeSurface (memory leak)
According to the libsdl docs, surfaces created with calls to
SDL_CreateRGBSurfaceFrom are considered to have their pixel data
"pre-allocated", and SDL_FreeSurface won't free the pixel data
in those cases. The CreateRGBSurfaceFrom and SurfaceCopy
functions above both (now) allocate memory for the pixel data
themselves. FreeSurface was changed to free this self-malloced
area if it should be. The seemingly unintuitive order of frees
and the usage of temporary variables in the patch is due to
http://sdldoc.csn.ul.ie/sdlcreatergbsurfacefrom.php , which says
The pixel data is not copied into the SDL_Surface
structure so it should not be freed until the surface
has been freed with a called to SDL_FreeSurface.
Although I suppose this warning is just to help you avoid
freeing the block through the SDL_Surface structure you just
called SDL_FreeSurface for, which conceivably could cause a
segfault.
Note that I used the memory allocation functions documented in the 5.8.3
perlapi man page (New, Copy and Safefree) rather than the standard
malloc (really safemalloc (which is used all over the SDL_Perl source
and which I can not find a definition for)).
It would be fantastic if this patch could be applied to the official
sources and it would be even fantastic-er if a more recent official
release was made (which also includes my event push changes which were
posted to this list a few months ago). Perhaps 2.1.1? Pretty please,
with sugar on top?
--
Andy Bakun <[EMAIL PROTECTED]>
diff -Naur SDL_Perl-2.1.0/src/SDL.xs SDL_Perl-2.1.0-aab1/src/SDL.xs
--- SDL_Perl-2.1.0/src/SDL.xs 2004-03-25 01:41:23.000000000 -0600
+++ SDL_Perl-2.1.0-aab1/src/SDL.xs 2004-07-01 00:34:25.000000000 -0500
@@ -745,7 +745,11 @@
Uint32 Bmask
Uint32 Amask
CODE:
- RETVAL = SDL_CreateRGBSurfaceFrom ( pixels, width, height,
+ Uint8* newpixels;
+ Uint32 size = pitch * height;
+ New(0, newpixels, size, Uint8);
+ Copy(pixels, newpixels, size, Uint8);
+ RETVAL = SDL_CreateRGBSurfaceFrom ( newpixels, width, height,
depth, pitch, Rmask, Gmask, Bmask, Amask );
OUTPUT:
RETVAL
@@ -767,10 +771,9 @@
SDL_Surface *surface
CODE:
Uint8* pixels;
- Uint32 size = surface->pitch * surface->format->BytesPerPixel*
- surface->h;
- pixels = (Uint8*)safemalloc(size);
- memcpy(pixels,surface->pixels,size);
+ Uint32 size = surface->pitch * surface->h;
+ New(0, pixels, size, Uint8);
+ Copy(surface->pixels, pixels, size, Uint8);
RETVAL = SDL_CreateRGBSurfaceFrom(pixels,surface->w,surface->h,
surface->format->BitsPerPixel, surface->pitch,
surface->format->Rmask, surface->format->Gmask,
@@ -782,7 +785,11 @@
FreeSurface ( surface )
SDL_Surface *surface
CODE:
+ Uint8* pixels = surface->pixels;
+ Uint32 flags = surface->flags;
SDL_FreeSurface(surface);
+ if (flags & SDL_PREALLOC)
+ Safefree(pixels);
Uint32
SurfaceFlags ( surface )
