Last Comment Bug 781058 - Fix -Werror=int-to-pointer-cast in dom/
: Fix -Werror=int-to-pointer-cast in dom/
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla17
Assigned To: :Ms2ger
: 781686 (view as bug list)
Depends on: 743573
Blocks: buildwarning fatvals
  Show dependency treegraph
Reported: 2012-08-07 17:36 PDT by Daniel Holbert [:dholbert]
Modified: 2012-08-10 07:09 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Part a: nsPluginNativeWindowGtk2 (4.42 KB, patch)
2012-08-08 03:25 PDT, :Ms2ger
mounir: review+
Details | Diff | Splinter Review
Part b: PluginInstanceParent (947 bytes, patch)
2012-08-08 03:27 PDT, :Ms2ger
mounir: review+
Details | Diff | Splinter Review
Part c: nsDOMClassInfo (1.06 KB, patch)
2012-08-08 03:28 PDT, :Ms2ger
mounir: review+
Details | Diff | Splinter Review

Description Daniel Holbert [:dholbert] 2012-08-07 17:36:44 PDT
Bug 743573 annotated dom/base/ with FAIL_ON_WARNINGS, and there's at least one warning in that directory that's now killing my build  (with --enable-warnings-as-errors):
dom/base/nsDOMClassInfo.cpp: In function ‘void* FlagsToPrivate(PRUint32)’:
dom/base/nsDOMClassInfo.cpp:9069:29: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]

I'm using GCC 4.7 (though IIRC that warning exists in GCC 4.6 as well, and I'd guess we have a decent number of developers using GCC 4.6)
Comment 1 Daniel Holbert [:dholbert] 2012-08-07 17:41:12 PDT
This warning is for this line:
from Bug 549143.

>  return (void *)(flags << 1);
(where flags is a PRUint32)
Comment 2 :Ms2ger 2012-08-08 03:25:21 PDT
Created attachment 650015 [details] [diff] [review]
Part a: nsPluginNativeWindowGtk2

We try to stick an XID into the void* window in NPWindow; this patch keeps the conversion in dedicated functions.
Comment 3 :Ms2ger 2012-08-08 03:27:29 PDT
Created attachment 650016 [details] [diff] [review]
Part b: PluginInstanceParent

setvalue is a horrible API
Comment 4 :Ms2ger 2012-08-08 03:28:04 PDT
Created attachment 650018 [details] [diff] [review]
Part c: nsDOMClassInfo
Comment 5 Daniel Holbert [:dholbert] 2012-08-08 08:38:22 PDT
(At some point, I think uintptr_t wasn't supported by MSVC -- does it compile there now?)
Comment 6 Mounir Lamouri (:mounir) 2012-08-08 09:23:08 PDT
Comment on attachment 650015 [details] [diff] [review]
Part a: nsPluginNativeWindowGtk2

Review of attachment 650015 [details] [diff] [review]:

r=me if you check if static_cast<> is usable instead of reinterpret_cast<>.

::: dom/plugins/base/nsPluginNativeWindowGtk2.cpp
@@ +29,5 @@
>    virtual nsresult CallSetWindow(nsRefPtr<nsNPAPIPluginInstance> &aPluginInstance);
>  private:
> +  void SetWindow(XID aWindow)
> +  {
> +    window = reinterpret_cast<void*>(static_cast<uintptr_t>(aWindow));

I wonder why reinterpret_cast<> instead of static_cast<>... Could you confirm static_cast<> doesn't work here?
Comment 7 Mounir Lamouri (:mounir) 2012-08-08 09:23:44 PDT
Comment on attachment 650016 [details] [diff] [review]
Part b: PluginInstanceParent

Review of attachment 650016 [details] [diff] [review]:

Same thing for reinterpret_cast<>/static_cast<>.
Comment 9 Benoit Jacob [:bjacob] (mostly away) 2012-08-09 09:01:36 PDT
*** Bug 781545 has been marked as a duplicate of this bug. ***
Comment 10 Daniel Holbert [:dholbert] 2012-08-09 09:07:14 PDT
Patch b here didn't finish off PluginInstanceParent.cpp -- a very-similar chunk of code -- with "(void*)drawingModel" -- also appears at line 453 (50 lines after the chunk touched by patch b), and that chunk triggers the same warning/error.
Comment 11 Benoit Jacob [:bjacob] (mostly away) 2012-08-09 09:09:14 PDT
Yep, that's what I ran into in bug 781545. Landed on inbound.

*** This bug has been marked as a duplicate of bug 781545 ***
Comment 12 Daniel Holbert [:dholbert] 2012-08-09 09:12:16 PDT
Yup, just caught that. Thanks!

(Re-resolving as FIXED rather than dupe, since this bug and bug 781545 both had patches land, to fix different (but related) things, and it feels icky to have had patches land on a bug that's marked as a duplicate.)
Comment 13 Benoit Jacob [:bjacob] (mostly away) 2012-08-10 07:09:02 PDT
*** Bug 781686 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.