
<html><head><meta http-equiv="Content-Type" content="text/html; charset=UTF-8"></head><body ><div>As the one who introduced that bug: please go ahead, your solution looks right to me. ;-)</div><div><br></div><div><div style="font-size:75%;color:#575757">Von Samsung Mobile gesendet</div></div> <br><br><br>-------- UrsprÃ¼ngliche Nachricht --------<br>Von: "Manuel A. Fernandez Montecelo" <manuel.montezelo@gmail.com> <br>Datum:  <br>An: Dominique Dumont <dod@debian.org> <br>Cc: Fabian Greffrath <fabian@greffrath.com>,715461@bugs.debian.org,718129@bugs.debian.org <br>Betreff: Re: Re: Bug#715461: libsdl-mixer1.2: no sf2 sound fonts loaded by default <br> <br><br>2013/8/8 Dominique Dumont <dod@debian.org>:<br>> On Wednesday 07 August 2013 22:13:49 you wrote:<br>>> For example, one fix that comes to mind is to change the line in the<br>>> first patch:<br>>><br>>> char* soundfont_paths =<br>>> "/usr/share/sounds/sf2/TimGM6mb.sf2:/usr/share/sounds/sf2/FluidR3_GM.sf2";<br>>><br>>> to this:<br>>><br>>> char* soundfont_paths =<br>>> SDL_strdup("/usr/share/sounds/sf2/TimGM6mb.sf2:/usr/share/sounds/sf2/FluidR3<br>>> _GM.sf2");<br>>><br>>> What do you think?  Feels less intrusive than having a second patch.<br>><br>> ok to reduce the number of patches.<br>><br>> But the SDL_strdup solution is needlessly complicated and will probably have<br>> some eyebrows raised very high in the future.<br>><br>> I'd rather see bug-718129-rm-bad-free.patch merged into bug-715461-<br>> soundfont_paths.patch so as to have one simple, correct patch.<br><br>I don't know if my intentions were clear.<br><br>I meant to modify the first patch bug-715461-soundfont_paths.patch so<br>when that variable "soundfont_paths" is assigned, it's done with<br>SDL_strdup() (it's done in several places in the code --that's where I<br>got the idea from--, so "it fits"), and remove the second patch<br>altogether, bug-718129-rm-bad-free.patch.<br><br>I think that this fits the "simple, correct patch" idea that you<br>mention, and I don't see anything complicated about it -- it's just to<br>assign the variable with dynamic memory, which is the way the rest of<br>the code thinks that it should be (there are more instances trying to<br>free memory from this varaible).<br><br>The variable can be set by users of the library to use dynamic memory<br>[1], so removing that SDL_free() is theoretically incorrect -- if it<br>gets assigned other content in runtime, it would not free it where the<br>SDL_free() is removed (which is the end of the program, so actually it<br>shoudn't be that important, bug e.g. valgrind would report it as a<br>leak).<br><br>Still, if anybody thinks that other solutions are preferrable, it's OK<br>by me -- I have no special interest in pushing this solution over<br>others.  I volunteer to fix this, no matter the solution chosen, if<br>nobody else wants.<br><br>And Dominique, sorry that I didn't catch this when you asked me, I was<br>busy at work and couldn't pay full attention to the issue.<br><br><br>Cheers.<br><br>[1] music.c, int Mix_SetSoundFonts(const char *paths)<br><br>-- <br>Manuel A. Fernandez Montecelo <manuel.montezelo@gmail.com><br><br></body>