http://codereview.chromium.org/6240002/diff/1/src/platform-win32.cc File src/platform-win32.cc (right):
http://codereview.chromium.org/6240002/diff/1/src/platform-win32.cc#newcode919 src/platform-win32.cc:919: Win32MemoryMappedFile(HANDLE file, HANDLE file_mapping, void* memory, Arg per line, please. http://codereview.chromium.org/6240002/diff/1/src/platform-win32.cc#newcode922 src/platform-win32.cc:922: size_(size) { } And init-per-line. http://codereview.chromium.org/6240002/diff/1/src/utils.cc File src/utils.cc (right): http://codereview.chromium.org/6240002/diff/1/src/utils.cc#newcode280 src/utils.cc:280: : filename_(NULL), data_(NULL), length_(0), Please either have a single line of initializations or init-per-line, not a mix. http://codereview.chromium.org/6240002/diff/1/src/utils.cc#newcode282 src/utils.cc:282: Please remove this redundant empty line. http://codereview.chromium.org/6240002/diff/1/src/utils.cc#newcode301 src/utils.cc:301: I this empty line and the one below a redundant. http://codereview.chromium.org/6240002/diff/1/src/utils.cc#newcode342 src/utils.cc:342: const char* err_context = p - 10; Please introduce a constant for this magic number. http://codereview.chromium.org/6240002/diff/1/src/utils.cc#newcode351 src/utils.cc:351: PrintF(" after \"%s\"", buffer); You can use the "precision" field to specify the maximum number of chars to print, e.g. "%.10s". To get the precision value from the args list, use the '*' char: "%.*s" http://codereview.chromium.org/6240002/diff/1/src/v8utils.h File src/v8utils.h (right): http://codereview.chromium.org/6240002/diff/1/src/v8utils.h#newcode326 src/v8utils.h:326: explicit MemoryMappedExternalResource(const char* filename, 2-args constructor does not need to be explicit. http://codereview.chromium.org/6240002/diff/1/src/v8utils.h#newcode333 src/v8utils.h:333: bool Exists() const { return file_ != NULL; } Simple getter-like functions should be named lowercase. http://codereview.chromium.org/6240002/diff/1/src/v8utils.h#newcode337 src/v8utils.h:337: bool EnsureIsAscii() const { return EnsureIsAscii(true); } Not sure that both versions of EnsureIsAscii should exist. http://codereview.chromium.org/6240002/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
