| Summary: | mutil-char constant warning in BMPImageDecoder.cpp under gcc | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Xan Lopez <xan.lopez> | ||||||||
| Component: | Images | Assignee: | Peter Kasting <pkasting> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | abarth, eric, evan | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Linux | ||||||||||
| Attachments: |
|
||||||||||
Why do you think this is controversial? (In reply to comment #1) > Why do you think this is controversial? Well, at least for me it's not 100% obvious that this is correct, won't break anything, that the old code is not somehow relying on the use of multi-character character constants... That being said the value of multi-character character constants is not guaranteed to be the same across platforms (according to Google), so I think a fix similar to this one (if not this one) should be applied. If you believe the bots this caused: - A crash in GTK+ in test svg/dom/SVGScriptElement/script-load-and-error-events.svg (it works fine in my box). - A layout failure in Qt for test animations/change-keyframes.html. - A crash in Windows Debug for test http/tests/security/aboutBlank/xss-DENIED-navigate-opener-document-write.html (!?). So I've rolled it out. Comment on attachment 33803 [details]
bmpwarning.patch
rolled out, marking r-
Created attachment 34171 [details]
Fix two different warnings
The original patch was sound. I think the test failures encountered must have been due to flakiness or another change. Verified that this still decodes BMPs OK.
I also fixed another warning I just introduced (sorry!).
Created attachment 34172 [details]
Fix two different warnings (correct patch)
Oops, attached wrong patch. I have too many patches in flight :(
|
Created attachment 33803 [details] bmpwarning.patch GCC does not like multi-character character constants, so it produces the following warning for BMPImageDecoder.cpp: CXX WebCore/platform/image-decoders/bmp/libWebCore_la-BMPImageDecoder.lo ../../WebCore/platform/image-decoders/bmp/BMPImageDecoder.cpp:128:16: warning: multi-character character constant The attached patch fixes the warning, but since this might be a bit controversial *and* I can't run the tests to verify that this does not break anything (they all fail with 'timed out' now, something must be broken somewhere...), I'm opening a bug for this.