Hi all,
I’ve been working to clear up some of the bugs in SDL_mixer:
http://bugzilla.libsdl.org/buglist.cgi?product=SDL_mixer&component=misc&resolution=---
A few of the bugs involving resource leaks (1003, 1021 & 1168) all
seem to stem from the same cause: confusion over the responsibility
for cleaning up after SDL_RWops. Some functions assume the SDL_RWops
should be SDL_RWclosed right away. Some systems store the SDL_RWops
object in another struct for later usage, and of those, some close (or
incorrectly free) the object, and others do nothing. Some functions
provide a way for callers to express their intent (freesrc parameters,
and the like), others do not.
In an effort to to clean this up, I’m trying to standardize and refine
RWops usage. Below is a birds-eye view of my proposal. Any feedback
would be appreciated.---------------------------
Attachments:
SDL.diff - shows changes to the base SDL_RWops functionality
SDL_mixer.diff - shows fixes for bugs 1021 & 1168 following the guidelines below
I’ll be following up with an example of bringing SDL_image into
compliance per Sam’s request
Public API functions that abstract the stream as a whole
(Mix_LoadMUS_RW, IMG_LoadGIF_RW) shall provide an "autoclose"
parameter (not necessarily with that name) indicating that SDL should
take responsibility for closing the RWops. See below for how this
parameter is used
Functions that operate on parts of the stream
(SDL_RWread/write/seek/etc) shall NOT provide that parameter
Internal functions are free to ignore autoclose as appropriate. Just
be sure that it’s observed somewhere up the call stack.
If an autoclose function exits with an error, seek to the beginning
without making any changes to the RWops (NOTE: This overrides any of
the rules below)
The rules for handling the autoclose parameter depend on whether the
function uses the RWops transiently or persistently. Library functions
shall document which is the case.
Functions that use the RWops transiently (e.g. MOD_new_RW):
* When autoclose is true, call SDL_RWclose on the RWops before
exiting the function.
* When autoclose is false, seek to the end and leave the RWops open
Functions that use the RWops persistently (e.g. WAVStream_LoadSong_RW):
* When autoclose is true, set the RWops.autoclose field.
* If autoclose is false, no extra action is necessary on the
Library’s part. However, client code must not close the RWops until
the new container (the WAVStream in the case of WAVStream_LoadSong_RW)
is freed
NOTE: In both cases, it is an error for client code to pass an RWops
that is already flagged for autoclose.
Another shortcoming of the RWops system is that it’s not easily
extensible by 3rd-party libraries. The different types (windows file,
stdio file, pre-allocated memory) are baked into the SDL_RWops struct.
In order to make it easier for library writers to take advantage of
this useful abstraction, I’ve generalized the design a bit.
The base data structure now looks like this:
typedef struct SDL_RWops
{
long (* seek)(<parameters>);
size_t (* read)(<parameters>);
size_t (* write)(<parameters>);
int (* close)(<parameters>);
SDL_bool autoclose;
void *extra;
} SDL_RWops;
The “extra” field replaces the old “hidden” union, whose typedefs are
now internal to SDL_rwops.c. The basic process for creating a new
RWops type looks like this:
typedef struct DoughnutsExtra
{
FILLING filling;
FROSTING frosting;
int count;
} DoughnutsExtra;
/* doughnut_seek/read/write go here */
static int SDLCALL doughnut_close(SDL_RWops *context)
{
if (context) {
/* Release any resources stored *inside* the DoughnutsExtra struct
* where appropriate (this example doesn't use any)
*/
SDL_FreeRW(context);
}
return 0;
}
SDL_RWops *SDL_RWFromDoughnuts(Dougnuts doughnuts)
{
SDL_RWops *rwops = NULL;
Doughnuts *extra = NULL;
rwops = SDL_AllocRW(sizeof(DoughnutsExtra));
if (rwops != NULL) {
rwops->seek = doughnut_seek;
rwops->read = doughnut_read;
rwops->write = doughnut_write;
rwops->close = doughnut_close;
extra = (DoughnutsExtra *)rwops->extra;
extra->filling = FILLING_LEMON;
extra->frosting = FROSTING_GLAZE;
extra->count = BAKERS_DOZEN;
}
return (rwops);
}
If there aren’t any major objections, I’ll keep on this course and
make sure all instances of RWops usage are brought up to date.
–
Matthew Orlando
http://cogwheel.info
-------------- next part --------------
A non-text attachment was scrubbed…
Name: SDL.diff
Type: text/x-patch
Size: 19440 bytes
Desc: not available
URL: http://lists.libsdl.org/pipermail/sdl-libsdl.org/attachments/20110412/7bd665b9/attachment.bin
-------------- next part --------------
A non-text attachment was scrubbed…
Name: SDL_mixer.diff
Type: text/x-patch
Size: 4345 bytes
Desc: not available
URL: http://lists.libsdl.org/pipermail/sdl-libsdl.org/attachments/20110412/7bd665b9/attachment-0001.bin
I was already well into the changes I’ve posted so far,
If you want finer-grained control now, you