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

n-api: does napi_create_external_buffer copy? #33471

Closed
mafintosh opened this issue May 19, 2020 · 3 comments
Closed

n-api: does napi_create_external_buffer copy? #33471

mafintosh opened this issue May 19, 2020 · 3 comments
Labels
doc Issues and PRs related to the documentations. node-api Issues and PRs related to the Node-API.

Comments

@mafintosh
Copy link
Member

Hunting down a bug related to napi_create_external_buffer, I got a bit confused by reading the docs here:

https://nodejs.org/dist/latest-v14.x/docs/api/n-api.html#n_api_napi_create_external_buffer

[in] data: Raw pointer to the underlying buffer to copy from.

Does this mean that the buffer is created with the same pointer or a new pointer that's a memcopy?

Further down on the docs for the same method it also says:

This API allocates a node::Buffer object and initializes it with data backed by the passed in buffer.

Which I'm also not sure wheather to interpret as a memcopy

@mafintosh mafintosh added the doc Issues and PRs related to the documentations. label May 19, 2020
@mafintosh
Copy link
Member Author

Did some testing and it seems to be not memcopying it. If other people agree that this is the correct behaivour and that the docs are confusing I can volunteer a PR

@addaleax addaleax added the node-api Issues and PRs related to the Node-API. label May 19, 2020
@addaleax
Copy link
Member

Yes, this is an error in the docs, it does not copy (and there would be no point in doing so for externalized buffers anyway).

@mhdawson
Copy link
Member

@mafintosh a PR for the docs would be great.

gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this issue Jun 29, 2020
Remove text regarding copying, because `napi_create_external_buffer`
does not copy.

Fixes: nodejs#33471
@jasnell jasnell closed this as completed in cfa3d8f Jul 3, 2020
MylesBorins pushed a commit that referenced this issue Jul 14, 2020
Remove text regarding copying, because `napi_create_external_buffer`
does not copy.

Fixes: #33471

PR-URL: #34125
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Mathias Buus <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue Jul 16, 2020
Remove text regarding copying, because `napi_create_external_buffer`
does not copy.

Fixes: #33471

PR-URL: #34125
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Mathias Buus <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit that referenced this issue Sep 22, 2020
Remove text regarding copying, because `napi_create_external_buffer`
does not copy.

Fixes: #33471

PR-URL: #34125
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Mathias Buus <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants