[FFmpeg-soc] [soc]: r5736 - mms/mmst.c

[Benoit Fouet]

On Fri, 9 Apr 2010 00:43:33 +0800 zhentan feng wrote:
> Hi
> 
> On Fri, Apr 9, 2010 at 12:15 AM, Ronald S. Bultje <rsbultje at gmail.com>wrote:
> 
> > Hi,
> >
> > On Thu, Apr 8, 2010 at 12:10 PM, zhentan feng <spyfeng at gmail.com> wrote:
> > > On Thu, Apr 8, 2010 at 9:48 PM, Ronald S. Bultje <rsbultje at gmail.com>
> > wrote:
> > >> On Thu, Apr 8, 2010 at 9:10 AM, zhentan feng <spyfeng at gmail.com> wrote:
> > >> > On Thu, Apr 8, 2010 at 4:43 AM, Aurelien Jacobs <aurel at gnuage.org>
> > >> wrote:
> > >> >> On Wed, Apr 07, 2010 at 06:34:05PM +0200, spyfeng wrote:
> > >> >> > Author: spyfeng
> > >> >> > Date: Wed Apr  7 18:34:05 2010
> > >> >> > New Revision: 5736
> > >> >> >
> > >> >> > Log:
> > >> >> > check av_realloc() failure.
> > >> >> >
> > >> >> > Modified:
> > >> >> >    mms/mmst.c
> > >> >> >
> > >> >> > Modified: mms/mmst.c
> > >> >> >
> > >> >>
> > >>
> > ==============================================================================
> > >> >> > --- mms/mmst.c        Wed Apr  7 18:27:52 2010        (r5735)
> > >> >> > +++ mms/mmst.c        Wed Apr  7 18:34:05 2010        (r5736)
> > >> >> > @@ -321,6 +321,8 @@ static MMSSCPacketType get_tcp_server_re
> > >> >> >                              mms->asf_header =
> > >> >> av_realloc(mms->asf_header,
> > >> >> >                                                mms->asf_header_size
> > >> >> >                                                + mms->pkt_buf_len);
> > >> >> > +                            if (!mms->asf_header)
> > >> >> > +                                return -1;
> > >> >>
> > >> >> memory leak...
> > >> >
> > >> > i fixed it, thanks.
> > >>
> > >> Nope, see man realloc:
> > >>
> > >>     For realloc(), the input pointer is still valid if reallocation
> > failed.
> > >>
> > >> In other words, you should free() mms->asf_header() if realloc
> > >> returned NULL but mms->asf_header (the argument) was non-NULL.
> > >>
> > > yes, you are right. I check the FFmpeg files and found some other places
> > > seems make the same mistakes.
> > > is it necessary to modify them?
> >
> > Uhm, probably. A patch for that would b great, but if you could just
> > point out the places where that happens, it'd help a lot already.
> >
> > for example:
> 1)  libavfilter/avfiltergraph.c  line 39
>     graph->filters = av_realloc(graph->filters,
>                                 sizeof(AVFilterContext*) *
> ++graph->filter_count);
> 
>     if (!graph->filters)
>         return -1;
> 
> 2) libavformat/avien.c line 554
>             idx->cluster = av_realloc(idx->cluster, (cl+1)*sizeof(void*));
>             if (!idx->cluster)
>                 return -1;
> 
> I am not sure these codes are the same situation like mine.
> It just looks similarly.
> 

they are, and from a quick grep, most of the realloc usage are wrong.
At least, don't add another one :)

Ben