Bug 128211 – &HWND should not be used as OLE_HANDLE*

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Alex Christensen]

An OLE_HANDLE is a UINT, which is always a 32-bit integer.  A HWND is a HWND__*, which is 32-bits for Win32 and 64-bits for Win64.  Casting between the two seems to be ok in most cases, and they're often used interchangeably without a problem.  There are several functions in WebKit that take an OLE_HANDLE* to put something into, and we're giving it a casted pointer to a HWND.  This does not cause a problem on Win32 because they're equivalent, but it puts a 32-bit integer at the beginning of a 64-bit integer with my Win64 builds, which causes some crashes sometimes.  This fixes those crashes.

[Alex Christensen]

Created attachment 223170 [details]
Patch

[WebKit Commit Bot]

Attachment 223170 [details] did not pass style-queue:


ERROR: Tools/DumpRenderTree/win/TestRunnerWin.cpp:717:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebKit/win/WebCoreSupport/WebContextMenuClient.cpp:75:  Wrong number of spaces before statement. (expected: 8)  [whitespace/indent] [4]
ERROR: Source/WebKit/win/WebCoreSupport/WebContextMenuClient.cpp:76:  Wrong number of spaces before statement. (expected: 8)  [whitespace/indent] [4]
ERROR: Source/WebKit/win/WebCoreSupport/WebContextMenuClient.cpp:76:  Wrong number of spaces before statement. (expected: 8)  [whitespace/indent] [4]
ERROR: Source/WebKit/win/WebDropSource.cpp:102:  Wrong number of spaces before statement. (expected: 8)  [whitespace/indent] [4]
Total errors found: 5 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alex Christensen]

I'll fix the spacing, but is it ok to use (HWND) style casting like everything else in this code, or should I change it to reinterpret_cast?

[Brent Fulgham]

(In reply to comment #3)
> I'll fix the spacing, but is it ok to use (HWND) style casting like everything else in this code, or should I change it to reinterpret_cast?

You should use static_cast/reinterpret_cast/const_cast, etc., as necessary to be clear about what's going on.

Thanks for working on this!

[Alex Christensen]

Would you be ok with me using helpers like PtrToUInt in windows-specific code?

http://msdn.microsoft.com/en-us/library/windows/desktop/aa384267(v=vs.85).aspx

[Alex Christensen]

Created attachment 223891 [details]
patch

[WebKit Commit Bot]

Attachment 223891 [details] did not pass style-queue:


ERROR: Tools/DumpRenderTree/win/TestRunnerWin.cpp:717:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebKit/win/WebCoreSupport/WebContextMenuClient.cpp:75:  Wrong number of spaces before statement. (expected: 8)  [whitespace/indent] [4]
ERROR: Source/WebKit/win/WebCoreSupport/WebContextMenuClient.cpp:76:  Wrong number of spaces before statement. (expected: 8)  [whitespace/indent] [4]
ERROR: Source/WebKit/win/WebCoreSupport/WebContextMenuClient.cpp:76:  Wrong number of spaces before statement. (expected: 8)  [whitespace/indent] [4]
ERROR: Source/WebKit/win/WebDropSource.cpp:102:  Wrong number of spaces before statement. (expected: 8)  [whitespace/indent] [4]
Total errors found: 5 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alex Christensen]

I think the style failure is a bug, and I think reinterpret_cast is better than PtrToUInt because it does not assume that a OLE_HANDLE is an unsigned int.

[Brent Fulgham]

Comment on attachment 223891 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=223891&action=review

Looks good, but I'd like the one C-cast to be changed to a C++ cast, please!  :-)

> Source/WebKit/win/WebCoreSupport/WebContextMenuClient.cpp:66
> +    OLE_HANDLE nativeMenu = (OLE_HANDLE)menu->platformContextMenu();

reinterpret_cast<OLE_HANDLE>

[Alex Christensen]

http://trac.webkit.org/changeset/163903 with fixed c-cast and removed duplicate change log entry