warning C4244: '=' : conversion from 'LONG' to 'PRInt8', possible loss of data

RESOLVED FIXED in mozilla21

Status

()

Core
ImageLib
P5
minor
RESOLVED FIXED
13 years ago
5 years ago

People

(Reporter: timeless, Assigned: filidautore)

Tracking

(Blocks: 1 bug)

Trunk
mozilla21
x86
Windows XP
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [build_warning], URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

13 years ago
r:/mozilla\modules\libpr0n\decoders\icon\win\nsIconChannel.cpp(307) : warning 
C4244: '=' : conversion from 'LONG' to 'PRInt8', possible loss of data
r:/mozilla\modules\libpr0n\decoders\icon\win\nsIconChannel.cpp(308) : warning 
C4244: '=' : conversion from 'LONG' to 'PRInt8', possible loss of data

Comment 1

13 years ago
Because of the API used we know that the icon can't have dimensions out of
bounds, right? So we just need to shut up the compiler?
(Reporter)

Updated

12 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P5
QA Contact: imagelib

Updated

6 years ago
Whiteboard: [build_warning]

Updated

6 years ago
Blocks: 187528

Comment 2

6 years ago
Still an issue, though the code has moved:
e:/builds/moz2_slave/m-cen-w32/build/image/decoders/icon/win/nsIconChannel.cpp(561) : warning C4244: '=' : conversion from 'LONG' to 'PRInt8', possible loss of data

e:/builds/moz2_slave/m-cen-w32/build/image/decoders/icon/win/nsIconChannel.cpp(562) : warning C4244: '=' : conversion from 'LONG' to 'PRInt8', possible loss of data
(Assignee)

Comment 3

5 years ago
Created attachment 699195 [details] [diff] [review]
Make the cast explicit

I only made the cast explicit.
Attachment #699195 - Flags: review?(joe)
Comment on attachment 699195 [details] [diff] [review]
Make the cast explicit

Review of attachment 699195 [details] [diff] [review]:
-----------------------------------------------------------------

While this code looks great, I don't know enough about GetDIBits to comment on whether it's actually not going to overflow - i.e., whether we have a latent bug here.
Attachment #699195 - Flags: review?(joe) → review?(netzen)

Comment 5

5 years ago
(In reply to Joe Drew from comment #4)
> While this code looks great, I don't know enough about GetDIBits to comment
> on whether it's actually not going to overflow - i.e., whether we have a
> latent bug here.

Seems legit, the bitmap came from an HICON in the first place.
Comment on attachment 699195 [details] [diff] [review]
Make the cast explicit

Review of attachment 699195 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the patch!

I think GetIconInfo will always return handles to bitmaps that have the correct dimensions but we might as well add some sanity checks while we're here.

Could you add some extra checks here:
http://dxr.mozilla.org/mozilla-central/image/decoders/icon/win/nsIconChannel.cpp.html#l489

colorHeader.biWidth >= 0 && colorHeader.biWidth <= 255 &&
colorHeader.biHeight >= 0 && colorHeader.biHeight <= 255 &&

Re-request review after that and we should be good to land this patch.
Attachment #699195 - Flags: review?(netzen) → feedback+
(Assignee)

Comment 7

5 years ago
Created attachment 700283 [details] [diff] [review]
Make the cast explicit, plus feedback
Attachment #700283 - Flags: review?(netzen)
Comment on attachment 700283 [details] [diff] [review]
Make the cast explicit, plus feedback

Review of attachment 700283 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, I'll run it through try and if it passes we can land it!
Thanks again for the patch.
Attachment #700283 - Flags: review?(netzen) → review+
Attachment #699195 - Attachment is obsolete: true
https://hg.mozilla.org/integration/mozilla-inbound/rev/9c2be62cb0c0
Target Milestone: --- → mozilla21
https://hg.mozilla.org/mozilla-central/rev/9c2be62cb0c0
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Assignee: neil → filidautore
You need to log in before you can comment on or make changes to this bug.