Uninitialized memory read error in SimpleFontData::unitsPerEm() is reported by Purify

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Dimitri Glazkov (Google)]

The easy fix is to initialize m_unitsPerEm on creation.

[Dimitri Glazkov (Google)]

Created attachment 26146 [details]
Initialize m_unitsPerEm in SimpleFontData constructor

[Darin Adler]

Why is 0 an acceptable value for this?

[Dimitri Glazkov (Google)]

It's not -- apologies.  There's at least one place where this can cause a div/0 error -- SVGInlineTextBox.cpp(490)

Actually, looking at SVGFontFaceElement.cpp(136) , it appears that the default value is 1000? WDYT?

[Dimitri Glazkov (Google)]

Created attachment 26160 [details]
Initialize m_unitsPerEm in SimpleFontData constructor v2

Darin, thanks again for making me look. I updated the patch to use the value, provided by SVGFontFaceElement::unitsPerEm(), which is what would be used if the font did not have the units_per_em attribute.

[Darin Adler]

Comment on attachment 26160 [details]
Initialize m_unitsPerEm in SimpleFontData constructor v2

> +    const unsigned cDefaultUnitsPerEm = 1000;
> +
>      class CSSFontFaceRule;
>      class CSSMutableStyleDeclaration;
>      class SVGFontElement;

We'd normally put our own declarations after any forward declarations of things defined in other files. So this would go after the "class XXX" declarations.

We also don't use the "c" prefix for constants consistently. We're leaning more towards just normal local-variable-style naming for constants.

r=me despite those quibbles

[Dimitri Glazkov (Google)]

Created attachment 26469 [details]
Initialize m_unitsPerEm in SimpleFontData constructor v3

For ease of landing and cleaner conscience, I updated the patch (v3) to address the quibbles and rebased it against trunk.

[Dimitri Glazkov (Google)]

Landed in changeset 39828.

http://trac.webkit.org/changeset/39828