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

Unclear docs on copying/sharing memory for Buffer.from(arrayBuffer) #10770

Closed
zbjornson opened this issue Jan 12, 2017 · 1 comment
Closed

Unclear docs on copying/sharing memory for Buffer.from(arrayBuffer) #10770

zbjornson opened this issue Jan 12, 2017 · 1 comment
Labels
buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations.

Comments

@zbjornson
Copy link
Contributor

  • Version: v7.4.0
  • Subsystem: docs (buffer)

The docs for Buffer.from(arrayBuffer) use the word copy/copying:

  • byteOffset Where to start copying from arrayBuffer. Default: 0
  • length How many bytes to copy from arrayBuffer. Default: arrayBuffer.length - byteOffset

But as far as I can tell, it always references a subarray and never copies.

The line below it:

When passed a reference to the .buffer property of a TypedArray instance, the newly created Buffer will share the same allocated memory as the TypedArray.

is also confusing -- I think that's meant as an example to emphasize the point that it's shared memory, but to me it sounds like there's a situation when it would copy and not share (e.g. Buffer.from(new ArrayBuffer(8)).

Trivial change but wanted to make sure my understanding was correct before submitting a PR. Something like: "byteOffset - element to begin at", "length - number of elements to include" and adding "For example" to the "when passed a reference..." bit.

@addaleax addaleax added buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations. labels Jan 12, 2017
@addaleax
Copy link
Member

Trivial change but wanted to make sure my understanding was correct before submitting a PR.

You’re right, the docs are not correct. Go for it! :)

If you need a better word to use instead of copying, maybe slicing works. Also, this matches the new TypedArray(buffer [, byteOffset [, length]]); form of the TypedArray constructors, so feel free to link to MDN and/or look there for inspiration. :)

zbjornson added a commit to zbjornson/node that referenced this issue Jan 13, 2017
* Clarify that memory is always shared and never copied.
* Fix wording that sounded like ArrayBuffer has a buffer property.

Fixes nodejs#10770
italoacasas pushed a commit that referenced this issue Jan 16, 2017
* Clarify that memory is always shared and never copied.
* Fix wording that sounded like ArrayBuffer has a buffer property.

PR-URL: #10778
Ref: #10770
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 18, 2017
* Clarify that memory is always shared and never copied.
* Fix wording that sounded like ArrayBuffer has a buffer property.

PR-URL: nodejs#10778
Ref: nodejs#10770
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 23, 2017
* Clarify that memory is always shared and never copied.
* Fix wording that sounded like ArrayBuffer has a buffer property.

PR-URL: nodejs#10778
Ref: nodejs#10770
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 24, 2017
* Clarify that memory is always shared and never copied.
* Fix wording that sounded like ArrayBuffer has a buffer property.

PR-URL: nodejs#10778
Ref: nodejs#10770
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 27, 2017
* Clarify that memory is always shared and never copied.
* Fix wording that sounded like ArrayBuffer has a buffer property.

PR-URL: nodejs#10778
Ref: nodejs#10770
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Apr 13, 2017
* Clarify that memory is always shared and never copied.
* Fix wording that sounded like ArrayBuffer has a buffer property.

PR-URL: #10778
Ref: #10770
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Apr 19, 2017
* Clarify that memory is always shared and never copied.
* Fix wording that sounded like ArrayBuffer has a buffer property.

PR-URL: #10778
Ref: #10770
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
andrew749 pushed a commit to michielbaird/node that referenced this issue Jul 19, 2017
* Clarify that memory is always shared and never copied.
* Fix wording that sounded like ArrayBuffer has a buffer property.

PR-URL: nodejs/node#10778
Ref: nodejs/node#10770
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@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

No branches or pull requests

3 participants