[FFmpeg-devel] [PATCH] libavformat/mov.c memleak bugfix

[Måns Rullgård]

Baptiste Coudurier <baptiste.coudurier at smartjog.com> writes:

> Zdenek Kabelac wrote:
>> 2008/1/26, M?ns Rullg?rd <mans at mansr.com>:
>>> "Zdenek Kabelac" <zdenek.kabelac at gmail.com> writes:
>>>
>> 
>>>> If the avformat.h  specify in the comment that only alllocated memory
>>>> block could be used as a priv_data - than everything is fine and
>>>> release could be done in generic stream_close code -  it is that
>>>> simple....
>>>>
>>>> It's just not set clearly - so the priv_date might be used for
>>>> unpredictable data.
>>> The priv_data pointer in AVStream, which I assume you are referring
>>> to, is managed by each (de)muxer.  Nobody else has any business
>>> touching it.  That's what priv(ate) means.
>> 
>> Of course - that's clear I guess for everyone this list.
>> 
>> But it's upto to the (de)muxer how it will be used - and so far
>> (de)muxer could use it to store anything it needs. When the pointer
>> freeing will be placed into the generic close function and not into
>> the codec - than I would expect this would be somewhere for the
>> (de)muxer writter.
>> 
>> I hope it's now much more clear...
>> 
>
> So far av_write_trailer free priv_data for AVStream, I thought it was
> more consistent to do the same for demuxers.
>
> If people thinks every demuxer should free priv_data, Im ok, that would
> just duplicate some code.

As long as the value is either NULL or a pointer from av_malloc(),
freeing in a common place is safe.  The trouble is if some demuxer
were to store a pointer to static data, or anything else that isn't
valid for av_free().  Apparently that is not being done, though.

I realise that I am probably backtracking on what I said earlier.

-- 
M?ns Rullg?rd
mans at mansr.com