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.

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

Buffer.byteLength's implementation is not consistent with the documentation #38536

Closed
pd4d10 opened this issue May 4, 2021 · 2 comments · Fixed by #38545
Closed

Buffer.byteLength's implementation is not consistent with the documentation #38536

pd4d10 opened this issue May 4, 2021 · 2 comments · Fixed by #38545
Labels
buffer Issues and PRs related to the buffer subsystem.

Comments

@pd4d10
Copy link
Contributor

pd4d10 commented May 4, 2021

node/lib/buffer.js

Lines 729 to 752 in f71166b

function byteLength(string, encoding) {
if (typeof string !== 'string') {
if (isArrayBufferView(string) || isAnyArrayBuffer(string)) {
return string.byteLength;
}
throw new ERR_INVALID_ARG_TYPE(
'string', ['string', 'Buffer', 'ArrayBuffer'], string
);
}
const len = string.length;
const mustMatch = (arguments.length > 2 && arguments[2] === true);
if (!mustMatch && len === 0)
return 0;
if (!encoding)
return (mustMatch ? -1 : byteLengthUtf8(string));
const ops = getEncodingOps(encoding);
if (ops === undefined)
return (mustMatch ? -1 : byteLengthUtf8(string));
return ops.byteLength(string);
}

In line 741 it checks the third argument mustMatch, which is not mentioned in the document

So I guess either the implementation or the documentation should be updated.

@Trott
Copy link
Member

Trott commented May 5, 2021

@nodejs/buffer @nodejs/documentation

@mscdex
Copy link
Contributor

mscdex commented May 5, 2021

I believe it was part of a now outdated optimization. I think it can be removed from the code (the parts where mustMatch would've been true).

pd4d10 added a commit to pd4d10/node that referenced this issue May 5, 2021
The third argument `mustMatch` is now outdated, so remove it.

Fixes: nodejs#38536
@Ayase-252 Ayase-252 added the buffer Issues and PRs related to the buffer subsystem. label May 7, 2021
Trott added a commit to Trott/io.js that referenced this issue Mar 26, 2022
Buffer.bytelength() has an undocumented third parameterm `mustMatch`.
Remove it.

Closes: nodejs#38536
Trott pushed a commit to pd4d10/node that referenced this issue Mar 29, 2022
The third argument `mustMatch` is now outdated, so remove it.

Fixes: nodejs#38536
nodejs-github-bot pushed a commit that referenced this issue Mar 29, 2022
The third argument `mustMatch` is now outdated, so remove it.

Fixes: #38536

PR-URL: #38545
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
xtx1130 pushed a commit to xtx1130/node that referenced this issue Apr 25, 2022
The third argument `mustMatch` is now outdated, so remove it.

Fixes: nodejs#38536

PR-URL: nodejs#38545
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem.
Projects
None yet
4 participants