LGTM with comments.
http://codereview.chromium.org/9283015/diff/1/src/d8.cc File src/d8.cc (right): http://codereview.chromium.org/9283015/diff/1/src/d8.cc#newcode1026 src/d8.cc:1026: Handle<Value> Shell::ReadBinary(const Arguments& args) { Wouldn't it make more sense to call this ReadExternalString or even ReadIntoExternalString? I don't see what this has to do with binary files. http://codereview.chromium.org/9283015/diff/1/src/d8.cc#newcode1027 src/d8.cc:1027: String::Utf8Value filename(args[0]); What happens when the file doesn't exist? Maybe add: if (*filename == NULL) { return ThrowException(String::New("Error reading file")); } http://codereview.chromium.org/9283015/diff/1/src/d8.h File src/d8.h (right): http://codereview.chromium.org/9283015/diff/1/src/d8.h#newcode200 src/d8.h:200: explicit BinaryResource(const char* string, int length) nit: "explicit" is unnecessary. http://codereview.chromium.org/9283015/diff/1/src/d8.h#newcode205 src/d8.h:205: delete data_; shouldn't this be "delete[] data_"? http://codereview.chromium.org/9283015/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
