http://codereview.chromium.org/9114050/diff/4001/src/d8.cc File src/d8.cc (right):
http://codereview.chromium.org/9114050/diff/4001/src/d8.cc#newcode284 src/d8.cc:284: static int convertToInt(Local<Value> value_in, TryCatch* try_catch) { Rename to convertToUint? http://codereview.chromium.org/9114050/diff/4001/src/d8.cc#newcode339 src/d8.cc:339: if (args.Length() > 3 || args.Length() == 0) { Arguments.operator[] is benign enough to simply return 'undefined' for args[0] if args.Length == 0, but nevertheless I'd prefer to check for Length() > 0 before doing anything else (you can print a better error message that way, too). http://codereview.chromium.org/9114050/diff/4001/src/d8.cc#newcode350 src/d8.cc:350: : args[2]; args[2] is optional and could be undefined. As implemented, the code defaults to 0 in that case, which is not correct. http://codereview.chromium.org/9114050/diff/4001/src/d8.cc#newcode363 src/d8.cc:363: array->Get(String::New("BYTES_PER_ELEMENT"))->ToInt32()->Int32Value(); My reading of the spec at http://www.khronos.org/registry/typedarray/specs/latest/#7 is that |array| must be an ArrayBuffer (and cannot be another kind of TypedArray), so BYTES_PER_ELEMENTS is always 1, and you'll have to throw an exception if a TypedArray is passed in. Further, IIUC, the "length" parameter specifies the new array's length in terms of number of elements in the new array, so all this conversion-fu is unnecessary. http://codereview.chromium.org/9114050/diff/4001/src/d8.cc#newcode389 src/d8.cc:389: // Hold a reference to the ArrayBuffer so it's buffer doesn't get collected. s/it's/its/ http://codereview.chromium.org/9114050/diff/4001/src/d8.cc#newcode393 src/d8.cc:393: reinterpret_cast<uint8_t*>(data) + offset, type, Whoa... please add checks that both |offset| and |offset + length*element_size| are within the existing |data| buffer. http://codereview.chromium.org/9114050/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
