534150 - Fix "gfxFont.cpp:263: warning: ‘matchFE’ may be used uninitialized in this function"

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Daniel Holbert [:dholbert]]

Attachment: fix: initialize matchFE to null

GCC 4.3.3 on Linux gives us this warning, when building mozilla-central:
> /gfx/thebes/src/gfxFont.cpp: In member function ‘gfxFontEntry* gfxFontFamily::FindFontForStyle(const gfxFontStyle&, PRBool&)’:
> /gfx/thebes/src/gfxFont.cpp:263: warning: ‘matchFE’ may be used uninitialized in this function
(For reference, this warning is in the build log linked in this bug's URL field.)

Not all instances of this type of build warning are actually bugs, but this particular one appears to be.  Relevant snippet of code:

263     gfxFontEntry *matchFE;
[...]
276     for (i = matchBaseWeight; i < 10 && i > 0; i += direction) {
277         if (weightList[i]) {
278             matchFE = weightList[i];
[...]
288 
289     if (!matchFE) {
290         matchFE = weightList[matchBaseWeight];
291     }
[...]
299     return matchFE;
http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/src/gfxFont.cpp#263

So matchFE starts out uninitialized (line 263).  If we find something non-null in any of the weightList[] entries, we'll set matchFE to that value (277-278). Otherwise it stays uninitialized -- no other lines touch its value.  So after the "for" loop, matchFE is either still uninitialized, or it's been set to a non-null value.

Then, we null-check matchFE (line 289).  This check is bogus, because we never allow ourselves to set it to null.  So line 290 will never be hit.

Finally, we return matchFE (line 299), which is potentially bad if it's still uninitialized. (which I presume it might be, given that we thought it was necessary to null-check it at line 289)

I think we want to initialize matchFE to null in the first place.  Line 289-291 would then actually do something -- they'd handle the case where that null value made it through the "for" loop untouched.  Attached patch makes this change.

Comment 1

[John Daggett (:jtd)]

Comment on attachment 417060 [details] [diff] [review]
fix: initialize matchFE to null

The only way unitialized data could end up be used would be if there are no faces in the font, which we should never allow to happen.  

Change is fine but also trim the bogus check (line 289).

Comment 2

[Daniel Holbert [:dholbert]]

Landed, with bogus check removed:
http://hg.mozilla.org/mozilla-central/rev/57f08266001a