Bug 769191 - Have OS.File.prototype.{readTo, write} extrapolate a default length. r=froydnj

toolkit/components/osfile/osfile_shared_front.jsm

@@ -76,24 +76,26 @@ AbstractFile.prototype = {
    * following fields:
    * - {number} offset The offset in |buffer| at which to start placing
    * data
+   * - {number} bytes The number of |bytes| to write from the buffer. If
+   * unspecified, this is |buffer.byteLength|. Note that |bytes| is required
+   * if |buffer| is a C pointer.
    *
    * @return {number} The number of bytes actually read, which may be
-   * less than |bytes| if the file did not contain that many bytes left
+   * less than |bytes| if the file did not contain that many bytes left.
-   * or if |options.once| was set.
    */
-  readTo: function readTo(buffer, bytes, options) {
+  readTo: function readTo(buffer, options) {
     options = options || noOptions;
 
-    let pointer = AbstractFile.normalizeToPointer(buffer, bytes,
+    let {ptr, bytes} = AbstractFile.normalizeToPointer(buffer, options.bytes,
       options.offset);
     let pos = 0;
     while (pos < bytes) {
-      let chunkSize = this._read(pointer, bytes - pos, options);
+      let chunkSize = this._read(ptr, bytes - pos, options);
       if (chunkSize == 0) {
         break;
       }
       pos += chunkSize;
-      pointer = exports.OS.Shared.offsetBy(pointer, chunkSize);
+      ptr = exports.OS.Shared.offsetBy(ptr, chunkSize);
     }
 
     return pos;
@@ -113,21 +115,23 @@ AbstractFile.prototype = {
    * following fields:
    * - {number} offset The offset in |buffer| at which to start extracting
    * data
+   * - {number} bytes The number of |bytes| to write from the buffer. If
+   * unspecified, this is |buffer.byteLength|. Note that |bytes| is required
+   * if |buffer| is a C pointer.
    *
-   * @return {number} The number of bytes actually written, which may be
+   * @return {number} The number of bytes actually written.
-   * less than |bytes| if |options.once| was set.
    */
-  write: function write(buffer, bytes, options) {
+  write: function write(buffer, options) {
     options = options || noOptions;
 
-    let pointer = AbstractFile.normalizeToPointer(buffer, bytes,
+    let {ptr, bytes} = AbstractFile.normalizeToPointer(buffer, options.bytes,
       options.offset);
 
     let pos = 0;
     while (pos < bytes) {
-      let chunkSize = this._write(pointer, bytes - pos, options);
+      let chunkSize = this._write(ptr, bytes - pos, options);
       pos += chunkSize;
-      pointer = exports.OS.Shared.offsetBy(pointer, chunkSize);
+      ptr = exports.OS.Shared.offsetBy(ptr, chunkSize);
     }
     return pos;
   }
@@ -146,8 +150,8 @@ AbstractFile.prototype = {
  * @param {number=} offset Optionally, a number of bytes by which to shift
  * |candidate|.
  *
- * @return {C pointer} A C pointer of type uint8_t, corresponding to the
+ * @return {ptr:{C pointer}, bytes:number} A C pointer of type uint8_t,
- * start of |candidate| + |offset| bytes.
+ * corresponding to the start of |candidate| + |offset| bytes.
  */
 AbstractFile.normalizeToPointer = function normalizeToPointer(candidate, bytes, offset) {
   if (!candidate) {
@@ -162,8 +166,14 @@ AbstractFile.normalizeToPointer = function normalizeToPointer(candidate, bytes,
       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("C pointer missing bytes indication.");
+    }
   } else if ("byteLength" in candidate) {
     ptr = exports.OS.Shared.Type.uint8_t.out_ptr.implementation(candidate);
+    if (bytes == null) {
+      bytes = candidate.byteLength - offset;
+    }
     if (candidate.byteLength < offset + bytes) {
       throw new TypeError("Buffer is too short. I need at least " +
                          (offset + bytes) +
@@ -177,7 +187,7 @@ AbstractFile.normalizeToPointer = function normalizeToPointer(candidate, bytes,
   if (offset != 0) {
     ptr = exports.OS.Shared.offsetBy(ptr, offset);
   }
-  return ptr;
+  return {ptr: ptr, bytes: bytes};
 };
 
 /**

toolkit/components/osfile/tests/mochi/worker_test_osfile_front.js

@@ -9,6 +9,15 @@ function send(message) {
   self.postMessage(message);
 }
 
+function should_throw(f) {
+  try {
+    f();
+  } catch (x) {
+    return x;
+  }
+  return null;
+}
+
 self.onmessage = function onmessage_start(msg) {
   self.onmessage = function onmessage_ignored(msg) {
     log("ignored message " + JSON.stringify(msg.data));
@@ -230,10 +239,10 @@ function test_readall_writeall_file()
   let size = source.stat().size;
 
   let buf = new ArrayBuffer(size);
-  let readResult = source.readTo(buf, size);
+  let readResult = source.readTo(buf);
   is(readResult, size, "test_readall_writeall_file: read the right number of bytes");
 
-  dest.write(buf, size);
+  dest.write(buf);
 
   ok(true, "test_readall_writeall_file: copy complete (manual allocation)");
   source.close();
@@ -248,10 +257,10 @@ function test_readall_writeall_file()
   dest = OS.File.open(tmp_file_name, {write: true, trunc:true});
   buf = new ArrayBuffer(size);
   let ptr = OS.Shared.Type.voidptr_t.implementation(buf);
-  readResult = source.readTo(ptr, size);
+  readResult = source.readTo(ptr, {bytes: size});
   is(readResult, size, "test_readall_writeall_file: read the right number of bytes (C buffer)");
 
-  dest.write(ptr, readResult, {bytes: size});
+  dest.write(ptr, {bytes: size});
 
   ok(true, "test_readall_writeall_file: copy complete (C buffer)");
   source.close();
@@ -260,6 +269,17 @@ function test_readall_writeall_file()
   compare_files("test_readall_writeall_file (C buffer)", src_file_name, tmp_file_name);
   OS.File.remove(tmp_file_name);
 
+  // read/write, C buffer, missing |bytes| option
+  source = OS.File.open(src_file_name);
+  dest = OS.File.open(tmp_file_name, {write: true, trunc:true});
+  let exn = should_throw(function() { source.readTo(ptr); });
+  ok(exn != null && exn instanceof TypeError, "test_readall_writeall_file: read with C pointer and without bytes fails with the correct error");
+  exn = should_throw(function() { dest.write(ptr); });
+  ok(exn != null && exn instanceof TypeError, "test_readall_writeall_file: write with C pointer and without bytes fails with the correct error");
+
+  source.close();
+  dest.close();
+
   // readTo, ArrayBuffer + offset
   let OFFSET = 12;
   let LEFT = size - OFFSET;
@@ -267,10 +287,10 @@ function test_readall_writeall_file()
   source = OS.File.open(src_file_name);
   dest = OS.File.open(tmp_file_name, {write: true, trunc:true});
 
-  readResult = source.readTo(buf, LEFT, {offset: OFFSET});
+  readResult = source.readTo(buf, {offset: OFFSET});
   is(readResult, LEFT, "test_readall_writeall_file: read the right number of bytes (with offset)");
 
-  dest.write(buf, LEFT, {offset: OFFSET});
+  dest.write(buf, {offset: OFFSET});
   is(dest.stat().size, LEFT, "test_readall_writeall_file: wrote the right number of bytes (with offset)");
 
   ok(true, "test_readall_writeall_file: copy complete (with offset)");
@@ -287,10 +307,10 @@ function test_readall_writeall_file()
   source = OS.File.open(src_file_name);
   dest = OS.File.open(tmp_file_name, {write: true, trunc:true});
 
-  readResult = source.readTo(ptr, LEFT, {offset: OFFSET});
+  readResult = source.readTo(ptr, {bytes: LEFT, offset: OFFSET});
   is(readResult, LEFT, "test_readall_writeall_file: read the right number of bytes (with offset)");
 
-  dest.write(ptr, LEFT, {offset: OFFSET});
+  dest.write(ptr, {bytes: LEFT, offset: OFFSET});
   is(dest.stat().size, LEFT, "test_readall_writeall_file: wrote the right number of bytes (with offset)");
 
   ok(true, "test_readall_writeall_file: copy complete (with offset)");
@@ -308,7 +328,7 @@ function test_readall_writeall_file()
   readResult = source.read();
   is(readResult.bytes, size, "test_readall_writeall_file: read the right number of bytes (auto allocation)");
 
-  dest.write(readResult.buffer, readResult.bytes);
+  dest.write(readResult.buffer, {bytes: readResult.bytes});
 
   ok(true, "test_readall_writeall_file: copy complete (auto allocation)");
   source.close();
@@ -316,8 +336,6 @@ function test_readall_writeall_file()
 
   compare_files("test_readall_writeall_file (auto allocation)", src_file_name, tmp_file_name);
   OS.File.remove(tmp_file_name);
-
-
 }
 
 /**