[webkit-reviews] review denied: [Bug 28150] WINCE PORT: Image Encoders : [Attachment 34480] the patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Sep 1 18:22:56 PDT 2009
Adam Barth <abarth at webkit.org> has denied Yong Li <yong.li at torchmobile.com>'s
request for review:
Bug 28150: WINCE PORT: Image Encoders
https://bugs.webkit.org/show_bug.cgi?id=28150
Attachment 34480: the patch
https://bugs.webkit.org/attachment.cgi?id=34480&action=review
------- Additional Comments from Adam Barth <abarth at webkit.org>
+ // Zero base class memory
+ memset(this, 0, sizeof(jpeg_destination_mgr));
C++ semantics don't guarantee that this is safe! You don't know what the
internal layout of your class is going to be. Yikes! (This error is repeated
multiple times.)
+ Vector<char> m_buffer
Member variables should be private.
+ dest->next_output_byte = reinterpret_cast<JOCTET*>(dest->m_buffer.data());
This line and the following line are not proper WebKit style. Note: this style
error is made repeatedly throughout the file.
+ struct jpeg_compress_struct compressData= { 0 };
This line has improper style.
+ #if IMAGE_NO_ALPHA_USE_RGB555
Magic constants following this line should be removed.
+ // This section of the code is based on nsPNGEncoder.cpp in Mozilla
I suspect we're missing the Mozilla license block then.
Also, your ChangeLog leads me to believe this patch contains a bunch of
copy-and-paste code duplication.
More information about the webkit-reviews
mailing list