From 53d81e78de7606bb95f1896f529533b244238bf5 Mon Sep 17 00:00:00 2001 From: Kp Date: Sat, 24 Sep 2022 17:47:53 +0000 Subject: [PATCH] Fix SDL2 double-free on failure to load music Fixes: 0142c02edd15bb30785013266c82e5781aa19d11 ("Use Mix_LoadMUSType_RW for named files, too") --- common/arch/sdl/digi_mixer_music.cpp | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/common/arch/sdl/digi_mixer_music.cpp b/common/arch/sdl/digi_mixer_music.cpp index 6a7375bf0..b6263718e 100644 --- a/common/arch/sdl/digi_mixer_music.cpp +++ b/common/arch/sdl/digi_mixer_music.cpp @@ -32,12 +32,6 @@ namespace dcx { namespace { -#if SDL_MIXER_MAJOR_VERSION == 2 -#define DXX_SDL_MIXER_MANAGES_RWOPS 1 -#else -#define DXX_SDL_MIXER_MANAGES_RWOPS 0 -#endif - struct Music_delete { void operator()(Mix_Music *m) const @@ -70,11 +64,19 @@ void current_music_t::reset(SDL_RWops *const rw) return; } reset(Mix_LoadMUSType_RW(rw, MUS_NONE, SDL_TRUE)); - if (!*this) - /* If the underlying resource failed to load, then SDL does not - * free the RWops structure. Free it here. + if constexpr (SDL_MIXER_MAJOR_VERSION == 1) + { + /* In SDL_mixer-1, setting freesrc==SDL_TRUE only transfers ownership + * of the RWops structure on success. On failure, the structure is + * still owned by the caller, and must be freed here. + * + * In SDL_mixer-2, setting freesrc==SDL_TRUE always transfers ownership + * of the RWops structure. On failure, SDL_mixer-2 will free the RWops + * before returning, so the structure must not be freed here. */ - SDL_RWclose(rw); + if (!*this) + SDL_RWclose(rw); + } } static current_music_t current_music;