[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