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

[Ulion]

2007/12/26, Ulion <ulion2002 at gmail.com>:
> 2007/12/26, Guillaume LECERF <foxcore at gmail.com>:
> > 2007/12/24, Ulion <ulion2002 at gmail.com>:
> > > There are more leak on the vo_font and sub_font, they'd better be
> > > fixed all together.
> > > There are two font module could using vo_font and sub_font, one for
> > > bitmap, one for freetype, they share same struct type font_desc_t. The
> > > two font module could be enabled at the same time (normal case), and
> > > the font load by freetype module will override font loaded by bitmap
> > > module without free.
> > > And font loaded by bitmap will not be free when freetype not enabled
> > > (maybe never get free since the freetype could possibly always
> > > override bitmap font), the free function for font_desc_t only
> > > available when freetype enabled, these should all be fixed.
> >
> > Attached patch addresses this.
> > I put free_font_desc in font_load.h for it to be used on both bitmap
> > and freetype fonts.
> > It also frees sub_font (maybe this must be splitted in 2 patches?)
> >
> > > When change subfont size, it reload fonts, and will not do any free
> > > again, etc.
> >
> > load_font_ft() actually frees vo_font before reloading it. Look at
> > line 1102 of font_load_ft.c
>
> I missed that. In the first look, this patch looks much better :)
> But the double free still not checked since the vo_font and sub_font
> could pointing to the same font desc.

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

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?

-- 
Ulion