[MPlayer-cvslog] r32906 - trunk/gui/skin/font.c

[Clément Bœsch]

On Wed, Feb 16, 2011 at 05:04:01PM +0100, Ingo Brückl wrote:
> Clément Boesch wrote on Wed, 16 Feb 2011 16:38:34 +0100:
> 
> > What about using av_malloc and av_free/av_freep?
> 
> Didn't know about these. (I hardly know the gui stuff yet.)
> 
> If the gui stuff at some point will be separated from MPlayer there should
> not be too much dependencies, so I think it's best not to use these for the
> moment.
> 

Ok :)

> >> +#include "../interface.h"
> 
> > Shouldn't this fixed adding the correct -I?
> 
> Yes, you're right. I'll change this.
> 

Good; ask Diego for this stuff if you're not sure of it. I already
proposed it, but we are available on IRC for quick replies.

> > I know this is not relative to the current patch, but:
> 
> >    bmpFont *Fonts[MAX_FONTS] = {0};
> 
> > is enough.
> 
> Thanks for mentioning. I wasn't aware of that. <shame>
> (Is that C standard or a compiler feature?)

C standard. If any field is initialized, the rest of the array will be
filled with zeros.

> 
> >> +     gfree( (void **) &Fonts[i] );
> 
> > The cast seems useless (yes I know it's like that everywhere).
> 
> As there is no generic pointer-to-pointer type in C, it's to avoid a warning
> without explicit cast.

Mmmh, I didn't look well to the gfree prototype: it should be:

    gfree(void *p);

with a cast into void ** in this function. It will avoid a lot of cast all
over the code.

> [...]
> Thanks for looking over the patch.
> 

np :)

-- 
Clément B.