Fix gcc compiler warnings in chromium platform graphics code
Created attachment 54360 [details] Patch
Created attachment 54361 [details] patch that will pass checkstyle
The warnings are from gcc 4.4.1-4ubuntu9, which is what is currently shipping on Karmic, with -Wall -Werror.
Comment on attachment 54361 [details] patch that will pass checkstyle Clearing flags on attachment: 54361 Committed r58340: <http://trac.webkit.org/changeset/58340>
All reviewed patches have been landed. Closing bug.
Thanks for the review!
Reverted: http://trac.webkit.org/changeset/58404
This was reverted because this snippet: if (x < walker.width()) in FontLinux.cpp would evaluate to false if 'x' was less than 'walker.width()' or if 'x' was negative, because walker.width() returned an unsigned value. C's type promotion rules would promote a negative 'x' to a large positive integer. I've changed walker.width() to return a signed value and changed this statement to: if (x >= 0 && x < walker.width())
Created attachment 55074 [details] Patch
Comment on attachment 55074 [details] Patch Again, removing the unused locals, and replacing NULL/0 and fixing parens (all completely uncontrovercial, mindless changes) would be easier to do first. The signed/unsigned stuff requires some thinking to review correctly. :(
Created attachment 55311 [details] Patch
Comment on attachment 55311 [details] Patch + 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.
Committed r58916: <http://trac.webkit.org/changeset/58916>
Thanks for the review. Landed with an explanation of the x >= 0 change in the ChangeLog and a citation of the relevant test.
http://trac.webkit.org/changeset/58916 might have broken Qt Windows 32-bit Release The following changes are on the blame list: http://trac.webkit.org/changeset/58912 http://trac.webkit.org/changeset/58913 http://trac.webkit.org/changeset/58914 http://trac.webkit.org/changeset/58915 http://trac.webkit.org/changeset/58916 http://trac.webkit.org/changeset/58917 http://trac.webkit.org/changeset/58918