Skip to content

Commit

Permalink
fs: require callback in read
Browse files Browse the repository at this point in the history
Currently the read operation did not require the callback and that
was an oversight when callbacks became mandatory.

PR-URL: nodejs#22146
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
BridgeAR committed Aug 9, 2018
1 parent 85bfd71 commit 8e1b6e7
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
5 changes: 3 additions & 2 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -445,13 +445,14 @@ function openSync(path, flags, mode) {
function read(fd, buffer, offset, length, position, callback) {
validateUint32(fd, 'fd');
validateBuffer(buffer);
callback = maybeCallback(callback);

offset |= 0;
length |= 0;

if (length === 0) {
return process.nextTick(function tick() {
callback && callback(null, 0, buffer);
callback(null, 0, buffer);
});
}

Expand All @@ -467,7 +468,7 @@ function read(fd, buffer, offset, length, position, callback) {

function wrapper(err, bytesRead) {
// Retain a reference to buffer so that it can't be GC'ed too soon.
callback && callback(err, bytesRead || 0, buffer);
callback(err, bytesRead || 0, buffer);
}

const req = new FSReqCallback();
Expand Down
9 changes: 8 additions & 1 deletion test/parallel/test-fs-read.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ test(new Uint8Array(expected.length),
// Reading beyond file length (3 in this case) should return no data.
// This is a test for a bug where reads > uint32 would return data
// from the current position in the file.
const fd = fs.openSync(filepath, 'r');
const pos = 0xffffffff + 1; // max-uint32 + 1
const nRead = fs.readSync(fd, Buffer.alloc(1), 0, 1, pos);
assert.strictEqual(nRead, 0);
Expand All @@ -68,3 +67,11 @@ test(new Uint8Array(expected.length),
assert.strictEqual(nRead, 0);
}));
}

assert.throws(
() => fs.read(fd, Buffer.alloc(1), 0, 1, 0),
{
message: 'Callback must be a function',
code: 'ERR_INVALID_CALLBACK',
}
);

0 comments on commit 8e1b6e7

Please sign in to comment.