Closed
Bug 534171
Opened 15 years ago
Closed 15 years ago
nsBulletFrame::AppendCounterText can return an uninitialized value
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
Attachments
(1 file)
(deleted),
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
GCC 4.3.3 on Linux gives us this warning, when building mozilla-central:
> /layout/generic/nsBulletFrame.cpp: In static member function ‘static PRBool nsBulletFrame::AppendCounterText(PRInt32, PRInt32, nsString&)’:
> /layout/generic/nsBulletFrame.cpp:983: warning: ‘success’ may be used uninitialized in this function
Link to code:
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsBulletFrame.cpp#979
The method "AppendCounterText" is basically just a giant switch statement, after which we return the value of |success|. The problem is that |success| gets declared without an initial value, and the first four case clauses don't set it to anything. (while the other cases all set it to the return value of some helper function.)
So if we hit any of those first four clauses, I think AppendCounterText will return whatever random uninitialized value it ends up finding in |success|.
It seems like it'd be simplest & cleanest to just initialize |success| as PR_TRUE when we declare it. We'd initially assume that we'll succeed, since failure is impossible in the simple cases, and then if we try to do something complicated that may fail (i.e. call a helper function), we'll update that assumption by modifying |success|.
(It looks like this is exactly what we did in an older version of this function, "GetListItemText", before bug 3247's patch landed -- but we removed the initialization in that patch for some reason.)
Attachment #417080 -
Flags: review?(dbaron)
Assignee | ||
Updated•15 years ago
|
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Comment 1•15 years ago
|
||
Comment on attachment 417080 [details] [diff] [review]
fix: initialize success to PR_TRUE
r=dbaron
Attachment #417080 -
Flags: review?(dbaron) → review+
Assignee | ||
Comment 2•15 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•