Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fs: Add file descriptor support to *File() funcs #3163

Closed
wants to merge 1 commit into from
Closed

fs: Add file descriptor support to *File() funcs #3163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 25 additions & 11 deletions doc/api/fs.markdown
View file
Open in desktop
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind adding that data written to fd will be appended?

Guess we don't expose an lseek() in fs so users couldn't reposition where data is written. Hm. Future feature.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nm. i see what's happening.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whether data will be appended depends on the flags that the fd was created with. The a flags will obviously append, but the w flags will create/truncate instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup. I crossed the following code from fs.append:

  if (isFd(path))
    options.flag = 'a';

Sorry


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])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that you have the note in fs.writeFileSync, but not here in fs.appendFileSync. Is it necessary here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what you are asking, exactly. The note is present in all asynchronous versions, while being omitted in the synchronous ones, since the latter segments only refer to the asynchronous documentation anyway.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoop. Sorry. Got lost in the diff. Can see that now.


The synchronous version of `fs.appendFile`. Returns `undefined`.

Expand Down
86 changes: 70 additions & 16 deletions lib/fs.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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