Re: [PATCH 1/3] dwrite: Don't crash on uninitialized cached and factory fields in dwritefontface_Release

[Nikolay Sivov]

On 27.04.2017 17:03, Piotr Caban wrote:
> The function may be called with the fields uninitialized from
> create_fontface when init_font_data fails.
>
> Signed-off-by: Piotr Caban <piotr(a)codeweavers.com>
> ---
>  dlls/dwrite/font.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

> -        factory_release_cached_fontface(This->cached);
> -        IDWriteFactory4_Release(This->factory);
> +        if (This->cached)
> +            factory_release_cached_fontface(This->cached);
> +        if (This->factory)
> +            IDWriteFactory4_Release(This->factory);

Yes, this makes sense. The problem with this change is that 'cached' and
'factory' are not necessarily NULL on this failure path. I think at this
point it would be better to use heap_alloc_zero(), and remove all
explicit 0 initialization from create_fontface().

By the way, did you check why init_font_data() fails? If it's because
opentype_get_font_familyname() fails, I'm interested in a log, or font
file that causes it.

[PATCH 1/3] dwrite: Don't crash on uninitialized cached and factory fields in dwritefontface_Release

[Piotr Caban]

On 04/27/17 16:18, Nikolay Sivov wrote:
> By the way, did you check why init_font_data() fails? If it's because
> opentype_get_font_familyname() fails, I'm interested in a log, or font
> file that causes it.
> Yes, the opentype_get_font_familyname was failing. Patch 2 fixes the 
> issue for the font I found it with. Do you want the log anyway?

Thanks,
Piotr

[PATCH 1/3] dwrite: Don't crash on uninitialized cached and factory fields in dwritefontface_Release

[Nikolay Sivov]

On 27.04.2017 17:21, Piotr Caban wrote:
> On 04/27/17 16:18, Nikolay Sivov wrote:
> > By the way, did you check why init_font_data() fails? If it's because
> > opentype_get_font_familyname() fails, I'm interested in a log, or font
> > file that causes it.
> > Yes, the opentype_get_font_familyname was failing. Patch 2 fixes the
> > issue for the font I found it with. Do you want the log anyway?

Yes, please send a log, or file itself directly to this address.
> Thanks,
> Piotr

[PATCH 1/3] dwrite: Don't crash on uninitialized cached and factory fields in dwritefontface_Release

[Piotr Caban]

On 04/27/17 16:41, Nikolay Sivov wrote:
> On 27.04.2017 17:21, Piotr Caban wrote:
> > On 04/27/17 16:18, Nikolay Sivov wrote:
> > > By the way, did you check why init_font_data() fails? If it's because
> > > opentype_get_font_familyname() fails, I'm interested in a log, or font
> > > file that causes it.
> > > Yes, the opentype_get_font_familyname was failing. Patch 2 fixes the
> > > issue for the font I found it with. Do you want the log anyway?
>
> Yes, please send a log, or file itself directly to this address.
> Here's the log (I've copied the font to tests and reproduced the issue 
> there, the original log is way to big).