Bug 51554 – Compiler warning in double->int conversion in TestNetscapePlugIn

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Evan Martin]

Compiler warning in double->int conversion in TestNetscapePlugIn

[Evan Martin]

Created attachment 77360 [details]
Patch

[Evan Martin]

I'm not sure this is correct, but the npruntime.h docs list JS to NPVariant type mappings, and it says:


        Number                          NPVariantType_Double or NPVariantType_Int32


Does that mean both the doubleValue and the intValue are filled in?

...wait a sec, it's a union, this can't be right.  Let me retry.

[Evan Martin]

Created attachment 77361 [details]
Patch

[Darin Adler]

Comment on attachment 77360 [details]
Patch

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

This change is wrong. I have no idea why the test case works!

> Tools/DumpRenderTree/TestNetscapePlugIn/Tests/NPRuntimeRemoveProperty.cpp:66
> -                int32_t number = arguments[1].value.doubleValue;
> +                int32_t number = arguments[1].value.intValue;

This is incorrect. The code above checks for NPVARIANT_IS_DOUBLE, and it won’t work to just read intValue in that case. You can only read the union member that correctly corresponds to the variant that was used. For example, if you call NPVARIANT_IS_INT32 then you can read intValue.

[Evan Martin]

(I'm hoping the bots try this change for me, which is how I managed to upload the previous change which was totally wrong.)

[Eric Seidel (no email)]

You can force the bots to try a patch with queues.webkit.org/submit-to-ews

[WebKit Review Bot]

Comment on attachment 77361 [details]
Patch

Clearing flags on attachment: 77361

Committed r89202: <http://trac.webkit.org/changeset/89202>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.