[vlc-devel] [PATCH] lib: assert instead of abort

[Thomas Guillem]

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,
>>               return;
>>           }
>>      }
>> -    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:
> https://mailman.videolan.org/listinfo/vlc-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20190920/b8ebf661/attachment.html>