Bug 27851 – mutil-char constant warning in BMPImageDecoder.cpp under gcc

• Component: Images
• Status: RESOLVED FIXED

[Xan Lopez]

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.

[Adam Barth]

Why do you think this is controversial?

[Xan Lopez]

(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.

[Xan Lopez]

I landed this as r46626, closing.

[Xan Lopez]

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.

[Eric Seidel (no email)]

Comment on attachment 33803 [details]
bmpwarning.patch

rolled out, marking r-

[Peter Kasting]

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!).

[Peter Kasting]

Created attachment 34172 [details]
Fix two different warnings (correct patch)

Oops, attached wrong patch.  I have too many patches in flight :(

[Peter Kasting]

Fixed in r46814.