nsCARenderer::DrawSurfaceToCGContext comparison between signed and unsigned integer expressions

RESOLVED FIXED in mozilla7

Status

()

Core
Graphics
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: timeless, Assigned: BenWa)

Tracking

Trunk
mozilla7
All
Mac OS X
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

7 years ago
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
(Reporter)

Comment 1

7 years ago
Created attachment 456244 [details] [diff] [review]
patch
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #456244 - Flags: review?(joshmoz)

Updated

7 years ago
Attachment #456244 - Flags: review?(joshmoz) → review?(b56girard)
(Assignee)

Comment 2

7 years ago
Comment on attachment 456244 [details] [diff] [review]
patch

Thanks!
Attachment #456244 - Flags: review?(b56girard) → review+
(Reporter)

Comment 3

7 years ago
http://hg.mozilla.org/mozilla-central/rev/06cc16f7954e
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Comment 4

7 years ago
Backed out because this can cause plugins to not draw.

http://hg.mozilla.org/mozilla-central/rev/330178c7235b
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Updated

7 years ago
Attachment #456244 - Attachment is obsolete: true

Updated

7 years ago
Assignee: timeless → b56girard
(Assignee)

Comment 5

7 years ago
Created attachment 457725 [details] [diff] [review]
Change size_t to int
Attachment #457725 - Flags: review?
(Assignee)

Updated

7 years ago
Attachment #457725 - Flags: review? → review?(timeless)
(Assignee)

Comment 6

7 years ago
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 7

7 years ago
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-
(Assignee)

Comment 8

6 years ago
Created attachment 539838 [details] [diff] [review]
int->size_t, reworked inequality
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)?
(Assignee)

Comment 10

6 years ago
Created attachment 541675 [details] [diff] [review]
int->size_t, reworked inequality v2
Attachment #539838 - Attachment is obsolete: true
Attachment #541675 - Flags: review?(smichaud)
Attachment #539838 - Flags: review?(smichaud)
Attachment #541675 - Flags: review?(smichaud) → review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
(Assignee)

Comment 11

6 years ago
Created attachment 541682 [details] [diff] [review]
int->size_t, reworked inequality v3

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
Last Resolved: 7 years ago6 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.