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

[Steve Lhomme]

My point is that relying on specific tools for a specific OS as the sole solution for code safety have proved insufficient so many times.

As for the double free issue that would be harder to debug, it's a non issue since double free would never occur with my solution. Why would you need tools to debug it then?

> On 18 Feb 2019, at 17:38, Rémi Denis-Courmont <remi at remlab.net> wrote:
> 
> Le maanantaina 18. helmikuuta 2019, 17.52.32 EET Steve Lhomme a écrit :
>> On 18/02/2019 16:32, Thomas Guillem wrote:
>>> On Mon, Feb 18, 2019, at 16:11, Steve Lhomme wrote:
>>>> vlc | branch: master | Steve Lhomme <robux4 at ycbcr.xyz> | Mon Feb 18
>>>> 14:50:21 2019 +0100| [4a79e6a4929085cd78024e0c0367c2bc01ab11b4] |
>>>> committer: Steve Lhomme
>>>> 
>>>> decoder: add Destroy helper function for decoder owners
>>>> 
>>>>> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=4a79e6a4929085cd7
>>>>> 8024e0c0367c2bc01ab11b4>> 
>>>> ---
>>>> 
>>>>  include/vlc_codec.h         |  7 +++++++
>>>>  src/input/decoder_helpers.c | 29 +++++++++++++++++++++++++++++
>>>>  src/libvlccore.sym          |  1 +
>>>>  3 files changed, 37 insertions(+)
>>>> 
>>>> diff --git a/include/vlc_codec.h b/include/vlc_codec.h
>>>> index c287953ba7..2042ad2424 100644
>>>> --- a/include/vlc_codec.h
>>>> +++ b/include/vlc_codec.h
>>>> @@ -320,6 +320,13 @@ VLC_API void decoder_AbortPictures( decoder_t
>>>> *dec, bool b_abort );
>>>> 
>>>>  VLC_API void decoder_Init( decoder_t *dec, const es_format_t * );
>>>> 
>>>>  /**
>>>> 
>>>> + * Destroy a decoder and reset the structure.
>>>> + *
>>>> + * To be used by decoder owners.
>>>> + */
>>>> +VLC_API void decoder_Destroy( decoder_t **p_dec );
>>>> +
>>>> +/**
>>>> 
>>>>   * This function queues a single picture to the video output.
>>>>   *
>>>>   * \note
>>>> 
>>>> diff --git a/src/input/decoder_helpers.c b/src/input/decoder_helpers.c
>>>> index 51f7ce8f6d..b60b830123 100644
>>>> --- a/src/input/decoder_helpers.c
>>>> +++ b/src/input/decoder_helpers.c
>>>> @@ -28,6 +28,8 @@
>>>> 
>>>>  #include <vlc_common.h>
>>>>  #include <vlc_codec.h>
>>>> 
>>>> +#include <vlc_meta.h>
>>>> +#include <vlc_modules.h>
>>>> 
>>>>  void decoder_Init( decoder_t *p_dec, const es_format_t *restrict p_fmt
>>>> 
>>>> )
>>>> 
>>>>  {
>>>> 
>>>> @@ -43,3 +45,30 @@ void decoder_Init( decoder_t *p_dec, const
>>>> es_format_t *restrict p_fmt )
>>>> 
>>>>      es_format_Copy( &p_dec->fmt_in, p_fmt );
>>>>      es_format_Init( &p_dec->fmt_out, p_fmt->i_cat, 0 );
>>>> 
>>>>  }
>>>> 
>>>> +
>>>> +static void decoder_Clean( decoder_t *p_dec )
>>>> +{
>>>> +    es_format_Clean( &p_dec->fmt_in );
>>>> +    es_format_Clean( &p_dec->fmt_out );
>>>> +
>>>> +    if ( p_dec->p_description )
>>>> +    {
>>>> +        vlc_meta_Delete(p_dec->p_description);
>>>> +        p_dec->p_description = NULL;
>>>> +    }
>>>> +    if ( p_dec->p_module != NULL )
>>>> +    {
>>>> +        module_unneed(p_dec, p_dec->p_module);
>>>> +        p_dec->p_module = NULL;
>>>> +    }
>>>> +}
>>>> +
>>>> +void decoder_Destroy( decoder_t **p_dec )
>>>> +{
>>>> +    if (*p_dec != NULL)
>>>> +    {
>>>> +        decoder_Clean( *p_dec );
>>>> +        vlc_object_release( *p_dec );
>>>> +        *p_dec = NULL;
>>>> +    }
>>> 
>>> I really prefer to use a decoder_t * here.
>>> There is not many functions in VLC that set the pointer to NULL. This will
>>> add more confusion.
>>> 
>>> 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.
> 
>> A double free doesn't always crash on Windows and there's no ASAN.
> 
> So your point is that because the bug is hard to observe on Windows, it should 
> be made hard to observe everywhere? This is punishing everybody for your 
> personal selection of development environment. What the hell...
> 
> -- 
> 雷米‧德尼-库尔蒙
> http://www.remlab.net/
> 
> 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel