Hi Jean-Jacques,
thanks a lot for your feedback!
2017-06-01 1:54 GMT+02:00 Ceresa Jean-Jacques <
address@hidden>:
> After a glance to your patch example in ticket 166 , to avoid memory leak we need to somes minor changes
> [...]
> fluid_synth_add_default_mod(synth, mod, FLUID_SYNTH_ADD);
>> fluid_mod_delete (mod); // free the modulator to avoid memory leak.
This is the example usage from the ticket you are refering to. And you are right, the modulator can be immediately freed after calling the function. I left out all deallocation from the example, because I thought it's obvious. But especially with an object that is passed into the function, it's not immediately clear that only it's values are copied and that the structure can be thrown away after the call. I'll add that to the function doc string.
> 2) The new API fluid_synth_add_default_mod() store modulator in an internal default_mod[] table.
> As this table is and defined in fluid_synth.h, it will be declared in all modules that include fluid_synth.h. This results in lost of memory.
> It seems that this table is only necessary in the module fluid_synth.c, so it must be declared only once in fluid_synth.c.
> If later this table must be visible to other modules it will be necessary to define the table as external for theses modules.
I'm not quite sure I understand what you mean here. The default_mod list is a member of the fluid_synth_t struct and as such will only get allocated when a new object of that type is created (via new_fluid_synth). The reason it's in the fluid_synth_t struct and not a private member of fluid_synth.c is that I wanted the default modulator list to be local to the synth. That way you can create multiple synths with different default modulator configurations.
Or maybe you mean that it's wasteful to allocate the list with a static length? That I can relate to... the fluid_mod_t structure already has a "next" pointer, so the default module list could be implemented as a dynamic list. That would save allocating around 1000 bytes of memory. I'll work that into the patch.
> This is a good job.
Thanks again! :-)
Cheers,
Marcus