LGTM with comments addressed.
http://codereview.chromium.org/6240002/diff/15001/src/utils.cc File src/utils.cc (right): http://codereview.chromium.org/6240002/diff/15001/src/utils.cc#newcode301 src/utils.cc:301: if (file_) { Please use explicit comparison: if (file_ != NULL). Sorry, I missed this in the first round. http://codereview.chromium.org/6240002/diff/15001/src/utils.cc#newcode313 src/utils.cc:313: if (file_) { Ditto. http://codereview.chromium.org/6240002/diff/15001/src/utils.cc#newcode329 src/utils.cc:329: if (c & 0x80) { Ditto. http://codereview.chromium.org/6240002/diff/15001/src/utils.cc#newcode352 src/utils.cc:352: if (err_context_length) { Ditto. http://codereview.chromium.org/6240002/diff/15001/src/utils.cc#newcode356 src/utils.cc:356: abort(); OS::Abort(); http://codereview.chromium.org/6240002/diff/15001/src/v8utils.h File src/v8utils.h (right): http://codereview.chromium.org/6240002/diff/15001/src/v8utils.h#newcode334 src/v8utils.h:334: bool IsEmpty() const { return length_ == 0; } is_empty() -- this is a getter-like operation. http://codereview.chromium.org/6240002/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
