Bug#718129: Re: Bug#715461: libsdl-mixer1.2: no sf2 sound fonts loaded by default

[Manuel A. Fernandez Montecelo]

2013/8/8 Dominique Dumont <dod at debian.org>:
> On Wednesday 07 August 2013 22:13:49 you wrote:
>> For example, one fix that comes to mind is to change the line in the
>> first patch:
>>
>> char* soundfont_paths =
>> "/usr/share/sounds/sf2/TimGM6mb.sf2:/usr/share/sounds/sf2/FluidR3_GM.sf2";
>>
>> to this:
>>
>> char* soundfont_paths =
>> SDL_strdup("/usr/share/sounds/sf2/TimGM6mb.sf2:/usr/share/sounds/sf2/FluidR3
>> _GM.sf2");
>>
>> What do you think?  Feels less intrusive than having a second patch.
>
> ok to reduce the number of patches.
>
> But the SDL_strdup solution is needlessly complicated and will probably have
> some eyebrows raised very high in the future.
>
> I'd rather see bug-718129-rm-bad-free.patch merged into bug-715461-
> soundfont_paths.patch so as to have one simple, correct patch.

I don't know if my intentions were clear.

I meant to modify the first patch bug-715461-soundfont_paths.patch so
when that variable "soundfont_paths" is assigned, it's done with
SDL_strdup() (it's done in several places in the code --that's where I
got the idea from--, so "it fits"), and remove the second patch
altogether, bug-718129-rm-bad-free.patch.

I think that this fits the "simple, correct patch" idea that you
mention, and I don't see anything complicated about it -- it's just to
assign the variable with dynamic memory, which is the way the rest of
the code thinks that it should be (there are more instances trying to
free memory from this varaible).

The variable can be set by users of the library to use dynamic memory
[1], so removing that SDL_free() is theoretically incorrect -- if it
gets assigned other content in runtime, it would not free it where the
SDL_free() is removed (which is the end of the program, so actually it
shoudn't be that important, bug e.g. valgrind would report it as a
leak).

Still, if anybody thinks that other solutions are preferrable, it's OK
by me -- I have no special interest in pushing this solution over
others.  I volunteer to fix this, no matter the solution chosen, if
nobody else wants.

And Dominique, sorry that I didn't catch this when you asked me, I was
busy at work and couldn't pay full attention to the issue.


Cheers.

[1] music.c, int Mix_SetSoundFonts(const char *paths)

-- 
Manuel A. Fernandez Montecelo <manuel.montezelo at gmail.com>