Skip to content

Commit

Permalink
fs: add read(buffer[, options]) versions
Browse files Browse the repository at this point in the history
This adds the following:
- `fs.read(fd, buffer[, options], callback)`.
- `filehandle.read(buffer[, options])`.

PR-URL: nodejs/node#42768
Refs: nodejs/node#42601
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
LiviaMedeiros authored and guangwong committed Oct 10, 2022
1 parent 9bb6b23 commit 8fe30cb
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 54 deletions.
49 changes: 49 additions & 0 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,33 @@ Reads data from the file and stores that in the given buffer.
If the file is not modified concurrently, the end-of-file is reached when the
number of bytes read is zero.
#### `filehandle.read(buffer[, options])`
<!-- YAML
added: REPLACEME
-->
* `buffer` {Buffer|TypedArray|DataView} A buffer that will be filled with the
file data read.
* `options` {Object}
* `offset` {integer} The location in the buffer at which to start filling.
**Default:** `0`
* `length` {integer} The number of bytes to read. **Default:**
`buffer.byteLength - offset`
* `position` {integer} The location where to begin reading data from the
file. If `null`, data will be read from the current file position, and
the position will be updated. If `position` is an integer, the current
file position will remain unchanged. **Default:**: `null`
* Returns: {Promise} Fulfills upon success with an object with two properties:
* `bytesRead` {integer} The number of bytes read
* `buffer` {Buffer|TypedArray|DataView} A reference to the passed in `buffer`
argument.
Reads data from the file and stores that in the given buffer.
If the file is not modified concurrently, the end-of-file is reached when the
number of bytes read is zero.
#### `filehandle.readFile(options)`
<!-- YAML
Expand Down Expand Up @@ -3088,6 +3115,28 @@ Similar to the [`fs.read()`][] function, this version takes an optional
`options` object. If no `options` object is specified, it will default with the
above values.
### `fs.read(fd, buffer[, options], callback)`
<!-- YAML
added: REPLACEME
-->
* `fd` {integer}
* `buffer` {Buffer|TypedArray|DataView} The buffer that the data will be
written to.
* `options` {Object}
* `offset` {integer} **Default:** `0`
* `length` {integer} **Default:** `buffer.byteLength - offset`
* `position` {integer|bigint} **Default:** `null`
* `callback` {Function}
* `err` {Error}
* `bytesRead` {integer}
* `buffer` {Buffer}
Similar to the [`fs.read()`][] function, this version takes an optional
`options` object. If no `options` object is specified, it will default with the
above values.
### `fs.readdir(path[, options], callback)`
<!-- YAML
Expand Down
37 changes: 22 additions & 15 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ const {
validateEncoding,
validateFunction,
validateInteger,
validateObject,
validateString,
} = require('internal/validators');

Expand Down Expand Up @@ -591,7 +592,7 @@ function openSync(path, flags, mode) {
* Reads file from the specified `fd` (file descriptor).
* @param {number} fd
* @param {Buffer | TypedArray | DataView} buffer
* @param {number} offset
* @param {number} offsetOrOptions
* @param {number} length
* @param {number | bigint | null} position
* @param {(
Expand All @@ -601,30 +602,36 @@ function openSync(path, flags, mode) {
* ) => any} callback
* @returns {void}
*/
function read(fd, buffer, offset, length, position, callback) {
function read(fd, buffer, offsetOrOptions, length, position, callback) {
fd = getValidatedFd(fd);

if (arguments.length <= 3) {
// Assume fs.read(fd, options, callback)
let options = ObjectCreate(null);
if (arguments.length < 3) {
let offset = offsetOrOptions;
let params = null;
if (arguments.length <= 4) {
if (arguments.length === 4) {
// This is fs.read(fd, buffer, options, callback)
validateObject(offsetOrOptions, 'options', { nullable: true });
callback = length;
params = offsetOrOptions;
} else if (arguments.length === 3) {
// This is fs.read(fd, bufferOrParams, callback)
if (!isArrayBufferView(buffer)) {
// This is fs.read(fd, params, callback)
params = buffer;
({ buffer = Buffer.alloc(16384) } = params ?? ObjectCreate(null));
}
callback = offsetOrOptions;
} else {
// This is fs.read(fd, callback)
// buffer will be the callback
callback = buffer;
} else {
// This is fs.read(fd, {}, callback)
// buffer will be the options object
// offset is the callback
options = buffer;
callback = offset;
buffer = Buffer.alloc(16384);
}

({
buffer = Buffer.alloc(16384),
offset = 0,
length = buffer.byteLength - offset,
position = null
} = options);
} = params ?? ObjectCreate(null));
}

validateBuffer(buffer);
Expand Down
17 changes: 13 additions & 4 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -456,20 +456,29 @@ async function open(path, flags, mode) {
flagsNumber, mode, kUsePromises));
}

async function read(handle, bufferOrOptions, offset, length, position) {
let buffer = bufferOrOptions;
async function read(handle, bufferOrParams, offset, length, position) {
let buffer = bufferOrParams;
if (!isArrayBufferView(buffer)) {
bufferOrOptions ??= ObjectCreate(null);
// This is fh.read(params)
({
buffer = Buffer.alloc(16384),
offset = 0,
length = buffer.byteLength - offset,
position = null
} = bufferOrOptions);
} = bufferOrParams ?? ObjectCreate(null));

validateBuffer(buffer);
}

if (offset !== null && typeof offset === 'object') {
// This is fh.read(buffer, options)
({
offset = 0,
length = buffer.byteLength - offset,
position = null
} = offset ?? ObjectCreate(null));
}

if (offset == null) {
offset = 0;
} else {
Expand Down
44 changes: 33 additions & 11 deletions test/parallel/test-fs-read-offset-null.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,29 +14,51 @@ const filepath = fixtures.path('x.txt');
const buf = Buffer.alloc(1);
// Reading only one character, hence buffer of one byte is enough.

// Test for callback API.
// Tests are done by making sure the first letter in buffer is
// same as first letter in file.
// 120 is the ascii code of letter x.

// Tests for callback API.
fs.open(filepath, 'r', common.mustSucceed((fd) => {
fs.read(fd, { offset: null, buffer: buf },
common.mustSucceed((bytesRead, buffer) => {
// Test is done by making sure the first letter in buffer is
// same as first letter in file.
// 120 is the hex for ascii code of letter x.
assert.strictEqual(buffer[0], 120);
fs.close(fd, common.mustSucceed(() => {}));
}));
}));

fs.open(filepath, 'r', common.mustSucceed((fd) => {
fs.read(fd, buf, { offset: null },
common.mustSucceed((bytesRead, buffer) => {
assert.strictEqual(buffer[0], 120);
fs.close(fd, common.mustSucceed(() => {}));
}));
}));

let filehandle = null;

// Test for promise api
// Tests for promises api
(async () => {
filehandle = await fsPromises.open(filepath, 'r');
const readObject = await filehandle.read(buf, { offset: null });
assert.strictEqual(readObject.buffer[0], 120);
})()
.finally(() => filehandle?.close())
.then(common.mustCall());

// Undocumented: omitted position works the same as position === null
(async () => {
filehandle = await fsPromises.open(filepath, 'r');
const readObject = await filehandle.read(buf, null, buf.length);
assert.strictEqual(readObject.buffer[0], 120);
})()
.then(common.mustCall())
.finally(async () => {
// Close the file handle if it is opened
if (filehandle)
await filehandle.close();
});
.finally(() => filehandle?.close())
.then(common.mustCall());

(async () => {
filehandle = await fsPromises.open(filepath, 'r');
const readObject = await filehandle.read(buf, null, buf.length, 0);
assert.strictEqual(readObject.buffer[0], 120);
})()
.finally(() => filehandle?.close())
.then(common.mustCall());
46 changes: 24 additions & 22 deletions test/parallel/test-fs-read-optional-params.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,34 @@ const fixtures = require('../common/fixtures');
const fs = require('fs');
const assert = require('assert');
const filepath = fixtures.path('x.txt');
const fd = fs.openSync(filepath, 'r');

const expected = Buffer.from('xyz\n');
const defaultBufferAsync = Buffer.alloc(16384);
const bufferAsOption = Buffer.allocUnsafe(expected.length);
const bufferAsOption = Buffer.allocUnsafe(expected.byteLength);

// Test not passing in any options object
fs.read(fd, common.mustCall((err, bytesRead, buffer) => {
assert.strictEqual(bytesRead, expected.length);
assert.deepStrictEqual(defaultBufferAsync.length, buffer.length);
}));
function testValid(message, ...options) {
const paramsMsg = `${message} (as params)`;
const paramsFilehandle = fs.openSync(filepath, 'r');
fs.read(paramsFilehandle, ...options, common.mustSucceed((bytesRead, buffer) => {
assert.strictEqual(bytesRead, expected.byteLength, paramsMsg);
assert.deepStrictEqual(defaultBufferAsync.byteLength, buffer.byteLength, paramsMsg);
fs.closeSync(paramsFilehandle);
}));

// Test passing in an empty options object
fs.read(fd, { position: 0 }, common.mustCall((err, bytesRead, buffer) => {
assert.strictEqual(bytesRead, expected.length);
assert.deepStrictEqual(defaultBufferAsync.length, buffer.length);
}));
const optionsMsg = `${message} (as options)`;
const optionsFilehandle = fs.openSync(filepath, 'r');
fs.read(optionsFilehandle, bufferAsOption, ...options, common.mustSucceed((bytesRead, buffer) => {
assert.strictEqual(bytesRead, expected.byteLength, optionsMsg);
assert.deepStrictEqual(bufferAsOption.byteLength, buffer.byteLength, optionsMsg);
fs.closeSync(optionsFilehandle);
}));
}

// Test passing in options
fs.read(fd, {
buffer: bufferAsOption,
testValid('Not passing in any object');
testValid('Passing in a null', null);
testValid('Passing in an empty object', {});
testValid('Passing in an object', {
offset: 0,
length: bufferAsOption.length,
position: 0
},
common.mustCall((err, bytesRead, buffer) => {
assert.strictEqual(bytesRead, expected.length);
assert.deepStrictEqual(bufferAsOption.length, buffer.length);
}));
length: bufferAsOption.byteLength,
position: 0,
});
12 changes: 10 additions & 2 deletions test/parallel/test-fs-read-promises-optional-params.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,18 @@ const fd = fs.openSync(filepath, 'r');

const expected = Buffer.from('xyz\n');
const defaultBufferAsync = Buffer.alloc(16384);
const bufferAsOption = Buffer.allocUnsafe(expected.byteLength);

read(fd, {})
.then(function({ bytesRead, buffer }) {
assert.strictEqual(bytesRead, expected.length);
assert.deepStrictEqual(defaultBufferAsync.length, buffer.length);
assert.strictEqual(bytesRead, expected.byteLength);
assert.deepStrictEqual(defaultBufferAsync.byteLength, buffer.byteLength);
})
.then(common.mustCall());

read(fd, bufferAsOption, { position: 0 })
.then(function({ bytesRead, buffer }) {
assert.strictEqual(bytesRead, expected.byteLength);
assert.deepStrictEqual(bufferAsOption.byteLength, buffer.byteLength);
})
.then(common.mustCall());

0 comments on commit 8fe30cb

Please sign in to comment.