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

[Andreas Rheinhardt]

Steven Liu:
> 
> 
>> 在 2019年11月29日，13:57，Andreas Rheinhardt <andreas.rheinhardt at gmail.com> 写道：
>>
>> 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)).
> AVFormatContext **? Isn’t that too complex for user of the API caller?

I don't see anything complex in such a parameter. And this is actually
a quite common scenario (just think of av_freep() or
avio_context_free() which both need **-arguments to do their job).

- Andreas