[MPlayer-dev-eng] [PATCH][BUG] Incorrect memleak fix code in input/input.c might cause incorrect free

Shachar Raindel shacharr at gmail.com
Mon Aug 2 18:24:21 CEST 2004


Replying to myself, what a joy.....

On Sat, 31 Jul 2004 17:34:02 +0200, Shachar Raindel <shacharr at gmail.com> wrote:
> On Sat, 31 Jul 2004 12:24:48 +0200, Alexander Strasser <eclipse7 at gmx.net> wrote:
> > Hi Shachar,
> > On Mon, Jul 26, 2004 at 05:59:42PM +0200, Shachar Raindel wrote:
> > > > Just a view things:
> > > > >    if( mp_input_parse_config(file)) {
> > > > > -    free(file); // release the buffer created by get_path()
> > > > > +    //    free(file); // release the buffer created by get_path()
> > > > >    }
> > > > >    else {
> > > > This doesn't look to nice, but maybe i'm blind again.
> > > > I would suggest:
> > > >     if( !mp_input_parse_config(file)) {
> > > >
> > > > > +  free(tmpbufptr);
> > > > Maybe the freeing comment should be appended here.
> > > >
> > > >   Alex (beastd)
> > > >
> > >
> > > I agree. The patch was written under time presure, and therefore I
> > > hadn't had time to make it pretty. I should clean-up my tree at home
> > > soon, and send some cleanup patches, so that I will not have so much
> > > CVS merge problems....
> > Do you want to resend a cleaned up patch?
> > If not i'm going to send one and apply it asap,
> > because I want to correct my mistake asap.
> >
> 
> I will try to send a better one later today, but only in the
> evening/night. Feel free to submit a modified version if you have CVS
> write permissions, and please notify me if you submitted, so I won't
> do unneeded work.
> 
And here comes a version of the patch written while I am awake, and
got some time.... Getting ill is a great way to get lots of quality
time with the computer :-) Or maybe it is the other way around?
Getting lots of quality time with the computer is a great way to get
ill?  ;-) Anyway, it just mean I got mono, so I can't go anyway, so I
just sit and hack all day long...).

The patch this time is much more clean. I think I found the most
elegant solution - free file only if it is not the same as
config_file, and remove the weird stuff in the if added by alex. This
should do the trick, without any unneeded memory allocations (and BTW,
I was wrong about the ternary operator - only one of the expressions
is evaluate, so when one writes "a?b:c" than a is evaluated, and
afterward either b or c, but not both).

   Cheers,
   Shachar
-------------- next part --------------
A non-text attachment was scrubbed...
Name: input-memleak-patch-2nd.diff
Type: text/x-patch
Size: 870 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20040802/600ec14d/attachment.bin>


More information about the MPlayer-dev-eng mailing list