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

Documentation for creating a Buffer which shares a TypedArray's memory is misleading #31348

Closed
qntm opened this issue Jan 13, 2020 · 1 comment
Labels
buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations.

Comments

@qntm
Copy link

qntm commented Jan 13, 2020

  • Version: v12.13.0
  • Platform: Windows 10 64-bit
  • Subsystem: docs

The documentation for Buffer reads, in part (abbreviated for clarity):

It is possible to create a new Buffer that shares the same allocated memory as a TypedArray instance by using the TypedArray object's .buffer property.

const arr = new Uint16Array(2);
arr[0] = 5000;
arr[1] = 4000;
const buf2 = Buffer.from(arr.buffer);
console.log(buf2);
// Prints: <Buffer 88 13 a0 0f>

This works in this case, but I think it's hazardous advice for arbitrary TypedArrays because the backing ArrayBuffer could extend beyond the bounds of the TypedArray which is providing a view of it. For example:

const arrA = Uint8Array.from([0x63, 0x64, 0x65, 0x66]) // 4 elements
const arrB = new Uint8Array(arrA.buffer, 1, 2) // 2 elements
console.log(arrA.buffer === arrB.buffer) // true

const buf = Buffer.from(arrB.buffer)
console.log(buf)
// expected, based on documented advice: <Buffer 64 65>
// actual: <Buffer 63 64 65 66>

It might be better to have the docs suggest Buffer.from(arr.buffer, arr.byteOffset, arr.byteLength) as the general solution to this problem.

@targos targos added the doc Issues and PRs related to the documentations. label Dec 26, 2020
@targos
Copy link
Member

targos commented Dec 26, 2020

I agree. Would you like to open a pull request?

@targos targos added the buffer Issues and PRs related to the buffer subsystem. label Dec 26, 2020
jasnell added a commit to jasnell/node that referenced this issue Jan 4, 2021
Fixes: nodejs#31348
Signed-off-by: James M Snell <jasnell@gmail.com>
@jasnell jasnell closed this as completed in 4be66ca Jan 6, 2021
danielleadams pushed a commit that referenced this issue Jan 12, 2021
Fixes: #31348
Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #36785
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos pushed a commit that referenced this issue May 1, 2021
Fixes: #31348
Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #36785
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@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. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants