Bug 22936 - Uninitialized memory read error in SimpleFontData::unitsPerEm() is reported by Purify
Summary: Uninitialized memory read error in SimpleFontData::unitsPerEm() is reported b...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Dimitri Glazkov (Google)
URL: http://code.google.com/p/chromium/iss...
Keywords:
Depends on:
Blocks:
 
Reported: 2008-12-19 10:02 PST by Dimitri Glazkov (Google)
Modified: 2009-01-12 11:16 PST (History)
0 users

See Also:


Attachments
Initialize m_unitsPerEm in SimpleFontData constructor (1.12 KB, patch)
2008-12-19 10:04 PST, Dimitri Glazkov (Google)
sam: review+
Details | Formatted Diff | Diff
Initialize m_unitsPerEm in SimpleFontData constructor v2 (2.22 KB, patch)
2008-12-19 14:40 PST, Dimitri Glazkov (Google)
darin: review+
Details | Formatted Diff | Diff
Initialize m_unitsPerEm in SimpleFontData constructor v3 (2.31 KB, patch)
2009-01-06 13:03 PST, Dimitri Glazkov (Google)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dimitri Glazkov (Google) 2008-12-19 10:02:34 PST
The easy fix is to initialize m_unitsPerEm on creation.
Comment 1 Dimitri Glazkov (Google) 2008-12-19 10:04:54 PST
Created attachment 26146 [details]
Initialize m_unitsPerEm in SimpleFontData constructor
Comment 2 Darin Adler 2008-12-19 13:19:59 PST
Why is 0 an acceptable value for this?
Comment 3 Dimitri Glazkov (Google) 2008-12-19 13:42:16 PST
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?
Comment 4 Dimitri Glazkov (Google) 2008-12-19 14:40:46 PST
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.
Comment 5 Darin Adler 2009-01-02 10:46:29 PST
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
Comment 6 Dimitri Glazkov (Google) 2009-01-06 13:03:32 PST
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.
Comment 7 Dimitri Glazkov (Google) 2009-01-12 11:16:26 PST
Landed in changeset 39828.

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