From 8e7a193f82c3e35f4e02a78a103357c5a68ce3f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20W=C3=BCller?= Date: Mon, 6 Oct 2014 19:56:45 +0200 Subject: [PATCH] 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 uint >= 0, 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. --- doc/api/fs.markdown | 33 +++++-- lib/fs.js | 123 ++++++++++++++++++------ test/simple/test-fs-append-file-sync.js | 15 +++ test/simple/test-fs-append-file.js | 36 ++++++- test/simple/test-fs-readfile-empty.js | 42 ++++++++ test/simple/test-fs-write-file.js | 36 ++++++- 6 files changed, 242 insertions(+), 43 deletions(-) diff --git a/doc/api/fs.markdown b/doc/api/fs.markdown index 4c0f9bf7e8e3..8317b392f765 100644 --- a/doc/api/fs.markdown +++ b/doc/api/fs.markdown @@ -460,9 +460,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} * `encoding` {String | Null} default = `null` * `flag` {String} default = `'r'` @@ -480,18 +480,19 @@ contents of the file. If no encoding is specified, then the raw buffer is returned. +Note that any specified file descriptor needs to support reading. Specified file +descriptors will not be closed automatically. -## fs.readFileSync(filename[, options]) +## fs.readFileSync(file[, options]) -Synchronous version of `fs.readFile`. Returns the contents of the `filename`. +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} * `encoding` {String | Null} default = `'utf8'` @@ -516,9 +517,16 @@ Example: The synchronous version of `fs.writeFile`. Returns `undefined`. -## fs.appendFile(filename, data[, options], callback) +Any specified file descriptor needs to support writing. Specified file +descriptors will not be closed automatically. + +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. + +## fs.appendFile(file, data[, options], callback) -* `filename` {String} +* `file` {String | Integer} filename or file descriptor * `data` {String | Buffer} * `options` {Object} * `encoding` {String | Null} default = `'utf8'` @@ -540,6 +548,13 @@ Example: The synchronous version of `fs.appendFile`. Returns `undefined`. +Note that any specified file descriptor needs to be opened for appending. +Specified file descriptors will not be closed automatically. + +## fs.appendFileSync(file, data[, options]) + +The synchronous version of `fs.appendFile`. Returns `undefined`. + ## fs.watchFile(filename[, options], listener) Stability: 2 - Unstable. Use fs.watch instead, if possible. diff --git a/lib/fs.js b/lib/fs.js index daeb20e747ea..2e4976015224 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -121,6 +121,10 @@ function nullCheck(path, callback) { return true; } +function isFd(path) { + return (path >>> 0) === path && path >= 0; +} + // Static method to set the stats properties on a Stats object. fs.Stats = function( dev, @@ -263,17 +267,23 @@ fs.readFile = function(path, options, callback_) { var buffers; // list for when size is unknown var pos = 0; var fd; + var isUserFd = isFd(path); // file descriptor ownership + + if (isUserFd) { + readFd(null, path); + } else { + var flag = options.flag || 'r'; + fs.open(path, flag, 438 /*=0666*/, readFd); + } + + function readFd(err_, fd_) { + if (err_) return callback(err_); - var flag = options.flag || 'r'; - fs.open(path, flag, 438 /*=0666*/, function(er, fd_) { - if (er) return callback(er); fd = fd_; fs.fstat(fd, function(er, st) { if (er) { - return fs.close(fd, function() { - callback(er); - }); + return error(er); } size = st.size; @@ -287,14 +297,13 @@ fs.readFile = function(path, options, callback_) { if (size > kMaxLength) { var err = new RangeError('File size is greater than possible Buffer: ' + '0x3FFFFFFF bytes'); - return fs.close(fd, function() { - callback(err); - }); + + return error(err); } buffer = new Buffer(size); read(); }); - }); + } function read() { if (size === 0) { @@ -307,9 +316,7 @@ fs.readFile = function(path, options, callback_) { function afterRead(er, bytesRead) { if (er) { - return fs.close(fd, function(er2) { - return callback(er); - }); + return error(er); } if (bytesRead === 0) { @@ -328,7 +335,12 @@ fs.readFile = function(path, options, callback_) { } function close() { - fs.close(fd, function(er) { + if (isUserFd) + afterClose(null); + else + fs.close(fd, afterClose); + + function afterClose(er) { if (size === 0) { // collected the data into the buffers list. buffer = Buffer.concat(buffers, pos); @@ -338,7 +350,17 @@ fs.readFile = function(path, options, callback_) { if (encoding) buffer = buffer.toString(encoding); return callback(er, buffer); - }); + } + } + + function error(er) { + if (isUserFd) { + callback(er); + } else { + fs.close(fd, function() { + callback(er); + }); + } } }; @@ -355,7 +377,8 @@ fs.readFileSync = function(path, options) { assertEncoding(encoding); var flag = options.flag || 'r'; - var fd = fs.openSync(path, flag, 438 /*=0666*/); + var isUserFd = isFd(path); // file descriptor ownership + var fd = isUserFd ? path : fs.openSync(path, flag, 438 /*=0666*/); var size; var threw = true; @@ -363,7 +386,7 @@ fs.readFileSync = function(path, options) { size = fs.fstatSync(fd).size; threw = false; } finally { - if (threw) fs.closeSync(fd); + if (threw && !isUserFd) fs.closeSync(fd); } var pos = 0; @@ -378,7 +401,7 @@ fs.readFileSync = function(path, options) { buffer = new Buffer(size); threw = false; } finally { - if (threw) fs.closeSync(fd); + if (threw && !isUserFd) fs.closeSync(fd); } } @@ -399,14 +422,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. @@ -1048,12 +1072,10 @@ function writeAll(fd, buffer, offset, length, position, callback) { // write(fd, buffer, offset, length, position, callback) fs.write(fd, buffer, offset, length, position, function(writeErr, written) { if (writeErr) { - fs.close(fd, function() { - if (callback) callback(writeErr); - }); + callback(writeErr); } else { if (written === length) { - fs.close(fd, callback); + callback(null); } else { offset += written; length -= written; @@ -1078,16 +1100,41 @@ fs.writeFile = function(path, data, options, callback) { assertEncoding(options.encoding); var flag = options.flag || 'w'; - fs.open(path, flag, options.mode, function(openErr, fd) { + var fd; + var isUserFd; // file descriptor ownership + + if (isFd(path)) { + fd = path; + isUserFd = true; + writeFd(); + return; + } + + fs.open(path, flag, options.mode, function(openErr, fd_) { if (openErr) { - if (callback) callback(openErr); + callback(openErr); } else { - var buffer = util.isBuffer(data) ? data : new Buffer('' + data, - options.encoding || 'utf8'); - var position = /a/.test(flag) ? null : 0; - writeAll(fd, buffer, 0, buffer.length, position, callback); + fd = fd_; + isUserFd = false; + writeFd(); } }); + + function writeFd() { + var buffer = util.isBuffer(data) ? data : new Buffer('' + data, + options.encoding || 'utf8'); + var position = /a/.test(flag) ? null : 0; + + writeAll(fd, buffer, 0, buffer.length, position, function(writeErr) { + if (isUserFd) { + callback(writeErr); + } else { + fs.close(fd, function() { + callback(writeErr); + }); + } + }); + } }; fs.writeFileSync = function(path, data, options) { @@ -1102,7 +1149,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); + var fd = isUserFd ? path : fs.openSync(path, flag, options.mode); + if (!util.isBuffer(data)) { data = new Buffer('' + data, options.encoding || 'utf8'); } @@ -1115,7 +1164,7 @@ fs.writeFileSync = function(path, data, options) { position += written; } } finally { - fs.closeSync(fd); + if (!isUserFd) fs.closeSync(fd); } }; @@ -1132,6 +1181,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); }; @@ -1143,9 +1197,14 @@ fs.appendFileSync = function(path, data, options) { } else if (!util.isObject(options)) { throw new TypeError('Bad arguments'); } + 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); }; diff --git a/test/simple/test-fs-append-file-sync.js b/test/simple/test-fs-append-file-sync.js index 7bc6a0199fa8..8e15a175d704 100644 --- a/test/simple/test-fs-append-file-sync.js +++ b/test/simple/test-fs-append-file-sync.js @@ -90,6 +90,20 @@ 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); + +common.error('appending to ' + filename5); +var filename5fd = fs.openSync(filename5, 'a+'); +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() { @@ -99,4 +113,5 @@ process.on('exit', function() { fs.unlinkSync(filename2); fs.unlinkSync(filename3); fs.unlinkSync(filename4); + fs.unlinkSync(filename5); }); diff --git a/test/simple/test-fs-append-file.js b/test/simple/test-fs-append-file.js index 450c8d3b9e82..dd0b0534c315 100644 --- a/test/simple/test-fs-append-file.js +++ b/test/simple/test-fs-append-file.js @@ -123,12 +123,46 @@ 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++; + common.error('opened file ' + filename5); + + fs.appendFile(fd, s, function(e) { + if (e) throw e; + + ncallbacks++; + common.error('appended to ' + filename5); + + fs.readFile(filename5, function(e, buffer) { + if (e) throw e; + + common.error(filename5 + ' read'); + ncallbacks++; + assert.equal(Buffer.byteLength(s) + currentFileData.length, buffer.length); + + fs.close(fd, function(e) { + if (e) throw e; + + ncallbacks++; + common.error('closed file ' + filename5); + }); + }); + }); +}); + process.on('exit', function() { common.error('done'); - assert.equal(8, ncallbacks); + assert.equal(12, ncallbacks); fs.unlinkSync(filename); fs.unlinkSync(filename2); fs.unlinkSync(filename3); fs.unlinkSync(filename4); + fs.unlinkSync(filename5); }); diff --git a/test/simple/test-fs-readfile-empty.js b/test/simple/test-fs-readfile-empty.js index 31b83567ee44..a429a93ce61e 100644 --- a/test/simple/test-fs-readfile-empty.js +++ b/test/simple/test-fs-readfile-empty.js @@ -30,9 +30,51 @@ fs.readFile(fn, function(err, data) { assert.ok(data); }); +tempFd(function(fd, close) { + fs.readFile(fd, function(err, data) { + if (err) throw err; + assert.ok(data); + close(); + }); +}); + fs.readFile(fn, 'utf8', function(err, data) { assert.strictEqual('', data); }); +tempFd(function(fd, close) { + fs.readFile(fd, 'utf8', function(err, data) { + assert.strictEqual('', data); + close(); + }); +}); + assert.ok(fs.readFileSync(fn)); + +tempFdSync(function(fd) { + assert.ok(fs.readFileSync(fd)); +}); + assert.strictEqual('', fs.readFileSync(fn, 'utf8')); + +tempFdSync(function(fd) { + assert.strictEqual('', fs.readFileSync(fd, 'utf8')); +}); + +function tempFd(callback) { + fs.open(fn, 'r', function(err, fd) { + assert.strictEqual(err, null); + + callback(fd, function() { + fs.close(fd, function(err) { + assert.strictEqual(err, null); + }); + }); + }); +} + +function tempFdSync(callback) { + var fd = fs.openSync(fn, 'r'); + callback(fd); + fs.closeSync(fd); +} diff --git a/test/simple/test-fs-write-file.js b/test/simple/test-fs-write-file.js index b70ea444cf4b..c6e6789d1c60 100644 --- a/test/simple/test-fs-write-file.js +++ b/test/simple/test-fs-write-file.js @@ -97,12 +97,46 @@ 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'); +common.error('writing to ' + filename4); + +fs.open(filename4, 'w+', function(e, fd) { + if (e) throw e; + + ncallbacks++; + common.error('file4 opened'); + + fs.writeFile(fd, buf, function(e) { + if (e) throw e; + + ncallbacks++; + common.error('file4 written'); + + fs.readFile(filename4, function(e, buffer) { + if (e) throw e; + + common.error('file4 read'); + ncallbacks++; + assert.equal(buf.length, buffer.length); + + fs.close(fd, function(e) { + if (e) throw e; + + common.error('file4 read'); + ncallbacks++; + }); + }); + }); +}); process.on('exit', function() { common.error('done'); - assert.equal(6, ncallbacks); + assert.equal(10, ncallbacks); fs.unlinkSync(filename); fs.unlinkSync(filename2); fs.unlinkSync(filename3); + fs.unlinkSync(filename4); });