nsWindow.cpp compilation failure on mingw after landing bug 586228

RESOLVED FIXED

Status

()

Core
Widget: Win32
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Jacek Caban, Assigned: Jacek Caban)

Tracking

Trunk
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

8 years ago
There are two problems:
- mingw doesn't like function to void* implicit casts as a function argument

- On win64 we loose precision here:
  int windowStatus =
   reinterpret_cast<int>(GetPropW(hWnd, kManageWindowInfoProperty));
It's an error instead of warning on mingw-w64. Casting to LONG_PTR instead of int fixes it (then the value is implicitly casted to int anyways).

I've also fixed a warning about passing NULL as an int argument.
(Assignee)

Comment 1

8 years ago
Created attachment 490380 [details] [diff] [review]
fix v1.0
Assignee: nobody → jacek
Attachment #490380 - Flags: review?(vladimir)
Comment on attachment 490380 [details] [diff] [review]
fix v1.0

>diff --git a/widget/src/windows/nsWindow.cpp b/widget/src/windows/nsWindow.cpp
>index cb5bd30..05035da 100644
>--- a/widget/src/windows/nsWindow.cpp
>+++ b/widget/src/windows/nsWindow.cpp
>@@ -1992,7 +1992,7 @@ GetWindowInfoHook(HWND hWnd, PWINDOWINFO pwi)
>     return FALSE;
>   }
>   int windowStatus = 
>-    reinterpret_cast<int>(GetPropW(hWnd, kManageWindowInfoProperty));
>+    reinterpret_cast<LONG_PTR>(GetPropW(hWnd, kManageWindowInfoProperty));

Erm, this doesn't make sense -- the assignment is to an int, how is reinterpret_cast<LONG_PTR> correct?
(Assignee)

Comment 3

8 years ago
(In reply to comment #2)
> Erm, this doesn't make sense -- the assignment is to an int, how is
> reinterpret_cast<LONG_PTR> correct?

This way we have in fact two casts here. The first is done by reinterpret_cast. GetPropW returns HANDLE type, which is typedefed to void*. We do void* to int cast that loses precision and it's an error for GCC. Casting to LONG_PTR changes pointer to integer without losing precision.

The second cast is implicit from LONG_PTR to int. It loses precision, but its fine since both values are integers.
(Assignee)

Comment 4

8 years ago
vlad, reading again, my explanation could be not clean enough.

No we have one cast:
reinterpret_cast from HANDLE (void*) to int  <-- invalid, losing precision

With my patch:
reinterpret_cast from HANDLE (void*) to LONG_PTR  <-- good, doesn't lose precision
implicit cast LONG_PTR to int   <-- good, it's valid for different int type casting
(Assignee)

Comment 5

8 years ago
vlad, ping
(Assignee)

Updated

8 years ago
Attachment #490380 - Flags: review?(roc)
Attachment #490380 - Flags: review?(vladimir)
Attachment #490380 - Flags: review?(roc)
Attachment #490380 - Flags: review+
(Assignee)

Updated

8 years ago
Attachment #490380 - Flags: approval2.0?
Attachment #490380 - Flags: approval2.0? → approval2.0+
(Assignee)

Comment 6

8 years ago
Thanks for review, pushed:

http://hg.mozilla.org/mozilla-central/rev/e21c1c2813ad
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Depends on: 629695
You need to log in before you can comment on or make changes to this bug.