cbagwell / sox

SoX, Swiss Army knife of sound processing
http://sox.sourceforge.net
Other
42 stars 48 forks source link

sox_delete_effect is arguably broken #9

Open erikronstrom opened 9 months ago

erikronstrom commented 9 months ago

While sox_delete_effect is used internally and works correctly in that context, it is not usable from user code:

// This will double-free e->priv when chain is deleted
sox_effect_t * e = sox_create_effect(sox_find_effect("input"));
sox_add_effect(chain, e, in, out);
sox_delete_effect(e);

// This is the way to do it according to the examples.
// However it is not possible from user code when libsox is dynamically linked (at least on Windows)
sox_effect_t * e = sox_create_effect(sox_find_effect("input"));
sox_add_effect(chain, e, in, out);
free(e);  // memory could have been allocated on another heap

// And by the way, this (admittedly not very useful example)
// will leak memory since e->priv is never freed
sox_effect_t * e = sox_create_effect(sox_find_effect("input"));
sox_delete_effect(e);

sox_delete_effect should probably be fixed so that it works as a proper counterpart to sox_create_effect. As a plan B, it could be removed from the public API to avoid confusion (but that way there would still be no way to use the effects chain from user code without leaking memory).

erikronstrom commented 9 months ago

Since this repo doesn't seem to be maintained, I won't bother to create a pull request. Should anyone be interested, a fix is available here: https://github.com/cbagwell/sox/commit/f785535d35c0fe95f254d4c1ecfa135d10924dd7