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

[Måns Rullgård]

"Zdenek Kabelac" <zdenek.kabelac at gmail.com> writes:

> 2008/1/25, M?ns Rullg?rd <mans at mansr.com>:
>> Reimar D?ffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de> writes:
>>
>> > On Fri, Jan 25, 2008 at 08:18:01PM +0000, M?ns Rullg?rd wrote:
>> >> "Zdenek Kabelac" <zdenek.kabelac at gmail.com> writes:
>> >>
>> >> > 2008/1/25, Baptiste Coudurier <baptiste.coudurier at smartjog.com>:
>> >> >> Hi,
>> >> >>
>> >> >> Zdenek Kabelac wrote:
>> >> >> > Hi
>> >> >> >
>> >> >> > Here is a memleak patch for the allocation of
>> >> >> > MOVStreamContext in the mov_read_trak.
>> >> >> >
>> >> >> > Also I believe it is save to check the sc pointer prior
>> >> >> > doing dereference - thought I've not checked very deep if
>> >> >> > this combination will never happen.
>> >> >> >
>> >> >>
>> >> >> I think the free can be done in a generic way in
>> >> >> av_close_input_stream, where st is freed.
>> >> >
>> >> > But I think you cannot easily guarantee that priv_data will be always
>> >> > the allocated pointer - it might be some integer value casted to void*
>> >> > - or some other crazy type ??
>> >>
>> >> Only if you wrote the code.  Fortunately for us, you didn't.
>> >
>> > Umm... May I kindly request that you don't overdo it? I mean I'm not too
>> > friendly myself sometimes, but in this case I really think that was not
>> > quite justified, its not like we never had any kind of badly designed
>> > API...
>>
>> I'm not aware of any place where we dereference (or free) a pointer
>> that might not be a valid address.  Doing that is obviously crazy.
>
> 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.

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