[vlc-devel] [PATCH] lib: assert instead of abort
thomas at gllm.fr
Fri Sep 20 14:22:40 CEST 2019
On Fri, Sep 20, 2019, at 14:13, Rémi Denis-Courmont wrote:
> I completely disagree. When event registration/deregistration fails, you're almost guaranteed a deadlock or some use after free. Much better to abort asap.
In the VLC-IOS case, it was a detach called when no attach was called before. So no possible deadlock or use after-free.
This is the principal cause of crashes on iOS since a long time, the issue had just been fixed on the iOS side but it should also be fixed here. For me, it's like enabling assert in production builds.
> Le 20 septembre 2019 12:21:37 GMT+03:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>> Because you should not abort in production. Developers will still be notified
>> that they are miss-using the API with the assert.
>> PS: this abort was happening on iOS via VLCKit, the API miss-use had been fixed
>> here: https://code.videolan.org/videolan/VLCKit/merge_requests/19 lib/event.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> diff --git a/lib/event.c b/lib/event.c
>> index 28d3d0c232..4a57029b89 100644
>> --- a/lib/event.c
>> +++ b/lib/event.c
>> @@ -179,5 +179,5 @@ void libvlc_event_detach(libvlc_event_manager_t *em, libvlc_event_type_t type,
>> - abort();
>> + assert(!"event not found or already detached");
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the vlc-devel