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

[Thomas Guillem]

By the way, if you add a destroy function, you need to add a Create one.
Like this one:
vlc_dec_owner_Create(vlc_object_t *parent, size_t owner_size, const struct decoder_owner_callbacks *);

On Tue, Feb 19, 2019, at 10:09, Rémi Denis-Courmont wrote:
> 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é. 
> _______________________________________________
> 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/20190219/01fc13ae/attachment.html>