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

[Rémi Denis-Courmont]

No. I refer to the general case, to the mess that was some of VLC code way back, and to your proposal. It's all the same: it reliably hide bugs in single thread, making code fail silently (as Thomas pointed out), and it unreliably works around races, making them nigh-impossible to debug.

Either way, it makes debugging miserable.

Le 19 février 2019 11:02:27 GMT+02:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
>Again you're talking about defensive programming in general. I agreed 
>already in many cases it's probably wrong. I don't think it's the case
>here.
>
>On 19/02/2019 09:59, Rémi Denis-Courmont wrote:
>> 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é.
>>
>> _______________________________________________
>> 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/6615783e/attachment.html>