Closed Bug 577224 Opened 14 years ago Closed 13 years ago

nsCARenderer::DrawSurfaceToCGContext comparison between signed and unsigned integer expressions

Categories

(Core :: Graphics, defect)

All
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla7

People

(Reporter: timeless, Assigned: BenWa)

Details

Attachments

(1 file, 4 obsolete files)

gfx/src/thebes/utils/nsCoreAnimationSupport.mm:
 In static member function ‘static nsresult nsCARenderer::DrawSurfaceToCGContext(CGContext*, nsIOSurface*, CGColorSpace*, int, int, int, int)’:
695: warning: comparison between signed and unsigned integer expressions
697: warning: comparison between signed and unsigned integer expressions
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #456244 - Flags: review?(joshmoz)
Attachment #456244 - Flags: review?(joshmoz) → review?(b56girard)
Comment on attachment 456244 [details] [diff] [review]
patch

Thanks!
Attachment #456244 - Flags: review?(b56girard) → review+
http://hg.mozilla.org/mozilla-central/rev/06cc16f7954e
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Backed out because this can cause plugins to not draw.

http://hg.mozilla.org/mozilla-central/rev/330178c7235b
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #456244 - Attachment is obsolete: true
Assignee: timeless → b56girard
Attached patch Change size_t to int (obsolete) — Splinter Review
Attachment #457725 - Flags: review?
Attachment #457725 - Flags: review? → review?(timeless)
Comment on attachment 457725 [details] [diff] [review]
Change size_t to int

Changed the data type. The first proposed patch caused underflow.
Attachment #457725 - Flags: review?(timeless) → review?(joshmoz)
Comment on attachment 457725 [details] [diff] [review]
Change size_t to int

I don't think that is the right thing to do. You're assigning size_t to a signed int and then passing it off later as size_t, without knowing exactly what size_t is. You could do something like "(aWidth > int(ioWidth - aX))" but it might be better to sanitize, make sure that ioWidth and ioHeight aren't too big for a signed int and that "aX <= ioWidth" (same for height).
Attachment #457725 - Flags: review?(joshmoz) → review-
Attached patch int->size_t, reworked inequality (obsolete) — Splinter Review
Attachment #457725 - Attachment is obsolete: true
Attachment #539838 - Flags: review?(smichaud)
Comment on attachment 539838 [details] [diff] [review]
int->size_t, reworked inequality

This basically looks fine to me.  But do we also need to check that
the values of aX and aY are sane (that they're both positive, and that
aX < ioWidth and aY < ioHeight)?
Attachment #539838 - Attachment is obsolete: true
Attachment #541675 - Flags: review?(smichaud)
Attachment #539838 - Flags: review?(smichaud)
Attachment #541675 - Flags: review?(smichaud) → review+
Keywords: checkin-needed
Added commit message
Attachment #541675 - Attachment is obsolete: true
http://hg.mozilla.org/integration/mozilla-inbound/rev/1cb1bf67357e
Status: REOPENED → ASSIGNED
Flags: in-testsuite-
Keywords: checkin-needed
Whiteboard: [inbound]
Target Milestone: --- → mozilla7
Merged:
http://hg.mozilla.org/mozilla-central/rev/1cb1bf67357e
Status: ASSIGNED → RESOLVED
Closed: 14 years ago13 years ago
Hardware: x86 → All
Resolution: --- → FIXED
Whiteboard: [inbound]
Verified that the following files are updated in mozilla-central repository:
gfx/thebes/nsCoreAnimationSupport.h
gfx/thebes/nsCoreAnimationSupport.mm

Is this enough to verify the fix and mark the bug accordingly?

Thank you!
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: