[MPlayer-dev-eng] [PATCH] fix a memleak on sub_font, it was never freed

Ulion ulion2002 at gmail.com
Thu Dec 27 02:15:02 CET 2007


2007/12/27, Guillaume LECERF <foxcore at gmail.com>:
> 2007/12/26, Ulion <ulion2002 at gmail.com>:
> > Futher review:
> > should not add this:
> > +static int done_freetype(void) { return 0; }
> >
> > I thinks there's no need the #if defined || defined, only
> > done_freetype() need a #ifdef:
> >
> > -#ifdef HAVE_FREETYPE
> > +#if defined(HAVE_FREETYPE) || defined(HAVE_BITMAP_FONT)
> >    current_module="uninit_font";
> >    if (vo_font) free_font_desc(vo_font);
> >    vo_font = NULL;
> > +  if (sub_font) (sub_font);
> > +  sub_font = NULL;
> >    done_freetype();
> >  #endif
>
> Changed.
>
> > And also the double free check, need be fixed here.
> >
> > And because sometimes vo_font and sub_font pointing to the same
> > address, then in the load_font_ft, the free is not safe, right? before
> > call the load_font_ft, you must take good care of your vo_font and
> > sub_font, so you'd better free them before call the load_font_ft or
> > clear one  of them if they have same target address to prevent from
> > double free. Or any other better plan?
>
> I fixed it differently, as I will need two different instances of
> font_dest_t for font and subfont, to deal with the scale factor.
> So here is the corresponding patch.

Looks correct, but you missed another place in mplayer.c assign
sub_font by vo_font.

-- 
Ulion



More information about the MPlayer-dev-eng mailing list