Summary: | [WK2] Regression(r131990): plugins/npruntime/remove-property.html started failing | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||
Component: | WebKit2 | Assignee: | Anders Carlsson <andersca> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | andersca, ap, kling, ossy, zan | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 99865 | ||||||
Attachments: |
|
Description
Chris Dumez
2012-10-22 04:28:18 PDT
Created attachment 169956 [details]
Patch
Comment on attachment 169956 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169956&action=review > Source/WebKit2/ChangeLog:9 > + a temporary to be created which the number was then decoded into. Is there a way to prevent this from happening in the future? (In reply to comment #3) > (From update of attachment 169956 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=169956&action=review > > > Source/WebKit2/ChangeLog:9 > > + a temporary to be created which the number was then decoded into. > > Is there a way to prevent this from happening in the future? Yeah, I'm going to remove the overload. Comment on attachment 169956 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169956&action=review > Source/WebKit2/Shared/Plugins/NPIdentifierData.cpp:76 > - encoder->encode(static_cast<int32_t>(m_number)); > + encoder->encode(m_number); Why are all the other encode casts still a good idea and this one not? > Source/WebKit2/Shared/Plugins/NPIdentifierData.cpp:87 > - return decoder->decode(static_cast<int32_t>(result.m_number)); > + return decoder->decode(result.m_number); This cast is clearly wrong, but can’t we create an explicit temporary of the right type, instead? Was the whole idea of using these type casts a flawed one? Why didn’t the compiler give an error, since we were passing a temporary as a non-const reference argument? Is that a compiler bug? Committed r132120: <http://trac.webkit.org/changeset/132120> (In reply to comment #5) > (From update of attachment 169956 [details]) > Why didn’t the compiler give an error, since we were passing a temporary as a non-const reference argument? Is that a compiler bug? The ArgumentDecoder overload takes a const reference and casts away the constness. I'm going to get rid of that overload. (In reply to comment #5) > (From update of attachment 169956 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=169956&action=review > > > Source/WebKit2/Shared/Plugins/NPIdentifierData.cpp:76 > > - encoder->encode(static_cast<int32_t>(m_number)); > > + encoder->encode(m_number); > > Why are all the other encode casts still a good idea and this one not? > I think that the ones that cast between int32_t and int are completely unnecessary. |