Bug 924858 - Part 1: Add |append| mode flag to OS.File.open on Unix. r=yoric
authorNils Maier <maierman@web.de>
Sun, 27 Oct 2013 08:03:16 -0400
changeset 167160 a8af01ed5f521b02ef83863b09114caa68e25a55
parent 167159 26d92ad24d6702d42bf4a292428412ee5398be03
child 167161 2c93630e3109e1337f0674da8bed4d9b839df5b4
push id428
push userbbajaj@mozilla.com
push dateTue, 28 Jan 2014 00:16:25 +0000
treeherdermozilla-release@cd72a7ff3a75 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyoric
bugs924858
milestone27.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 924858 - Part 1: Add |append| mode flag to OS.File.open on Unix. r=yoric To match previous behavior, this mode flag will default to |true|.
toolkit/components/osfile/modules/osfile_shared_front.jsm
toolkit/components/osfile/modules/osfile_unix_front.jsm
toolkit/components/osfile/tests/xpcshell/test_osfile_async_append.js
toolkit/components/osfile/tests/xpcshell/xpcshell.ini
--- a/toolkit/components/osfile/modules/osfile_shared_front.jsm
+++ b/toolkit/components/osfile/modules/osfile_shared_front.jsm
@@ -299,39 +299,43 @@ AbstractFile.AbstractIterator.prototype 
  * and |existing|.
  */
 AbstractFile.normalizeOpenMode = function normalizeOpenMode(mode) {
   let result = {
     read: false,
     write: false,
     trunc: false,
     create: false,
-    existing: false
+    existing: false,
+    append: true
   };
   for (let key in mode) {
-    if (!mode[key]) continue; // Only interpret true-ish keys
+    let val = !!mode[key]; // bool cast.
     switch (key) {
     case "read":
-      result.read = true;
+      result.read = val;
       break;
     case "write":
-      result.write = true;
+      result.write = val;
       break;
     case "truncate": // fallthrough
     case "trunc":
-      result.trunc = true;
-      result.write = true;
+      result.trunc = val;
+      result.write |= val;
       break;
     case "create":
-      result.create = true;
-      result.write = true;
+      result.create = val;
+      result.write |= val;
       break;
     case "existing": // fallthrough
     case "exist":
-      result.existing = true;
+      result.existing = val;
+      break;
+    case "append":
+      result.append = val;
       break;
     default:
       throw new TypeError("Mode " + key + " not understood");
     }
   }
   // Reject opposite modes
   if (result.existing && result.create) {
     throw new TypeError("Cannot specify both existing:true and create:true");
--- a/toolkit/components/osfile/modules/osfile_unix_front.jsm
+++ b/toolkit/components/osfile/modules/osfile_unix_front.jsm
@@ -198,18 +198,21 @@
       *  be specified with |truncate| or |existing|.
       * - {bool} existing. If the file does not exist, this function
       *  fails. Cannot be specified with |create|.
       * - {bool} read If |true|, the file will be opened for
       *  reading. The file may also be opened for writing, depending
       *  on the other fields of |mode|.
       * - {bool} write If |true|, the file will be opened for
       *  writing. The file may also be opened for reading, depending
-      *  on the other fields of |mode|. If neither |truncate| nor
-      *  |create| is specified, the file is opened for appending.
+      *  on the other fields of |mode|.
+      * - {bool} append If |true|, the file will be opened for appending,
+      *  meaning the equivalent of |.setPosition(0, POS_END)| is executed
+      *  before each write. The default is |true|, i.e. opening a file for
+      *  appending. Specify |append: false| to open the file in regular mode.
       *
       * If neither |truncate|, |create| or |write| is specified, the file
       * is opened for reading.
       *
       * Note that |false|, |null| or |undefined| flags are simply ignored.
       *
       * @param {*=} options Additional options for file opening. This
       * implementation interprets the following fields:
@@ -246,22 +249,21 @@
              flags |= Const.O_TRUNC;
            } else {
              flags |= Const.O_CREAT | Const.O_TRUNC;
            }
          } else if (mode.create) {
            flags |= Const.O_CREAT | Const.O_EXCL;
          } else if (mode.read && !mode.write) {
            // flags are sufficient
-         } else /*append*/ {
-           if (mode.existing) {
-             flags |= Const.O_APPEND;
-           } else {
-             flags |= Const.O_APPEND | Const.O_CREAT;
-           }
+         } else if (!mode.existing) {
+           flags |= Const.O_CREAT;
+         }
+         if (mode.append) {
+           flags |= Const.O_APPEND;
          }
        }
        return error_or_file(UnixFile.open(path, flags, omode));
      };
 
      /**
       * Checks if a file exists
       *
@@ -543,20 +545,22 @@
        // Implement |copy| using |pump|.
        // This implementation would require some work before being able to
        // copy directories
        File.copy = function copy(sourcePath, destPath, options = {}) {
          let source, dest;
          let result;
          try {
            source = File.open(sourcePath);
+           // Need to open the output file with |append:false|, or else |splice|
+           // won't work.
            if (options.noOverwrite) {
-             dest = File.open(destPath, {create:true});
+             dest = File.open(destPath, {create:true, append:false});
            } else {
-             dest = File.open(destPath, {trunc:true});
+             dest = File.open(destPath, {trunc:true, append:false});
            }
            if (options.unixUserland) {
              result = pump_userland(source, dest, options);
            } else {
              result = pump(source, dest, options);
            }
          } catch (x) {
            if (dest) {
new file mode 100644
--- /dev/null
+++ b/toolkit/components/osfile/tests/xpcshell/test_osfile_async_append.js
@@ -0,0 +1,122 @@
+"use strict";
+
+do_print("starting tests");
+
+Components.utils.import("resource://gre/modules/osfile.jsm");
+Components.utils.import("resource://gre/modules/Task.jsm");
+
+/**
+ * A test to check that the |append| mode flag is correctly implemented.
+ * (see bug 925865)
+ */
+
+function setup_mode(mode) {
+  // Complete mode.
+  let realMode = {
+    read: true,
+    write: true
+  };
+  for (let k in mode) {
+    realMode[k] = mode[k];
+  }
+  return realMode;
+}
+
+// Test append mode.
+function test_append(mode) {
+  let path = OS.Path.join(OS.Constants.Path.tmpDir,
+                          "test_osfile_async_append.tmp");
+
+  // Clear any left-over files from previous runs.
+  try {
+    yield OS.File.remove(path);
+  } catch (ex if ex.becauseNoSuchFile) {
+    // ignore
+  }
+
+  try {
+    mode = setup_mode(mode);
+    mode.append = true;
+    if (mode.trunc) {
+      // Pre-fill file with some data to see if |trunc| actually works.
+      yield OS.File.writeAtomic(path, new Uint8Array(500));
+    }
+    let file = yield OS.File.open(path, mode);
+    try {
+      yield file.write(new Uint8Array(1000));
+      yield file.setPosition(0, OS.File.POS_START);
+      yield file.read(100);
+      // Should be at offset 100, length 1000 now.
+      yield file.write(new Uint8Array(100));
+      // Should be at offset 1100, length 1100 now.
+      let stat = yield file.stat();
+      do_check_eq(1100, stat.size);
+    } finally {
+      yield file.close();
+    }
+  } catch(ex) {
+    try {
+      yield OS.File.remove(path);
+    } catch (ex if ex.becauseNoSuchFile) {
+      // ignore.
+    }
+  }
+}
+
+// Test no-append mode.
+function test_no_append(mode) {
+  let path = OS.Path.join(OS.Constants.Path.tmpDir,
+                          "test_osfile_async_noappend.tmp");
+
+  // Clear any left-over files from previous runs.
+  try {
+    yield OS.File.remove(path);
+  } catch (ex if ex.becauseNoSuchFile) {
+    // ignore
+  }
+
+  try {
+    mode = setup_mode(mode);
+    mode.append = false;
+    if (mode.trunc) {
+      // Pre-fill file with some data to see if |trunc| actually works.
+      yield OS.File.writeAtomic(path, new Uint8Array(500));
+    }
+    let file = yield OS.File.open(path, mode);
+    try {
+      yield file.write(new Uint8Array(1000));
+      yield file.setPosition(0, OS.File.POS_START);
+      yield file.read(100);
+      // Should be at offset 100, length 1000 now.
+      yield file.write(new Uint8Array(100));
+      // Should be at offset 200, length 1000 now.
+      let stat = yield file.stat();
+      do_check_eq(1000, stat.size);
+    } finally {
+      yield file.close();
+    }
+  } finally {
+    try {
+      yield OS.File.remove(path);
+    } catch (ex if ex.becauseNoSuchFile) {
+      // ignore.
+    }
+  }
+}
+
+let test_flags = [
+  {},
+  {create:true},
+  {trunc:true}
+];
+function run_test() {
+  do_test_pending();
+
+  for (let t of test_flags) {
+    add_task(test_append.bind(null, t));
+    add_task(test_no_append.bind(null, t));
+  }
+  add_task(do_test_finished);
+
+  run_next_test();
+}
--- a/toolkit/components/osfile/tests/xpcshell/xpcshell.ini
+++ b/toolkit/components/osfile/tests/xpcshell/xpcshell.ini
@@ -1,15 +1,16 @@
 [DEFAULT]
 head =
 tail =
 
 [test_osfile_closed.js]
 [test_path.js]
 [test_osfile_async.js]
+[test_osfile_async_append.js]
 [test_osfile_async_bytes.js]
 [test_osfile_async_copy.js]
 [test_profiledir.js]
 [test_logging.js]
 [test_creationDate.js]
 [test_exception.js]
 [test_path_constants.js]
 [test_removeDir.js]