fs: add file descriptor support to *File() funcs

These changes affect the following functions and their synchronous
counterparts:

 * fs.readFile()
 * fs.writeFile()
 * fs.appendFile()

If the first parameter is a uint32, it is treated as a file descriptor.
In all other cases, the original implementation is used to ensure
backwards compatibility. File descriptor ownership is never taken from
the user.

The documentation was adjusted to reflect these API changes. A note was
added to make the user aware of file descriptor ownership and the
conditions under which a file descriptor can be used by each of these
functions.

Tests were extended to test for file descriptor parameters under the
conditions noted in the relevant documentation.

PR-URL: https://github.com/nodejs/node/pull/3163
Reviewed-By: Trevor Norris <trev.norris@gmail.com>

doc/api/fs.markdown

@@ -468,9 +468,9 @@ The callback is given the three arguments, `(err, bytesRead, buffer)`.
 
 Synchronous version of `fs.read`. Returns the number of `bytesRead`.
 
-## fs.readFile(filename[, options], callback)
+## fs.readFile(file[, options], callback)
 
-* `filename` {String}
+* `file` {String | Integer} filename or file descriptor
 * `options` {Object | String}
   * `encoding` {String | Null} default = `null`
   * `flag` {String} default = `'r'`
@@ -492,18 +492,20 @@ If `options` is a string, then it specifies the encoding. Example:
 
     fs.readFile('/etc/passwd', 'utf8', callback);
 
+Any specified file descriptor has to support reading.
 
-## fs.readFileSync(filename[, options])
+_Note: Specified file descriptors will not be closed automatically._
 
-Synchronous version of `fs.readFile`. Returns the contents of the `filename`.
+## fs.readFileSync(file[, options])
+
+Synchronous version of `fs.readFile`. Returns the contents of the `file`.
 
 If the `encoding` option is specified then this function returns a
 string. Otherwise it returns a buffer.
 
+## fs.writeFile(file, data[, options], callback)
 
-## fs.writeFile(filename, data[, options], callback)
-
-* `filename` {String}
+* `file` {String | Integer} filename or file descriptor
 * `data` {String | Buffer}
 * `options` {Object | String}
   * `encoding` {String | Null} default = `'utf8'`
@@ -528,13 +530,21 @@ If `options` is a string, then it specifies the encoding. Example:
 
     fs.writeFile('message.txt', 'Hello Node.js', 'utf8', callback);
 
-## fs.writeFileSync(filename, data[, options])
+Any specified file descriptor has to support writing.
+
+Note that it is unsafe to use `fs.writeFile` multiple times on the same file
+without waiting for the callback. For this scenario,
+`fs.createWriteStream` is strongly recommended.
+
+_Note: Specified file descriptors will not be closed automatically._
+
+## fs.writeFileSync(file, data[, options])
 
 The synchronous version of `fs.writeFile`. Returns `undefined`.
 
-## fs.appendFile(filename, data[, options], callback)
+## fs.appendFile(file, data[, options], callback)
 
-* `filename` {String}
+* `file` {String | Integer} filename or file descriptor
 * `data` {String | Buffer}
 * `options` {Object | String}
   * `encoding` {String | Null} default = `'utf8'`
@@ -556,7 +566,11 @@ If `options` is a string, then it specifies the encoding. Example:
 
     fs.appendFile('message.txt', 'data to append', 'utf8', callback);
 
-## fs.appendFileSync(filename, data[, options])
+Any specified file descriptor has to have been opened for appending.
+
+_Note: Specified file descriptors will not be closed automatically._
+
+## fs.appendFileSync(file, data[, options])
 
 The synchronous version of `fs.appendFile`. Returns `undefined`.
 

lib/fs.js

@@ -101,6 +101,10 @@ function nullCheck(path, callback) {
   return true;
 }
 
+function isFd(path) {
+  return (path >>> 0) === path;
+}
+
 // Static method to set the stats properties on a Stats object.
 fs.Stats = function(
     dev,
@@ -243,10 +247,18 @@ fs.readFile = function(path, options, callback_) {
     return;
 
   var context = new ReadFileContext(callback, encoding);
+  context.isUserFd = isFd(path); // file descriptor ownership
   var req = new FSReqWrap();
   req.context = context;
   req.oncomplete = readFileAfterOpen;
 
+  if (context.isUserFd) {
+    process.nextTick(function() {
+      req.oncomplete(null, path);
+    });
+    return;
+  }
+
   binding.open(pathModule._makeLong(path),
                stringToFlags(flag),
                0o666,
@@ -257,6 +269,7 @@ const kReadFileBufferLength = 8 * 1024;
 
 function ReadFileContext(callback, encoding) {
   this.fd = undefined;
+  this.isUserFd = undefined;
   this.size = undefined;
   this.callback = callback;
   this.buffers = null;
@@ -293,6 +306,14 @@ ReadFileContext.prototype.close = function(err) {
   req.oncomplete = readFileAfterClose;
   req.context = this;
   this.err = err;
+
+  if (this.isUserFd) {
+    process.nextTick(function() {
+      req.oncomplete(null);
+    });
+    return;
+  }
+
   binding.close(this.fd, req);
 };
 
@@ -394,7 +415,8 @@ fs.readFileSync = function(path, options) {
   assertEncoding(encoding);
 
   var flag = options.flag || 'r';
-  var fd = fs.openSync(path, flag, 0o666);
+  var isUserFd = isFd(path); // file descriptor ownership
+  var fd = isUserFd ? path : fs.openSync(path, flag, 0o666);
 
   var st;
   var size;
@@ -404,7 +426,7 @@ fs.readFileSync = function(path, options) {
     size = st.isFile() ? st.size : 0;
     threw = false;
   } finally {
-    if (threw) fs.closeSync(fd);
+    if (threw && !isUserFd) fs.closeSync(fd);
   }
 
   var pos = 0;
@@ -419,7 +441,7 @@ fs.readFileSync = function(path, options) {
       buffer = new Buffer(size);
       threw = false;
     } finally {
-      if (threw) fs.closeSync(fd);
+      if (threw && !isUserFd) fs.closeSync(fd);
     }
   }
 
@@ -442,14 +464,15 @@ fs.readFileSync = function(path, options) {
       }
       threw = false;
     } finally {
-      if (threw) fs.closeSync(fd);
+      if (threw && !isUserFd) fs.closeSync(fd);
     }
 
     pos += bytesRead;
     done = (bytesRead === 0) || (size !== 0 && pos >= size);
   }
 
-  fs.closeSync(fd);
+  if (!isUserFd)
+    fs.closeSync(fd);
 
   if (size === 0) {
     // data was collected into the buffers list.
@@ -1096,25 +1119,33 @@ fs.futimesSync = function(fd, atime, mtime) {
   binding.futimes(fd, atime, mtime);
 };
 
-function writeAll(fd, buffer, offset, length, position, callback_) {
+function writeAll(fd, isUserFd, buffer, offset, length, position, callback_) {
   var callback = maybeCallback(arguments[arguments.length - 1]);
 
   // write(fd, buffer, offset, length, position, callback)
   fs.write(fd, buffer, offset, length, position, function(writeErr, written) {
     if (writeErr) {
-      fs.close(fd, function() {
+      if (isUserFd) {
         if (callback) callback(writeErr);
-      });
+      } else {
+        fs.close(fd, function() {
+          if (callback) callback(writeErr);
+        });
+      }
     } else {
       if (written === length) {
-        fs.close(fd, callback);
+        if (isUserFd) {
+          if (callback) callback(null);
+        } else {
+          fs.close(fd, callback);
+        }
       } else {
         offset += written;
         length -= written;
         if (position !== null) {
           position += written;
         }
-        writeAll(fd, buffer, offset, length, position, callback);
+        writeAll(fd, isUserFd, buffer, offset, length, position, callback);
       }
     }
   });
@@ -1134,16 +1165,27 @@ fs.writeFile = function(path, data, options, callback_) {
   assertEncoding(options.encoding);
 
   var flag = options.flag || 'w';
+
+  if (isFd(path)) {
+    writeFd(path, true);
+    return;
+  }
+
   fs.open(path, flag, options.mode, function(openErr, fd) {
     if (openErr) {
       if (callback) callback(openErr);
     } else {
-      var buffer = (data instanceof Buffer) ? data : new Buffer('' + data,
-          options.encoding || 'utf8');
-      var position = /a/.test(flag) ? null : 0;
-      writeAll(fd, buffer, 0, buffer.length, position, callback);
+      writeFd(fd, false);
     }
   });
+
+  function writeFd(fd, isUserFd) {
+    var buffer = (data instanceof Buffer) ? data : new Buffer('' + data,
+        options.encoding || 'utf8');
+    var position = /a/.test(flag) ? null : 0;
+
+    writeAll(fd, isUserFd, buffer, 0, buffer.length, position, callback);
+  }
 };
 
 fs.writeFileSync = function(path, data, options) {
@@ -1158,7 +1200,9 @@ fs.writeFileSync = function(path, data, options) {
   assertEncoding(options.encoding);
 
   var flag = options.flag || 'w';
-  var fd = fs.openSync(path, flag, options.mode);
+  var isUserFd = isFd(path); // file descriptor ownership
+  var fd = isUserFd ? path : fs.openSync(path, flag, options.mode);
+
   if (!(data instanceof Buffer)) {
     data = new Buffer('' + data, options.encoding || 'utf8');
   }
@@ -1175,7 +1219,7 @@ fs.writeFileSync = function(path, data, options) {
       }
     }
   } finally {
-    fs.closeSync(fd);
+    if (!isUserFd) fs.closeSync(fd);
   }
 };
 
@@ -1192,6 +1236,11 @@ fs.appendFile = function(path, data, options, callback_) {
 
   if (!options.flag)
     options = util._extend({ flag: 'a' }, options);
+
+  // force append behavior when using a supplied file descriptor
+  if (isFd(path))
+    options.flag = 'a';
+
   fs.writeFile(path, data, options, callback);
 };
 
@@ -1203,9 +1252,14 @@ fs.appendFileSync = function(path, data, options) {
   } else if (typeof options !== 'object') {
     throwOptionsError(options);
   }
+
   if (!options.flag)
     options = util._extend({ flag: 'a' }, options);
 
+  // force append behavior when using a supplied file descriptor
+  if (isFd(path))
+    options.flag = 'a';
+
   fs.writeFileSync(path, data, options);
 };
 

test/parallel/test-fs-append-file-sync.js

@@ -66,6 +66,19 @@ var fileData4 = fs.readFileSync(filename4);
 assert.equal(Buffer.byteLength('' + num) + currentFileData.length,
              fileData4.length);
 
+// test that appendFile accepts file descriptors
+var filename5 = join(common.tmpDir, 'append-sync5.txt');
+fs.writeFileSync(filename5, currentFileData);
+
+var filename5fd = fs.openSync(filename5, 'a+', 0o600);
+fs.appendFileSync(filename5fd, data);
+fs.closeSync(filename5fd);
+
+var fileData5 = fs.readFileSync(filename5);
+
+assert.equal(Buffer.byteLength(data) + currentFileData.length,
+             fileData5.length);
+
 //exit logic for cleanup
 
 process.on('exit', function() {
@@ -73,4 +86,5 @@ process.on('exit', function() {
   fs.unlinkSync(filename2);
   fs.unlinkSync(filename3);
   fs.unlinkSync(filename4);
+  fs.unlinkSync(filename5);
 });

test/parallel/test-fs-append-file.js

@@ -92,11 +92,42 @@ fs.appendFile(filename4, n, { mode: m }, function(e) {
   });
 });
 
+// test that appendFile accepts file descriptors
+var filename5 = join(common.tmpDir, 'append5.txt');
+fs.writeFileSync(filename5, currentFileData);
+
+fs.open(filename5, 'a+', function(e, fd) {
+  if (e) throw e;
+
+  ncallbacks++;
+
+  fs.appendFile(fd, s, function(e) {
+    if (e) throw e;
+
+    ncallbacks++;
+
+    fs.close(fd, function(e) {
+      if (e) throw e;
+
+      ncallbacks++;
+
+      fs.readFile(filename5, function(e, buffer) {
+        if (e) throw e;
+
+        ncallbacks++;
+        assert.equal(Buffer.byteLength(s) + currentFileData.length,
+                     buffer.length);
+      });
+    });
+  });
+});
+
 process.on('exit', function() {
-  assert.equal(8, ncallbacks);
+  assert.equal(12, ncallbacks);
 
   fs.unlinkSync(filename);
   fs.unlinkSync(filename2);
   fs.unlinkSync(filename3);
   fs.unlinkSync(filename4);
+  fs.unlinkSync(filename5);
 });

test/parallel/test-fs-readfile-fd.js

@@ -0,0 +1,47 @@
+'use strict';
+var common = require('../common');
+var assert = require('assert');
+
+var path = require('path'),
+    fs = require('fs'),
+    fn = path.join(common.fixturesDir, 'empty.txt');
+
+tempFd(function(fd, close) {
+  fs.readFile(fd, function(err, data) {
+    assert.ok(data);
+    close();
+  });
+});
+
+tempFd(function(fd, close) {
+  fs.readFile(fd, 'utf8', function(err, data) {
+    assert.strictEqual('', data);
+    close();
+  });
+});
+
+tempFdSync(function(fd) {
+  assert.ok(fs.readFileSync(fd));
+});
+
+tempFdSync(function(fd) {
+  assert.strictEqual('', fs.readFileSync(fd, 'utf8'));
+});
+
+function tempFd(callback) {
+  fs.open(fn, 'r', function(err, fd) {
+    if (err) throw err;
+
+    callback(fd, function() {
+      fs.close(fd, function(err) {
+        if (err) throw err;
+      });
+    });
+  });
+}
+
+function tempFdSync(callback) {
+  var fd = fs.openSync(fn, 'r');
+  callback(fd);
+  fs.closeSync(fd);
+}

test/parallel/test-fs-write-file-sync.js

@@ -47,6 +47,18 @@ assert.equal('abc', content);
 
 assert.equal(mode, fs.statSync(file2).mode & mode);
 
+// Test writeFileSync with file descriptor
+var file3 = path.join(common.tmpDir, 'testWriteFileSyncFd.txt');
+
+var fd = fs.openSync(file3, 'w+', mode);
+fs.writeFileSync(fd, '123');
+fs.closeSync(fd);
+
+content = fs.readFileSync(file3, {encoding: 'utf8'});
+assert.equal('123', content);
+
+assert.equal(mode, fs.statSync(file3).mode & 0o777);
+
 // Verify that all opened files were closed.
 assert.equal(0, openCount);
 

test/parallel/test-fs-write-file.js

@@ -69,11 +69,40 @@ fs.writeFile(filename3, n, { mode: m }, function(e) {
   });
 });
 
+// test that writeFile accepts file descriptors
+var filename4 = join(common.tmpDir, 'test4.txt');
+var buf = new Buffer(s, 'utf8');
+
+fs.open(filename4, 'w+', function(e, fd) {
+  if (e) throw e;
+
+  ncallbacks++;
+
+  fs.writeFile(fd, s, function(e) {
+    if (e) throw e;
+
+    ncallbacks++;
+
+    fs.close(fd, function(e) {
+      if (e) throw e;
+
+      ncallbacks++;
+
+      fs.readFile(filename4, function(e, buffer) {
+        if (e) throw e;
+
+        ncallbacks++;
+        assert.equal(Buffer.byteLength(s), buffer.length);
+      });
+    });
+  });
+});
 
 process.on('exit', function() {
-  assert.equal(6, ncallbacks);
+  assert.equal(10, ncallbacks);
 
   fs.unlinkSync(filename);
   fs.unlinkSync(filename2);
   fs.unlinkSync(filename3);
+  fs.unlinkSync(filename4);
 });