Closed Bug 797893 Opened 7 years ago Closed 7 years ago

ImageBridgeParent.cpp:60:32: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

Categories

(Core :: Graphics: Layers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19

People

(Reporter: dholbert, Assigned: nical)

References

(Blocks 1 open bug, )

Details

Attachments

(1 file)

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).
Attached patch Warning fixSplinter Review
Indeed, numChildren should have been unsigned.
Assignee: nobody → nical.bugzilla
Attachment #668602 - Flags: review?(dholbert)
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.
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.
Comment on attachment 668602 [details] [diff] [review]
Warning fix

Great, thanks for clarifying that!

r=me
Attachment #668602 - Flags: review?(dholbert) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b0e7f060ac7a
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in before you can comment on or make changes to this bug.