[webkit-reviews] review granted: [Bug 38158] Fix gcc compiler warnings in chromium platform graphics code : [Attachment 55311] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu May 6 15:45:17 PDT 2010
Eric Seidel <eric at webkit.org> has granted James Robinson
<jamesr at chromium.org>'s request for review:
Bug 38158: Fix gcc compiler warnings in chromium platform graphics code
https://bugs.webkit.org/show_bug.cgi?id=38158
Attachment 55311: Patch
https://bugs.webkit.org/attachment.cgi?id=55311&action=review
------- Additional Comments from Eric Seidel <eric at webkit.org>
+ if (!((format == GraphicsContext3D::RGBA && type ==
GraphicsContext3D::UNSIGNED_BYTE) || (format == m_implementationColorReadFormat
&& type == m_implementationColorReadType))) {
As far as I can tell this is correct. Certainly reads nicer. I would probably
have even used local variables if I had written this originally. :) GCC
warnings FTW.
WebCore/platform/graphics/chromium/FontLinux.cpp:608
+ if (x >= 0 && x < walker.width()) {
I'm curious how, if at all, this changes behavior. Is this testable?
Please add at least a ChangeLog comment explaining why the x >= does not change
behavior and does not require testing. Otherwise this looks good.
More information about the webkit-reviews
mailing list