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

[Steve Lhomme]

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