RWops revamp

NOTE: I’m splitting this off of the SDL_mixer discussion since there are two
parallel topics in that thread.

I sent the message below yesterday, but it was bounced for being too large
with the attachments. I uploaded the diffs to my website:
http://cogwheel.info/SDL_diffs_1.tgz

Here’s an updated overview of the proposal:

There are some resource leaks scattered around (particularly in SDL_mixer)
that arise from confusion over RWops cleanup. By adding a reference counter
and standardizing the rules for dealing with RWops, it will be much easier
to avoid these kinds of problems. Additionally, with a small change to the
structure of RWops, we can make it easily extensible by users and library
writers.

The proposed changes to RWops itself are in SDL.diff. The SDL_image & mixer
diffs show how the changes would be applied to those libraries. The new
rules are quite simple:

  • If you create a new RWops object using one of the SDL_RWFrom____
    functions, you must call SDL_RWclose on it when finished.

  • If you are library writer and you store a user-supplied RWops for later
    use, increment the reference count (++rwop->refs). When the container you
    stored it in is freed, call SDL_RWclose on it.

For an example of the new extensibility feature, see my original message
(search the page for doughnuts)
http://lists.libsdl.org/pipermail/sdl-libsdl.org/2011-April/080525.htmlOn Thu, Apr 14, 2011 at 12:03 PM, Matthew Orlando <@Matthew_Orlando>wrote:

Ok. I changed it to a reference counter and revised my changes to SDL_image
and SDL_mixer. Fresh diffs against the SDL tip are attached.

This GREATLY simplifies the rules for everyone involved.

Library writers: If you persist a client-provided RWops, increase the ref
count.

Everyone: If you create or persist an RWops, close it when finished.

Compare that with the 8 or so paragraphs from my original message
describing how autoclose should be handled. :stuck_out_tongue:

On Thu, Apr 14, 2011 at 9:08 AM, Matthew Orlando <@Matthew_Orlando>wrote:

As I was working on this, I actually thought to myself: “you know, this is
only a couple abstractions away from a reference-counting garbage
collector.” :slight_smile: I was already well into the changes I’ve posted so far,
though, and I wanted to get some feedback before I continued.

I definitely like the idea of requiring the client to close the RWops. It
would eliminate the need for the autoclose parameters entirely while still
enabling safe persistence.

I can’t imagine it being a good idea to provide an RWops to two different
functions that store it persistently (this could just be a failure of my
imagination), but I suppose that should be left up to the library writers
and clients to decide.

Speaking of which, would it be a good idea for the functions that persist
RWops (e.g. the wavestream loader) to reject any that have a reference count

1?

Cheers,

Matthew “Cogwheel” Orlando

On Wed, Apr 13, 2011 at 10:05 PM, Nathaniel J Fries wrote:

Your proposed change to the structure itself is most appropriate. This
would allow an application to, for example, treat an FTP response as a
readable file.

As far as auto-close… it’s a tricky subject. Some would say that it
should always be in the hands of the developer using the library, but then
there’s the fact that this can get quite complicated for him (because of
those recurring use functions you mentioned). I recommend using reference
counting and always requiring the developer to release it once manually, and
after this SDL can take care of it. Essentially the programmer would be
saying “I don’t need to reference this anymore” and SDL will act as a very
specific-purpose garbage collector. [image: Razz]