Fixed bugs 1003, 1021, 1168 - fixed memory leak loading music

jjs@jjs.at 2011-03-11 11:37:57 PST When using SDL_Mixer to play WAVE music, the call to Mix_FreeMusic does not close the associated file handle. There is a check in WAVStream_FreeSong() of wavestream.c like this: if ( wave->freerw ) { SDL_FreeRW(wave->rw); } But the variable freerw is not referenced anywhere else. SDL_FreeRW would also not close the file from what I can tell.
libsdl-org · Dec 31, 2011 · ba19e96 · ba19e96
1 parent 5e947c9
commit ba19e96
Show file tree

Hide file tree

Showing 24 changed files with 178 additions and 168 deletions.
diff --git a/CHANGES b/CHANGES
@@ -1,4 +1,6 @@
 1.2.12:
+Sam Lantinga - Sat Dec 31 18:32:05 EST 2011
+ * Fixed memory leak in SDL_LoadMUS()
 Sam Lantinga - Sat Dec 31 10:22:05 EST 2011
  * SDL_mixer is now under the zlib license
 James Le Cuirot - Mon Mar 21 16:54:11 PDT 2011

diff --git a/fluidsynth.c b/fluidsynth.c
@@ -148,9 +148,15 @@ FluidSynthMidiSong *fluidsynth_loadsong(const char *midifile)
 	return fluidsynth_loadsong_common(fluidsynth_loadsong_internal, (void*) midifile);
 }
 
-FluidSynthMidiSong *fluidsynth_loadsong_RW(SDL_RWops *rw)
+FluidSynthMidiSong *fluidsynth_loadsong_RW(SDL_RWops *rw, int freerw)
 {
-	return fluidsynth_loadsong_common(fluidsynth_loadsong_RW_internal, (void*) rw);
+	FluidSynthMidiSong *song;
+
+	song = fluidsynth_loadsong_common(fluidsynth_loadsong_RW_internal, (void*) rw);
+	if (freerw) {
+		SDL_RWclose(rw);
+	}
+	return song;
 }
 
 void fluidsynth_freesong(FluidSynthMidiSong *song)

diff --git a/fluidsynth.h b/fluidsynth.h
@@ -37,7 +37,7 @@ typedef struct {
 
 int fluidsynth_init(SDL_AudioSpec *mixer);
 FluidSynthMidiSong *fluidsynth_loadsong(const char *midifile);
-FluidSynthMidiSong *fluidsynth_loadsong_RW(SDL_RWops *rw);
+FluidSynthMidiSong *fluidsynth_loadsong_RW(SDL_RWops *rw, int freerw);
 void fluidsynth_freesong(FluidSynthMidiSong *song);
 void fluidsynth_start(FluidSynthMidiSong *song);
 void fluidsynth_stop(FluidSynthMidiSong *song);

diff --git a/music.c b/music.c
@@ -1391,6 +1391,7 @@ Mix_Music *Mix_LoadMUS_RW(SDL_RWops *rw)
 	Uint8 moremagic[9];
 	Mix_Music *music;
 	int start;
+	int freerw = 0;
 
 	if (!rw) {
 		Mix_SetError("RWops pointer is NULL");
@@ -1423,7 +1424,7 @@ Mix_Music *Mix_LoadMUS_RW(SDL_RWops *rw)
 	if ( ((strcmp((char *)magic, "RIFF") == 0) && (strcmp((char *)(moremagic+4), "WAVE") == 0)) ||
 	     (strcmp((char *)magic, "FORM") == 0) ) {
 		music->type = MUS_WAV;
-		music->data.wave = WAVStream_LoadSong_RW(rw, (char *)magic);
+		music->data.wave = WAVStream_LoadSong_RW(rw, (char *)magic, freerw);
 		if ( music->data.wave == NULL ) {
 			music->error = 1;
 		}
@@ -1434,7 +1435,7 @@ Mix_Music *Mix_LoadMUS_RW(SDL_RWops *rw)
 	/* Ogg Vorbis files have the magic four bytes "OggS" */
 	if ( strcmp((char *)magic, "OggS") == 0 ) {
 		music->type = MUS_OGG;
-		music->data.ogg = OGG_new_RW(rw);
+		music->data.ogg = OGG_new_RW(rw, freerw);
 		if ( music->data.ogg == NULL ) {
 			music->error = 1;
 		}
@@ -1444,7 +1445,7 @@ Mix_Music *Mix_LoadMUS_RW(SDL_RWops *rw)
 	/* FLAC files have the magic four bytes "fLaC" */
 	if ( strcmp((char *)magic, "fLaC") == 0 ) {
 		music->type = MUS_FLAC;
-		music->data.flac = FLAC_new_RW(rw);
+		music->data.flac = FLAC_new_RW(rw, freerw);
 		if ( music->data.flac == NULL ) {
 			music->error = 1;
 		}
@@ -1470,7 +1471,7 @@ Mix_Music *Mix_LoadMUS_RW(SDL_RWops *rw)
 #ifdef MP3_MAD_MUSIC
 	if ( ( magic[0] == 0xFF && (magic[1] & 0xF0) == 0xF0) || ( strncmp((char *)magic, "ID3", 3) == 0 ) ) {
 		music->type = MUS_MP3_MAD;
-		music->data.mp3_mad = mad_openFileRW(rw, &used_mixer);
+		music->data.mp3_mad = mad_openFileRW(rw, &used_mixer, freerw);
 		if (music->data.mp3_mad == 0) {
 			Mix_SetError("Could not initialize MPEG stream.");
 			music->error = 1;
@@ -1483,7 +1484,7 @@ Mix_Music *Mix_LoadMUS_RW(SDL_RWops *rw)
 		music->type = MUS_MID;
 #ifdef USE_NATIVE_MIDI
 		if ( native_midi_ok ) {
-			music->data.nativemidi = native_midi_loadsong_RW(rw);
+			music->data.nativemidi = native_midi_loadsong_RW(rw, freerw);
 	  		if ( music->data.nativemidi == NULL ) {
 		  		Mix_SetError("%s", native_midi_error());
 			  	music->error = 1;
@@ -1493,7 +1494,7 @@ Mix_Music *Mix_LoadMUS_RW(SDL_RWops *rw)
 #endif
 #ifdef USE_FLUIDSYNTH_MIDI
 		if ( fluidsynth_ok ) {
-			music->data.fluidsynthmidi = fluidsynth_loadsong_RW(rw);
+			music->data.fluidsynthmidi = fluidsynth_loadsong_RW(rw, freerw);
 			if ( music->data.fluidsynthmidi == NULL ) {
 				music->error = 1;
 			}
@@ -1502,7 +1503,7 @@ Mix_Music *Mix_LoadMUS_RW(SDL_RWops *rw)
 #endif
 #ifdef USE_TIMIDITY_MIDI
 		if ( timidity_ok ) {
-			music->data.midi = Timidity_LoadSong_RW(rw);
+			music->data.midi = Timidity_LoadSong_RW(rw, freerw);
 			if ( music->data.midi == NULL ) {
 				Mix_SetError("%s", Timidity_Error());
 				music->error = 1;
@@ -1520,7 +1521,7 @@ Mix_Music *Mix_LoadMUS_RW(SDL_RWops *rw)
 #ifdef MODPLUG_MUSIC
 		if ( music->error ) {
 			music->type = MUS_MODPLUG;
-			music->data.modplug = modplug_new_RW(rw);
+			music->data.modplug = modplug_new_RW(rw, freerw);
 			if ( music->data.modplug ) {
 				music->error = 0;
 			}
@@ -1529,7 +1530,7 @@ Mix_Music *Mix_LoadMUS_RW(SDL_RWops *rw)
 #ifdef MOD_MUSIC
 		if ( music->error ) {
 			music->type = MUS_MOD;
-			music->data.module = MOD_new_RW(rw);
+			music->data.module = MOD_new_RW(rw, freerw);
 			if ( music->data.module ) {
 				music->error = 0;
 			}

diff --git a/music_flac.c b/music_flac.c
@@ -60,7 +60,7 @@ FLAC_music *FLAC_new(const char *file)
 		SDL_SetError ("Couldn't open %s", file);
 		return NULL;
 	}
-	return FLAC_new_RW (rw);
+	return FLAC_new_RW (rw, 1);
 }
 
 static FLAC__StreamDecoderReadStatus flac_read_music_cb(
@@ -313,7 +313,7 @@ static void flac_error_music_cb(
 }
 
 /* Load an FLAC stream from an SDL_RWops object */
-FLAC_music *FLAC_new_RW(SDL_RWops *rw)
+FLAC_music *FLAC_new_RW(SDL_RWops *rw, int freerw)
 {
 	FLAC_music *music;
 	int init_stage = 0;
@@ -327,6 +327,7 @@ FLAC_music *FLAC_new_RW(SDL_RWops *rw)
 		FLAC_setvolume (music, MIX_MAX_VOLUME);
 		music->section = -1;
 		music->rwops = rw;
+		music->freerw = freerw;
 		music->flac_data.max_to_read = 0;
 		music->flac_data.overflow = NULL;
 		music->flac_data.overflow_len = 0;
@@ -375,14 +376,19 @@ FLAC_music *FLAC_new_RW(SDL_RWops *rw)
 				case 1:
 				case 0:
 					free(music);
-					SDL_RWclose(rw);
+					if (freerw) {
+						SDL_RWclose(rw);
+					}
 					break;
 			}
 			return NULL;
 		}
-	}
-	else {
+	} else {
 		SDL_OutOfMemory();
+		if (freerw) {
+			SDL_RWclose(rw);
+		}
+		return NULL;
 	}
 
 	return music;
@@ -556,6 +562,9 @@ void FLAC_delete(FLAC_music *music)
 			free (music->cvt.buf);
 		}
 
+		if (music->freerw) {
+			SDL_RWclose(music->rwops);
+		}
 		free (music);
 	}
 }

diff --git a/music_flac.h b/music_flac.h
@@ -52,6 +52,7 @@ typedef struct {
 	FLAC__StreamDecoder *flac_decoder;
 	FLAC_Data flac_data;
 	SDL_RWops *rwops;
+	int freerw;
 	SDL_AudioCVT cvt;
 	int len_available;
 	Uint8 *snd_available;
@@ -69,7 +70,7 @@ extern void FLAC_setvolume(FLAC_music *music, int volume);
 extern FLAC_music *FLAC_new(const char *file);
 
 /* Load an FLAC stream from an SDL_RWops object */
-extern FLAC_music *FLAC_new_RW(SDL_RWops *rw);
+extern FLAC_music *FLAC_new_RW(SDL_RWops *rw, int freerw);
 
 /* Start playback of a given FLAC stream */
 extern void FLAC_play(FLAC_music *music);

diff --git a/music_mad.c b/music_mad.c
@@ -26,7 +26,8 @@
 #include "music_mad.h"
 
 mad_data *
-mad_openFile(const char *filename, SDL_AudioSpec *mixer) {
+mad_openFile(const char *filename, SDL_AudioSpec *mixer)
+{
   SDL_RWops *rw;
   mad_data *mp3_mad;
 
@@ -35,23 +36,18 @@ mad_openFile(const char *filename, SDL_AudioSpec *mixer) {
 	return NULL;
   }
 
-  mp3_mad = mad_openFileRW(rw, mixer);
-  if (mp3_mad == NULL) {
-	SDL_FreeRW(rw);
-	return NULL;
-  }
-  mp3_mad->freerw = SDL_TRUE;
-  return mp3_mad;
+  return mad_openFileRW(rw, mixer, 1);
 }
 
 mad_data *
-mad_openFileRW(SDL_RWops *rw, SDL_AudioSpec *mixer) {
+mad_openFileRW(SDL_RWops *rw, SDL_AudioSpec *mixer, int freerw)
+{
   mad_data *mp3_mad;
 
   mp3_mad = (mad_data *)malloc(sizeof(mad_data));
   if (mp3_mad) {
 	mp3_mad->rw = rw;
-	mp3_mad->freerw = SDL_FALSE;
+	mp3_mad->freerw = freerw;
 	mad_stream_init(&mp3_mad->stream);
 	mad_frame_init(&mp3_mad->frame);
 	mad_synth_init(&mp3_mad->synth);
@@ -67,13 +63,14 @@ mad_openFileRW(SDL_RWops *rw, SDL_AudioSpec *mixer) {
 }
 
 void
-mad_closeFile(mad_data *mp3_mad) {
+mad_closeFile(mad_data *mp3_mad)
+{
   mad_stream_finish(&mp3_mad->stream);
   mad_frame_finish(&mp3_mad->frame);
   mad_synth_finish(&mp3_mad->synth);
 
   if (mp3_mad->freerw) {
-	SDL_FreeRW(mp3_mad->rw);
+	SDL_RWclose(mp3_mad->rw);
   }
   free(mp3_mad);
 }

diff --git a/music_mad.h b/music_mad.h
@@ -42,7 +42,7 @@ enum {
 
 typedef struct {
   SDL_RWops *rw;
-  SDL_bool freerw;
+  int freerw;
   struct mad_stream stream;
   struct mad_frame frame;
   struct mad_synth synth;
@@ -59,7 +59,7 @@ typedef struct {
 } mad_data;
 
 mad_data *mad_openFile(const char *filename, SDL_AudioSpec *mixer);
-mad_data *mad_openFileRW(SDL_RWops *rw, SDL_AudioSpec *mixer);
+mad_data *mad_openFileRW(SDL_RWops *rw, SDL_AudioSpec *mixer, int freerw);
 void mad_closeFile(mad_data *mp3_mad);
 
 void mad_start(mad_data *mp3_mad);

diff --git a/music_mod.c b/music_mod.c
@@ -152,7 +152,7 @@ MODULE *MOD_new(const char *file)
 		SDL_SetError("Couldn't open %s", file);
 		return NULL;
 	}
-	return MOD_new_RW(rw);
+	return MOD_new_RW(rw, 1);
 }
 
 
@@ -215,18 +215,24 @@ MODULE *MikMod_LoadSongRW(SDL_RWops *rw, int maxchan)
 }
 
 /* Load a MOD stream from an SDL_RWops object */
-MODULE *MOD_new_RW(SDL_RWops *rw)
+MODULE *MOD_new_RW(SDL_RWops *rw, int freerw)
 {
 	MODULE *module;
 
 	/* Make sure the mikmod library is loaded */
 	if ( !Mix_Init(MIX_INIT_MOD) ) {
+		if ( freerw ) {
+			SDL_RWclose(rw);
+		}
 		return NULL;
 	}
 
 	module = MikMod_LoadSongRW(rw,64);
 	if (!module) {
 		Mix_SetError("%s", mikmod.MikMod_strerror(*mikmod.MikMod_errno));
+		if ( freerw ) {
+			SDL_RWclose(rw);
+		}
 		return NULL;
 	}
 
@@ -239,6 +245,10 @@ MODULE *MOD_new_RW(SDL_RWops *rw)
 to query the status of the song or set trigger actions.  Hum. */
 	module->fadeout = 1;
 #endif
+
+	if ( freerw ) {
+		SDL_RWclose(rw);
+	}
 	return module;
 }
 

diff --git a/music_mod.h b/music_mod.h
@@ -42,7 +42,7 @@ extern void MOD_setvolume(struct MODULE *music, int volume);
 extern struct MODULE *MOD_new(const char *file);
 
 /* Load a MOD stream from an SDL_RWops object */
-extern struct MODULE *MOD_new_RW(SDL_RWops *rw);
+extern struct MODULE *MOD_new_RW(SDL_RWops *rw, int freerw);
 
 /* Start playback of a given MOD stream */
 extern void MOD_play(struct MODULE *music);

diff --git a/music_modplug.c b/music_modplug.c
@@ -85,12 +85,12 @@ modplug_data *modplug_new(const char *file)
 		SDL_SetError("Couldn't open %s", file);
 		return NULL;
 	}
-	return modplug_new_RW(rw);
+	return modplug_new_RW(rw, 1);
 
 }
 
 /* Load a modplug stream from an SDL_RWops object */
-modplug_data *modplug_new_RW(SDL_RWops *rw)
+modplug_data *modplug_new_RW(SDL_RWops *rw, int freerw)
 {
 	modplug_data *music=NULL;
 	long offset,sz;
@@ -101,20 +101,35 @@ modplug_data *modplug_new_RW(SDL_RWops *rw)
 	sz = SDL_RWtell(rw)-offset;
 	SDL_RWseek(rw, offset, RW_SEEK_SET);
 	buf=(char*)malloc(sz);
-	if(!buf)
-		return NULL;
-	if(SDL_RWread(rw, buf, sz, 1)==1)
+	if(buf)
 	{
-		music=(modplug_data*)malloc(sizeof(modplug_data));
-		music->playing=0;
-		music->file=ModPlug_Load(buf,sz);
-		if(!music->file)
+		if(SDL_RWread(rw, buf, sz, 1)==1)
 		{
-			free(music);
-			music=NULL;
+			music=(modplug_data*)malloc(sizeof(modplug_data));
+			if (music)
+			{
+				music->playing=0;
+				music->file=ModPlug_Load(buf,sz);
+				if(!music->file)
+				{
+					free(music);
+					music=NULL;
+				}
+			}
+			else
+			{
+				SDL_OutOfMemory();
+			}
 		}
+		free(buf);
+	}
+	else
+	{
+		SDL_OutOfMemory();
+	}
+	if (freerw) {
+		SDL_RWclose(rw);
 	}
-	free(buf);
 	return music;
 }
 

diff --git a/music_modplug.h b/music_modplug.h
@@ -22,7 +22,7 @@ void modplug_setvolume(modplug_data *music, int volume);
 modplug_data *modplug_new(const char *file);
 
 /* Load a modplug stream from an SDL_RWops object */
-modplug_data *modplug_new_RW(SDL_RWops *rw);
+modplug_data *modplug_new_RW(SDL_RWops *rw, int freerw);
 
 /* Start playback of a given modplug stream */
 void modplug_play(modplug_data *music);