Closed
Bug 620354
Opened 14 years ago
Closed 14 years ago
variable "may be used uninitialized" warnings in TextRunWordCache::FinishTextRun
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: Ms2ger, Assigned: Ms2ger)
References
Details
(Whiteboard: [build_warning])
Attachments
(1 file)
1.14 KB,
patch
|
karlt
:
review-
|
Details | Diff | Splinter Review |
Bug 609691 caused uninitialized variable warnings about wordStartsInsideCluster and wordStartsInsideLigature.
Attachment #498714 -
Flags: review?(jfkthame)
Comment 1•14 years ago
|
||
AFAICS, the warnings are incorrect. All uses of those variables are conditional on the aSuccessful flag; if it's true, they are initialized, and if it's false, they are never touched or read at all.
Assignee | ||
Comment 2•14 years ago
|
||
Sure, but my gcc doesn't realize that, and these would hide other, more useful warnings.
Comment 3•14 years ago
|
||
On the other hand, "initializing" them to fake values would prevent a smarter compiler (or a tool such as valgrind) correctly warning us if we were to introduce a codepath where they really are used when uninitialized. When reviewing bug 609691, Karl expressed a preference for doing it this way (see comment #2 there).
Assignee | ||
Comment 4•14 years ago
|
||
However, I believe it's much less likely that Valgrind will catch a bug here than that developers will just ignore all build warnings, some of which pointing to real bugs, due to the noise. Also, I'm expecting developers and reviewers to catch usage of those variables if they're not initialized correctly.
Comment 5•14 years ago
|
||
Comment on attachment 498714 [details] [diff] [review] Patch v1 Switching review to Karl - personally, I don't feel all that strongly one way or the other about this particular case, but I don't want to unilaterally reverse a decision that was explicitly brought up in previous review.
Attachment #498714 -
Flags: review?(jfkthame) → review?(karlt)
Comment 6•14 years ago
|
||
> /home/karl/moz/dev/gfx/thebes/gfxTextRunWordCache.cpp:455: warning: 'wordStartsInsideCluster' may be used uninitialized in this function
> /home/karl/moz/dev/gfx/thebes/gfxTextRunWordCache.cpp:456: warning: 'wordStartsInsideLigature' may be used uninitialized in this function
These seem to merely indicate that gcc hasn't gone to the effort to work out whether or not this variable was used. The compiler could have worked out that these are never used uninitialized merely by analysing the flow of this single function.
Summary: Uninitialized variable warnings in TextRunWordCache::FinishTextRun → variable "may be used uninitialized" warnings in TextRunWordCache::FinishTextRun
Comment 7•14 years ago
|
||
Comment on attachment 498714 [details] [diff] [review] Patch v1 (In reply to comment #4) > However, I believe it's much less likely that Valgrind will catch a bug here > than that developers will just ignore all build warnings, some of which > pointing to real bugs, due to the noise. I don't agree that these warnings will cause developers to ignore all build warnings. There is already plenty of build spew and developers need to know what to look for. "may be used uninitialized" warnings are not as helpful as other warnings. Similar discussions have happened in bug 456150 and elsewhere. e.g. http://kerneltrap.org/node/6591 I'm usually in favour of changing syntax to suppress warnings, but not changing the generated instructions.
Attachment #498714 -
Flags: review?(karlt) → review-
Assignee | ||
Comment 8•14 years ago
|
||
In that case, wontfix?
You need to log in
before you can comment on or make changes to this bug.
Description
•