fs: throws error if position argument is bigint in fs.readSync & fs.read #36198
Conversation
nodejs-github-bot
added
the
fs
|
hmm... while the changes in this look good, I'm actually wondering if we shouldn't go the other way and actually allow bigints in these apis. |
As @BridgeAR mentioned here, isn't it better to let the user handle it on their own? |
| @@ -543,6 +543,9 @@ function read(fd, buffer, offset, length, position, callback) { | |||
|
|
|||
| validateOffsetLengthRead(offset, length, buffer.byteLength); | |||
|
|
|||
| if (typeof position === 'bigint') | |||
There was a problem hiding this comment.
This seems to be a check that can be performed early instead of being this late? Before validateBuffer would be a good place I think.
There was a problem hiding this comment.
Wouldn't it be better to have arguments validation in order?
There was a problem hiding this comment.
My concern was mostly about the if (length === 0) above which means if the length is 0 you'd be able to get away with these checks..but thinking again it was already the case for a few other cases above, so it's probably not a bit deal.
|
I think this can be closed in favour of #36190 |
Fixes #36185
make -j4 test(UNIX), orvcbuild test(Windows) passes