sox icon indicating copy to clipboard operation
sox copied to clipboard

sox_delete_effect is arguably broken

Open erikronstrom opened this issue 11 months ago • 1 comments

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 avatar Feb 26 '24 19:02 erikronstrom

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

erikronstrom avatar Feb 26 '24 19:02 erikronstrom