Re: gdi32: Ensure child fonts have names

[Dmitry Timoshkov]

"Aric Stewart"  wrote:
> @@ -5792,6 +5792,9 @@ static BOOL load_child_font(GdiFont *font, CHILD_FONT *child)
>     child->font->scale_y = font->scale_y;
>     hfontlist = HeapAlloc(GetProcessHeap(), 0, sizeof(*hfontlist));
>     hfontlist->hfont = CreateFontIndirectW(&font->font_desc.lf);
> - /* ensure the child font has a name */
> - if (!child->font->name)
> -
>    child->font->name = strdupW(child->font->font_desc.lf.lfFaceName);

It doesn't look like an appropriate place for this. Font name is initialized
explicitly in both WineEngCreateFontInstance() and GetEnumStructs(). How it
could be possible that it's NULL in the above case?

gdi32: Ensure child fonts have names

[Aric Stewart]

load_child_font calls neither WineEngCreateFontInstance nor GetEnumStructs.

It allocates the the child font structure above in the function sets the 
few fields it thinks it needs and then goes on. I see no evidence that 
it is being expected to have been set at any other point.

-aric

Dmitry Timoshkov wrote:
> "Aric Stewart"  wrote:
> > @@ -5792,6 +5792,9 @@ static BOOL load_child_font(GdiFont *font, 
> > CHILD_FONT *child)
> >     child->font->scale_y = font->scale_y;
> >     hfontlist = HeapAlloc(GetProcessHeap(), 0, sizeof(*hfontlist));
> >     hfontlist->hfont = CreateFontIndirectW(&font->font_desc.lf);
> > - /* ensure the child font has a name */
> > - if (!child->font->name)
> > -
> >    child->font->name = 
> >
> > strdupW(child->font->font_desc.lf.lfFaceName);
>
> It doesn't look like an appropriate place for this. Font name is 
> initialized
> explicitly in both WineEngCreateFontInstance() and GetEnumStructs(). How it
> could be possible that it's NULL in the above case?

gdi32: Ensure child fonts have names

[Dmitry Timoshkov]

"Aric Stewart"  wrote:
> load_child_font calls neither WineEngCreateFontInstance nor GetEnumStructs.
>
> It allocates the the child font structure above in the function sets the 
> few fields it thinks it needs and then goes on. I see no evidence that 
> it is being expected to have been set at any other point.

Then child->font->name should be initialized explicitly, 'if ()' check
is redundant. Setting it to child->font->font_desc.lf.lfFaceName looks
also incorrect, other places set it to face->family->FamilyName.

gdi32: Ensure child fonts have names

[Aric Stewart]

Thanks, resent.

-aric

Dmitry Timoshkov wrote:
> "Aric Stewart"  wrote:
> > load_child_font calls neither WineEngCreateFontInstance nor 
> > GetEnumStructs.
> >
> > It allocates the the child font structure above in the function sets 
> > the few fields it thinks it needs and then goes on. I see no evidence 
> > that it is being expected to have been set at any other point.
>
> Then child->font->name should be initialized explicitly, 'if ()' check
> is redundant. Setting it to child->font->font_desc.lf.lfFaceName looks
> also incorrect, other places set it to face->family->FamilyName.