Last Comment Bug 577224 - nsCARenderer::DrawSurfaceToCGContext comparison between signed and unsigned integer expressions
: nsCARenderer::DrawSurfaceToCGContext comparison between signed and unsigned i...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: All Mac OS X
: -- normal (vote)
: mozilla7
Assigned To: Benoit Girard (:BenWa)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-07-07 02:07 PDT by timeless
Modified: 2011-09-01 06:09 PDT (History)
5 users (show)
Ms2ger: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (1.53 KB, patch)
2010-07-07 02:09 PDT, timeless
b56girard: review+
Details | Diff | Splinter Review
Change size_t to int (675 bytes, patch)
2010-07-15 17:31 PDT, Benoit Girard (:BenWa)
jaas: review-
Details | Diff | Splinter Review
int->size_t, reworked inequality (3.51 KB, patch)
2011-06-16 11:20 PDT, Benoit Girard (:BenWa)
no flags Details | Diff | Splinter Review
int->size_t, reworked inequality v2 (3.65 KB, patch)
2011-06-24 07:25 PDT, Benoit Girard (:BenWa)
smichaud: review+
Details | Diff | Splinter Review
int->size_t, reworked inequality v3 (3.68 KB, patch)
2011-06-24 07:52 PDT, Benoit Girard (:BenWa)
no flags Details | Diff | Splinter Review

Description timeless 2010-07-07 02:07:30 PDT
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
Comment 1 timeless 2010-07-07 02:09:10 PDT
Created attachment 456244 [details] [diff] [review]
patch
Comment 2 Benoit Girard (:BenWa) 2010-07-07 13:39:42 PDT
Comment on attachment 456244 [details] [diff] [review]
patch

Thanks!
Comment 4 Josh Aas 2010-07-09 13:17:11 PDT
Backed out because this can cause plugins to not draw.

http://hg.mozilla.org/mozilla-central/rev/330178c7235b
Comment 5 Benoit Girard (:BenWa) 2010-07-15 17:31:56 PDT
Created attachment 457725 [details] [diff] [review]
Change size_t to int
Comment 6 Benoit Girard (:BenWa) 2010-08-01 15:20:59 PDT
Comment on attachment 457725 [details] [diff] [review]
Change size_t to int

Changed the data type. The first proposed patch caused underflow.
Comment 7 Josh Aas 2010-08-02 14:41:02 PDT
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).
Comment 8 Benoit Girard (:BenWa) 2011-06-16 11:20:55 PDT
Created attachment 539838 [details] [diff] [review]
int->size_t, reworked inequality
Comment 9 Steven Michaud [:smichaud] (Retired) 2011-06-16 13:27:21 PDT
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)?
Comment 10 Benoit Girard (:BenWa) 2011-06-24 07:25:39 PDT
Created attachment 541675 [details] [diff] [review]
int->size_t, reworked inequality v2
Comment 11 Benoit Girard (:BenWa) 2011-06-24 07:52:00 PDT
Created attachment 541682 [details] [diff] [review]
int->size_t, reworked inequality v3

Added commit message
Comment 13 Mounir Lamouri (:mounir) 2011-06-27 02:17:11 PDT
Merged:
http://hg.mozilla.org/mozilla-central/rev/1cb1bf67357e
Comment 14 Mihaela Velimiroviciu (:mihaelav) 2011-09-01 06:09:59 PDT
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!

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