Common scenario: let a = new ArrayBuffer(21); let file = OS.File.open(...) file.read(a, a.byteLength); If no length is given, we should use the length of the array buffer.

Comment on attachment 637423 [details] [diff] [review] Default arguments of OS.File.prototype.read/write bugzilla is failing so I can't see the patch, but this makes sense

Doing this differently.

Comment on attachment 656581 [details] [diff] [review] Default arguments for AbstractFile.prototype.{readTo, write} Review of attachment 656581 [details] [diff] [review]: ----------------------------------------------------------------- Somewhere in worker_test_osfile_front.js it would be good to add a test for C pointer without options.bytes, for both readTo and write, to check that the error is thrown correctly. ::: toolkit/components/osfile/osfile_shared_front.jsm @@ +166,5 @@ > throw new TypeError("Expecting a non-null pointer"); > } > ptr = exports.OS.Shared.Type.uint8_t.out_ptr.cast(candidate); > + if (bytes == null) { > + throw new TypeError("This function expects either an ArrayBuffer or a C pointer and a number of bytes."); I think this error message should indicate that |bytes| is required with a C pointer and shouldn't talk about ArrayBuffers; this is the pointer case, after all.

Added default arguments and changed error message.

Hum, I meant "added *test* for default arguments".

(Remarkably) Green on Try. https://tbpl.mozilla.org/?tree=Try&rev=cf7f719d7f47 https://hg.mozilla.org/integration/mozilla-inbound/rev/a88c590a2c0b