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, On 2011/01/16 10:16:02, Michail Naganov wrote:
Arg per line, please.
Done. http://codereview.chromium.org/6240002/diff/1/src/platform-win32.cc#newcode922 src/platform-win32.cc:922: size_(size) { } On 2011/01/16 10:16:02, Michail Naganov wrote:
And init-per-line.
Done. 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), On 2011/01/16 10:16:02, Michail Naganov wrote:
Please either have a single line of initializations or init-per-line,
not a mix. Done. http://codereview.chromium.org/6240002/diff/1/src/utils.cc#newcode282 src/utils.cc:282: On 2011/01/16 10:16:02, Michail Naganov wrote:
Please remove this redundant empty line.
Done. http://codereview.chromium.org/6240002/diff/1/src/utils.cc#newcode301 src/utils.cc:301: On 2011/01/16 10:16:02, Michail Naganov wrote:
I this empty line and the one below a redundant.
Done. http://codereview.chromium.org/6240002/diff/1/src/utils.cc#newcode342 src/utils.cc:342: const char* err_context = p - 10; On 2011/01/16 10:16:02, Michail Naganov wrote:
Please introduce a constant for this magic number.
Done. http://codereview.chromium.org/6240002/diff/1/src/utils.cc#newcode351 src/utils.cc:351: PrintF(" after \"%s\"", buffer); On 2011/01/16 10:16:02, Michail Naganov wrote:
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" Done. 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, On 2011/01/16 10:16:02, Michail Naganov wrote:
2-args constructor does not need to be explicit.
Done. http://codereview.chromium.org/6240002/diff/1/src/v8utils.h#newcode333 src/v8utils.h:333: bool Exists() const { return file_ != NULL; } On 2011/01/16 10:16:02, Michail Naganov wrote:
Simple getter-like functions should be named lowercase.
Done. http://codereview.chromium.org/6240002/diff/1/src/v8utils.h#newcode337 src/v8utils.h:337: bool EnsureIsAscii() const { return EnsureIsAscii(true); } On 2011/01/16 10:16:02, Michail Naganov wrote:
Not sure that both versions of EnsureIsAscii should exist.
Done. This was partially revised code to remove the use of default arguments. The default arg is not removed. http://codereview.chromium.org/6240002/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
