504025 - [windows x64] casting pointer to long is illegal on win64

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[:Mook]

On 64 bit Windows, sizeof(long) is 32 bit and sizeof(void*) is 64 bits.  (Yes, that's pretty ridiculous).

When compiling this with GCC, trying to cast a void* to a long is a fatal error.

Filing this bug as a generic catch-all for these casts

Comment 1

[:Mook]

Attachment: network: nsRequestObserverProxy.cpp - don't cast to long

http://mxr.mozilla.org/mozilla-central/source/netwerk/base/src/nsRequestObserverProxy.cpp?rev=7239b015f833&mark=94,141#91

See comment 0 for rationale.

(Sorry, I was originally going to file one bug per patch, but was told a catch-all bug was better; hence the component was initially set to networking.)

Comment 2

[:Mook]

Attachment: don't cast HANDLE to UINT either [Checkin: Comment 11]

http://mxr.mozilla.org/mozilla-central/source/netwerk/base/src/nsAutodialWin.cpp?rev=7239b015f833&mark=691,740#688

This one's actually casting a HANDLE to a UINT, but it's another "don't cast void* to long" patch :)

Comment 3

[:Mook]

Attachment: cast HINST to LONG_PTR instead of int for comparison with 32 [Checkin: Comment 12]

http://mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/win/nsMIMEInfoWin.cpp?rev=7239b015f833&mark=149,329#146

See comment 0 for details; hInstaApp is a HINST(void* sized) and can't be cast to an int, it needs to be cast to something pointer-sized.  LONG_PTR is that on Windows.

(I'm not sure who to pick for r? on this one, given that it's windows-only code and none of the peers listed seem particularly windows-oriented...)

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 388416 [details] [diff] [review]
network: nsRequestObserverProxy.cpp - don't cast to long

just use %p and remove the cast

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 388418 [details] [diff] [review]
cast HINST to LONG_PTR instead of int for comparison with 32
[Checkin: Comment 12]

I thought I was a peer for this one, but apparently not :)

Comment 6

[:Mook]

Attachment: use %p instead of casts

Thanks for the fast review!

Also, didn't know NSPR's printf supported %p - too used to the intersection of (gcc, msvc) :)

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 388423 [details] [diff] [review]
use %p instead of casts

can you add a space after the comma?

both gcc and msvc support %p, right?

Comment 8

[:Mook]

Attachment: add spaces [Checkin: Comment 13]

carrying forward r+ from bug 388423

... and yes, msvc does have %p.  I _think_ I'll get things right eventually. Sigh.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Which patches need checkin here? All of the non-obsolete ones?

Comment 10

[:Mook]

(In reply to comment #9)
> Which patches need checkin here? All of the non-obsolete ones?

I think so, yes - nothing here touches any interfaces, so by the new rules nothing should need sr.  They also all happen to be trivial patches (<1k each) :)

Comment 11

[Serge Gautherie (:sgautherie)]

Comment on attachment 388417 [details] [diff] [review]
don't cast HANDLE to UINT either
[Checkin: Comment 11]


http://hg.mozilla.org/mozilla-central/rev/29dd8a03ef7d

Comment 12

[Serge Gautherie (:sgautherie)]

Comment on attachment 388418 [details] [diff] [review]
cast HINST to LONG_PTR instead of int for comparison with 32
[Checkin: Comment 12]


http://hg.mozilla.org/mozilla-central/rev/2311880a8e6a

Comment 13

[Serge Gautherie (:sgautherie)]

Comment on attachment 388425 [details] [diff] [review]
add spaces
[Checkin: Comment 13]


http://hg.mozilla.org/mozilla-central/rev/7a25fbd9ab9f