Skip to content

Commit

Permalink
fs: add file descriptor support to *File() funcs
Browse files Browse the repository at this point in the history 
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: #3163
Reviewed-By: Trevor Norris <[email protected]>
  • Loading branch information
@jwueller @rvagg
jwueller authored and rvagg committed Oct 19, 2015
1 parent ee2e641 commit 5e0759f
Show file tree
Hide file tree
Showing 7 changed files with 230 additions and 29 deletions.
36 changes: 25 additions & 11 deletions doc/api/fs.markdown
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'`
Expand All @@ -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'`
Expand All @@ -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'`
Expand All @@ -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`.

Expand Down
86 changes: 70 additions & 16 deletions lib/fs.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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);
};

Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
}
}

Expand All @@ -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.
Expand Down Expand Up @@ -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);
}
}
});
Expand All @@ -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) {
Expand All @@ -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');
}
Expand All @@ -1175,7 +1219,7 @@ fs.writeFileSync = function(path, data, options) {
}
}
} finally {
fs.closeSync(fd);
if (!isUserFd) fs.closeSync(fd);
}
};

Expand All @@ -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);
};

Expand All @@ -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);
};

Expand Down
14 changes: 14 additions & 0 deletions test/parallel/test-fs-append-file-sync.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,25 @@ 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() {
fs.unlinkSync(filename);
fs.unlinkSync(filename2);
fs.unlinkSync(filename3);
fs.unlinkSync(filename4);
fs.unlinkSync(filename5);
});
33 changes: 32 additions & 1 deletion test/parallel/test-fs-append-file.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Loading

0 comments on commit 5e0759f

Please sign in to comment.