[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