Closed
Bug 797893
Opened 12 years ago
Closed 12 years ago
ImageBridgeParent.cpp:60:32: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
Categories
(Core :: Graphics: Layers, defect)
Core
Graphics: Layers
Tracking
()
RESOLVED
FIXED
mozilla19
People
(Reporter: dholbert, Assigned: nical)
References
(Blocks 1 open bug, )
Details
Attachments
(1 file)
1.03 KB,
patch
|
dholbert
:
review+
|
Details | Diff | Splinter Review |
Build warning:
{
Warning: -Wsign-compare in /mozilla/gfx/layers/ipc/ImageBridgeParent.cpp: comparison between signed and unsigned integer expressions
/mozilla/gfx/layers/ipc/ImageBridgeParent.cpp:60:32: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
343.27 ShadowLayerChild.cpp
}
That's for this chunk of code:
> 59 int numChildren = ManagedPImageContainerParent().Length();
> 60 for (unsigned int i = 0; i < numChildren; ++i) {
> 61 static_cast<ImageContainerParent*>(
> 62 ManagedPImageContainerParent()[i]
> 63 )->DoStop();
I don't know if ManagedPImageContainerParent().Length(); can return a negative value, but if it can, we're in trouble... If numChildren is negative, it will be converted to a (gigantic) unsigned value for the "i < numChildren" comparison, and we'll iterate up to that gigantic value, indexing into bad memory.
We could fix this potential issue (and the build warning) by making "i" an int instead of an unsigned int. Or instead, maybe we can make numChildren unsigned? (I can't find the ManagedPImageContainerParent() definition, so I don't know if it returns int or unsigned).
Reporter | ||
Comment 1•12 years ago
|
||
Assignee | ||
Comment 2•12 years ago
|
||
Indeed, numChildren should have been unsigned.
Assignee: nobody → nical.bugzilla
Attachment #668602 -
Flags: review?(dholbert)
Reporter | ||
Comment 3•12 years ago
|
||
Comment on attachment 668602 [details] [diff] [review]
Warning fix
Thanks for looking into this!
Looks good to me -- one question, though: what type does ManagedPImageContainerParent() return, and is its Length() definitely unsigned? (an MXR link would be great)
I'd like to be more sure on that before r+'ing this.
Assignee | ||
Comment 4•12 years ago
|
||
Sure, ManagedPImageContainerParent is in the IPDL generated code for PImageBridge. You can find it in objdir/ipc/ipdl/_ipdlheaders/mozilla/layers/PImageBridgeParent.h and at line 162 we see that ManagedPImageContainerParent returns a InfallibleTArray<PImageContainerParent*>& that we can look up on dxr here:http://dxr.lanedo.com/mozilla-central/xpcom/glue/nsTArray.h.html?string=InfallibleTArray#l202 and the Length argument is an uint32_t.
Reporter | ||
Comment 5•12 years ago
|
||
Comment on attachment 668602 [details] [diff] [review]
Warning fix
Great, thanks for clarifying that!
r=me
Attachment #668602 -
Flags: review?(dholbert) → review+
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Comment 6•12 years ago
|
||
Status: NEW → ASSIGNED
Keywords: checkin-needed
Comment 7•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in
before you can comment on or make changes to this bug.
Description
•