[vlc-devel] [vlc-commits] decoder: add Destroy helper function for decoder owners

Rémi Denis-Courmont remi at remlab.net
Tue Feb 19 09:59:17 CET 2019


You are doing the exact same mistake as Jean-Paul Saman when he sprinkled defensive programming over the code base: incorrectly assuming that VLC is single-threaded.

Le 19 février 2019 10:44:38 GMT+02:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
>There cannot be any double-free occurring in my implementation. That's 
>the class of bugs it removes.
>
>It will of course not solve use after free issues. And I don't see how 
>it makes it easier/harder to track such issues.
>
>On 19/02/2019 09:02, Rémi Denis-Courmont wrote:
>> This does not remove any class of bug. If there is a use-after-free, 
>> it still fails or hides the bug (depending if the use checks for NULL
>
>> or not). If there is a double free race, it hides the bug in most 
>> cases making it very hard to reproduce, all the while not actually 
>> fixing it.
>>
>> Le 19 février 2019 09:13:36 GMT+02:00, Steve Lhomme
><robux4 at ycbcr.xyz> 
>> a écrit :
>>
>>     On 19/02/2019 08:03, Jean-Baptiste Kempf wrote:
>>
>>         Hello, On Tue, 19 Feb 2019, at 08:01, Steve Lhomme wrote:
>>
>>                 I really prefer having a crash (via assert/ASAN) when
>>                 a client is 
>>
>>             misusing an API (calling Destroy with a freed pointer)
>>             instead of ignoring it. This is true when calling Destroy
>>             twice is a bug. It isn't with this 
>>
>>         Then assert() it. 
>>
>>
>>     I'm not sure I'm following. Would you assert when calling
>>     decoder_Destroy() with a NULL decoder (meaning it doesn't exist
>at all,
>>     not even been through decoder_Init) ? Or in the solution I
>removed where
>>     you'd call decoder_Destroy() with a holder than has been emptied,
>which
>>     is not a bug at all.
>>
>>     Or you mean assert when decoder_Destroy() is passed an already
>freed
>>     pointer ? In which case no assert will help detect a double free.
>>
>>     As a more general remark, I think it's odd that rather than
>removing a
>>     class of bugs, we prefer to keep it and rely on tools to find
>them for
>>     us, when they eventually occur.
>>
>>     >
>>
>>         This is not a libVLC API, where you need to deal with invalid
>>         input, and where assert is not enough. -- Jean-Baptiste Kempf
>>         - President +33 672 704 734
>>        
>------------------------------------------------------------------------
>>         vlc-devel mailing list To unsubscribe or modify your
>>         subscription options:
>>         https://mailman.videolan.org/listinfo/vlc-devel 
>>
>>    
>------------------------------------------------------------------------
>>     vlc-devel mailing list
>>     To unsubscribe or modify your subscription options:
>>     https://mailman.videolan.org/listinfo/vlc-devel
>>
>>
>> -- 
>> 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
>
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20190219/b2641198/attachment.html>


More information about the vlc-devel mailing list