Summary: | &HWND should not be used as OLE_HANDLE* | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alex Christensen <alex.christensen> | ||||||
Component: | WebKit Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Blocker | CC: | bfulgham, commit-queue, roger_fong | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | Windows 8 | ||||||||
Attachments: |
|
Description
Alex Christensen
2014-02-04 15:06:43 PST
Created attachment 223170 [details]
Patch
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.
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? (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! 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 Created attachment 223891 [details]
patch
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.
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. 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> http://trac.webkit.org/changeset/163903 with fixed c-cast and removed duplicate change log entry |