http://codereview.chromium.org/7066048/diff/1/samples/shell.cc File samples/shell.cc (right):
http://codereview.chromium.org/7066048/diff/1/samples/shell.cc#newcode314 samples/shell.cc:314: if (compressed_data[i].compressed_size != 0) { When does this happen? http://codereview.chromium.org/7066048/diff/1/samples/shell.cc#newcode326 samples/shell.cc:326: compressed_data[i].data = decompressed; It seems strange that we assign a block of fresh uninitialized memory here in case compressed_size is 0. http://codereview.chromium.org/7066048/diff/1/src/SConscript File src/SConscript (right): http://codereview.chromium.org/7066048/diff/1/src/SConscript#newcode337 src/SConscript:337: libraries_src = env.JS2C(['libraries.cc'], library_files, **BuildJS2CEnv('CORE')) It's already a lost cause but let's try to fit lines in this file in 80 cols. http://codereview.chromium.org/7066048/diff/1/src/api.cc File src/api.cc (right): http://codereview.chromium.org/7066048/diff/1/src/api.cc#newcode354 src/api.cc:354: for (int i = 0, idx = kSnapshotDataCount; It may be easier to read to maintain a separate offset variable (updated before the loops) and replace usages of idx with offset + i. http://codereview.chromium.org/7066048/diff/1/src/api.cc#newcode355 src/api.cc:355: i < i::Natives::GetBuiltinsCount(); I hope no compiler will have trouble with 'i' both as a variable and a namespace name. http://codereview.chromium.org/7066048/diff/1/src/api.cc#newcode391 src/api.cc:391: for (int i = 0, idx = kSnapshotDataCount; Same here. http://codereview.chromium.org/7066048/diff/1/src/mksnapshot.cc File src/mksnapshot.cc (right): http://codereview.chromium.org/7066048/diff/1/src/mksnapshot.cc#newcode282 src/mksnapshot.cc:282: for (int i = 0; i < i::Natives::GetBuiltinsCount(); ++i) { Hmm, it seems annoying to repeat this code with the internal details of Natives exposed. Can we somehow rework it to take the decompressor as a function? http://codereview.chromium.org/7066048/diff/1/tools/js2c.py File tools/js2c.py (right): http://codereview.chromium.org/7066048/diff/1/tools/js2c.py#newcode354 tools/js2c.py:354: if env['COMPRESSION'] == 'bz2': I think we can get better compression by gluing together all sources. http://codereview.chromium.org/7066048/diff/1/tools/js2c.py#newcode355 tools/js2c.py:355: lines = bz2.compress(lines, 9) 9 seems to be the default compression level so we can avoid the magic number. http://codereview.chromium.org/7066048/diff/1/tools/js2c.py#newcode370 tools/js2c.py:370: raw_source_lines.append(RAW_SOURCE_DECLARATION % { 'raw_id': raw_id, 'id': id }) Let's at least fit the code lines in 80 cols. http://codereview.chromium.org/7066048/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
