Skip to content

Commit 446bdb6

Browse files
RaisinTenRaisinTen
committed
fs: change default value of position in read and readSync
Since the docs mention that `position` in `read` and `readSync` should be an `integer` or a `bigint`, the functions should rather accept what is actually fed into `read`, i.e., `-1` instead of any nullish value.
1 parent 01d8b39 commit 446bdb6

5 files changed

Lines changed: 58 additions & 10 deletions

File tree

‎doc/api/fs.md‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2704,6 +2704,9 @@ directory and subsequent read operations.
27042704
<!-- YAML
27052705
added: v0.0.2
27062706
changes:
2707+
- version: REPLACEME
2708+
pr-url: https://github.com/nodejs/node/pull/37101
2709+
description: Runtime description.
27072710
- version: v10.10.0
27082711
pr-url: https://github.com/nodejs/node/pull/22150
27092712
description: The `buffer` parameter can now be any `TypedArray`, or a
@@ -2722,8 +2725,8 @@ changes:
27222725
* `offset` {integer} The position in `buffer` to write the data to.
27232726
* `length` {integer} The number of bytes to read.
27242727
* `position` {integer|bigint} Specifies where to begin reading from in the
2725-
file. If `position` is `null` or `-1 `, data will be read from the current
2726-
file position, and the file position will be updated. If `position` is an
2728+
file. If `position` is `-1 `, data will be read from the current file
2729+
position, and the file position will be updated. If `position` is any other
27272730
integer, the file position will be unchanged.
27282731
* `callback` {Function}
27292732
* `err` {Error}
@@ -2746,6 +2749,9 @@ added:
27462749
- v13.11.0
27472750
- v12.17.0
27482751
changes:
2752+
- version: REPLACEME
2753+
pr-url: https://github.com/nodejs/node/pull/37101
2754+
description: Runtime description.
27492755
- version:
27502756
- v13.11.0
27512757
- v12.17.0
@@ -2758,7 +2764,7 @@ changes:
27582764
* `buffer` {Buffer|TypedArray|DataView} **Default:** `Buffer.alloc(16384)`
27592765
* `offset` {integer} **Default:** `0`
27602766
* `length` {integer} **Default:** `buffer.length`
2761-
* `position` {integer|bigint} **Default:** `null`
2767+
* `position` {integer|bigint} **Default:** `-1`
27622768
* `callback` {Function}
27632769
* `err` {Error}
27642770
* `bytesRead` {integer}

‎lib/fs.js‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,7 @@ function read(fd, buffer, offset, length, position, callback) {
534534
buffer = Buffer.alloc(16384),
535535
offset = 0,
536536
length = buffer.length,
537-
position
537+
position = -1,
538538
} = options);
539539
}
540540

@@ -562,8 +562,13 @@ function read(fd, buffer, offset, length, position, callback) {
562562

563563
validateOffsetLengthRead(offset, length, buffer.byteLength);
564564

565-
if (position == null)
565+
if (position === null) {
566+
process.emitWarning(
567+
`The provided ${position} is not a valid position, and is supported ` +
568+
'in the fs module solely for compatibility.',
569+
'DeprecationWarning', 'DEP01149');
566570
position = -1;
571+
}
567572

568573
validatePosition(position, 'position');
569574

@@ -592,7 +597,11 @@ function readSync(fd, buffer, offset, length, position) {
592597
// Assume fs.read(fd, buffer, options)
593598
const options = offset || {};
594599

595-
({ offset = 0, length = buffer.length, position } = options);
600+
({
601+
offset = 0,
602+
length = buffer.length,
603+
position = -1,
604+
} = options);
596605
}
597606

598607
validateBuffer(buffer);
@@ -616,8 +625,13 @@ function readSync(fd, buffer, offset, length, position) {
616625

617626
validateOffsetLengthRead(offset, length, buffer.byteLength);
618627

619-
if (position == null)
628+
if (position === null) {
629+
process.emitWarning(
630+
`The provided ${position} is not a valid position, and is supported ` +
631+
'in the fs module solely for compatibility.',
632+
'DeprecationWarning', 'DEP0149');
620633
position = -1;
634+
}
621635

622636
validatePosition(position, 'position');
623637

‎test/parallel/test-fs-promisified.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ const exists = promisify(fs.exists);
1111

1212
{
1313
const fd = fs.openSync(__filename, 'r');
14-
read(fd, Buffer.alloc(1024), 0, 1024, null).then(common.mustCall((obj) => {
14+
read(fd, Buffer.alloc(1024), 0, 1024, -1).then(common.mustCall((obj) => {
1515
assert.strictEqual(typeof obj.bytesRead, 'number');
1616
assert(obj.buffer instanceof Buffer);
1717
fs.closeSync(fd);

‎test/parallel/test-fs-read-type.js‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ const filepath = fixtures.path('x.txt');
88
const fd = fs.openSync(filepath, 'r');
99
const expected = 'xyz\n';
1010

11-
1211
// Error must be thrown with string
1312
assert.throws(
1413
() => fs.read(fd, expected.length, 0, 'utf-8', common.mustNotCall()),
@@ -90,6 +89,21 @@ assert.throws(() => {
9089
});
9190
});
9291

92+
{
93+
fs.read(fd,
94+
Buffer.allocUnsafe(expected.length),
95+
0,
96+
expected.length,
97+
null,
98+
common.mustCall());
99+
common.expectWarning(
100+
'DeprecationWarning',
101+
'The provided null is not a valid position, and is supported ' +
102+
'in the fs module solely for compatibility.',
103+
'DEP01149',
104+
);
105+
}
106+
93107
[0.5, 2 ** 53, 2n ** 63n].forEach((value) => {
94108
assert.throws(() => {
95109
fs.read(fd,
@@ -210,6 +224,20 @@ assert.throws(() => {
210224
});
211225
});
212226

227+
{
228+
fs.readSync(fd,
229+
Buffer.allocUnsafe(expected.length),
230+
0,
231+
expected.length,
232+
null);
233+
common.expectWarning(
234+
'DeprecationWarning',
235+
'The provided null is not a valid position, and is supported ' +
236+
'in the fs module solely for compatibility.',
237+
'DEP01149',
238+
);
239+
}
240+
213241
[0.5, 2 ** 53, 2n ** 63n].forEach((value) => {
214242
assert.throws(() => {
215243
fs.readSync(fd,

‎test/parallel/test-fs-readfile-fd.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ function tempFdSync(callback) {
7979
const buf = Buffer.alloc(5);
8080

8181
// Read only five bytes, so that the position moves to five.
82-
fs.read(fd, buf, 0, 5, null, common.mustSucceed((bytes) => {
82+
fs.read(fd, buf, 0, 5, -1, common.mustSucceed((bytes) => {
8383
assert.strictEqual(bytes, 5);
8484
assert.deepStrictEqual(buf.toString(), 'Hello');
8585

0 commit comments

Comments
 (0)