[FFmpeg-devel] [PATCH v1 1/4] avformat/dashenc: remove unused check of avformat_free_context

[Andreas Rheinhardt]

Steven Liu:
> 
> 
>> 在 2019年11月29日，13:40，Andreas Rheinhardt <andreas.rheinhardt at gmail.com> 写道：
>>
>> Jeyapal, Karthick:
>>>
>>> On 11/29/19 10:45 AM, Steven Liu wrote:
>>>> Signed-off-by: Steven Liu <lq at chinaffmpeg.org>
>>>> ---
>>>> libavformat/dashenc.c | 3 +--
>>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
>>>> index a462876c13..8c28fb6b6e 100644
>>>> --- a/libavformat/dashenc.c
>>>> +++ b/libavformat/dashenc.c
>>>> @@ -588,8 +588,7 @@ static void dash_free(AVFormatContext *s)
>>>>                 avio_close(os->ctx->pb);
>>>>         }
>>>>         ff_format_io_close(s, &os->out);
>>>> -        if (os->ctx)
>>>> -            avformat_free_context(os->ctx);
>>>> +        avformat_free_context(os->ctx);
>>> This 'if' condition is a safety net against double frees caused due to any corner case and/or wrong usage.
>>> Unless this is absolutely required for some reason, this 'if' condition should not be removed.
>>
>> This 'if' doesn't help against double-frees at all:
>> avformat_free_context() itself checks against its argument being NULL
>> and returns immediately if it is. (But double frees are still possible
>> given that avformat_free_context can't reset the pointer to NULL due
>> to its signature. Resetting is something the callers have to do for
>> themselves.)
> What about add reset pointer to NULL into the avformat_free_context?

This is pointless, because it will just reset the functions copy of
the pointer to NULL, not the callers pointer to the AVFormatContext.
In order to reset the latter, the parameter would have to be of type
AVFormatContext ** (and the call here would be
avformat_free_context(&os->ctx)).

- Andreas