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.

Sign up for GitHub

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

Jump to bottom

New Buffer API backport to v4.x (staging branch) #7562

Closed
wants to merge 2 commits into from
Closed

New Buffer API backport to v4.x (staging branch) #7562

wants to merge 2 commits into from

Conversation

ChALkeR
Copy link
Member

@ChALkeR ChALkeR commented Jul 6, 2016

Moved from #7475, retargeted to v4.x-staging branch.

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

buffer

Description of change

This backports the new Buffer.alloc(), Buffer.allocUnsafe(), Buffer.from(), and Buffer.allocUnsafeSlow() APIs for v4.

Note: some backported tests are disabled, but those are not related to the new API.

Important: Buffer.from(arrayBuffer[, byteOffset [, length]]) (as in 6.x) was not backported, I used Buffer.from(arrayBuffer) instead, that helped to significantly reduce the code changes.

Note that --zero-fill-buffers is not included here (see #5745 for that), also the encoding parameter for .fill() is not included.

Existing code changes are minor here, this is not invasive.

Only two changes in existing code:

  1. .fill('') now zero-fills,

  2. One call to Buffer(string, encoding) is replaced with Buffer.from.

    There is a typecheck in that code path, so the behaviour is identical. In fact it could be reverted back to Buffer(string, encoding) or even changed to fromString(string, encoding) — that would just affect the double checks.

All the other changes are additive, i.e. introduce new methods and don't change the existing ones.

@ChALkeR ChALkeR added buffer Issues and PRs related to the buffer subsystem. semver-minor PRs that contain new features and should be released in the next minor version. labels Jul 6, 2016
@ChALkeR ChALkeR mentioned this pull request Jul 6, 2016
Closed
4 tasks
@ChALkeR
Copy link
Member Author

ChALkeR commented Jul 6, 2016 •
edited
Loading

/cc @thealphanerd @nodejs/lts

Commit messages amended to include LGTMs and correct links, one comment removed that @trevnorris noted in #7475 (comment).

Should be good to land now.

@mscdex mscdex added the v4.x label Jul 6, 2016
rvagg
rvagg reviewed Jul 7, 2016
View reviewed changes
doc/api/buffer.md

* [`Buffer.from(array)`][buffer_from_array] returns a new `Buffer` containing
a *copy* of the provided octets.
* [`Buffer.from(arrayBuffer[, byteOffset [, length]])`][buffer_from_arraybuf]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs to be changed to remove arg 2 and 3?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argh, missed that. Will fix soon, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@rvagg
Copy link
Member

rvagg commented Jul 7, 2016

some backported tests are disabled, but those are not related to the new API.

I can't see any disabled tests, do you mean they are simply not included or am I not seeing them?

CI @ https://ci.nodejs.org/job/node-test-commit/4003/

Linter is failing on one of the tests, https://ci.nodejs.org/job/node-test-linter/3196/console

@ChALkeR
Copy link
Member Author

ChALkeR commented Jul 7, 2016 •
edited
Loading

@rvagg

I can't see any disabled tests, do you mean they are simply not included or am I not seeing them?

Grep for // fails on 4.x in the test/parallel/test-buffer-alloc.js file

Linter is failing on one of the tests

Hm. I guess I somehow missed that. Will fix, thanks!

@rvagg
Copy link
Member

rvagg commented Jul 7, 2016

ahh, not whole tests, individual assertions, gotcha 👍

addaleax and others added 2 commits July 8, 2016 06:36
@addaleax @ChALkeR
buffer: ignore negative allocation lengths
18b5ef9 
Treat negative length arguments to `Buffer()`/`allocUnsafe()`
as if they were zero so the allocation does not affect the
pool’s offset.

Fixes: nodejs#7047

Refs: nodejs#7051
Refs: nodejs#7221
Refs: nodejs#7475
PR-URL: nodejs#7562
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Nikolai Vavilov <[email protected]>
@ChALkeR
buffer: backport new buffer constructor APIs to v4.x
1b39ebc 
This backports the new `Buffer.alloc()`, `Buffer.allocUnsafe()`,
`Buffer.from()`, and `Buffer.allocUnsafeSlow()` APIs for v4.

Some backported tests are disabled, but those are not related to the
new API.

Note that `Buffer.from(arrayBuffer[, byteOffset [, length]])` is not
supported in v4.x, only `Buffer.from(arrayBuffer)` is.

Refs: nodejs#4682
Refs: nodejs#5833
Refs: nodejs#7475
PR-URL: nodejs#7562
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Nikolai Vavilov <[email protected]>
@ChALkeR
Copy link
Member Author

ChALkeR commented Jul 8, 2016

@rvagg Fixed! =)

@ChALkeR
Copy link
Member Author

ChALkeR commented Jul 8, 2016

New CI: https://ci.nodejs.org/job/node-test-pull-request/3212/

@rvagg
Copy link
Member

rvagg commented Jul 8, 2016

beautiful

lgtm

@jasnell
Copy link
Member

jasnell commented Jul 8, 2016

LGTM

@ChALkeR
Copy link
Member Author

ChALkeR commented Jul 8, 2016 •
edited
Loading

@thealphanerd @nodejs/lts, should I land this?

MylesBorins pushed a commit that referenced this pull request Jul 8, 2016
@addaleax
buffer: ignore negative allocation lengths
9ef71ab 
Treat negative length arguments to `Buffer()`/`allocUnsafe()`
as if they were zero so the allocation does not affect the
pool’s offset.

Fixes: #7047

Refs: #7051
Refs: #7221
Refs: #7475
PR-URL: #7562
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Nikolai Vavilov <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jul 8, 2016
@ChALkeR
buffer: backport new buffer constructor APIs to v4.x
34776ec 
This backports the new `Buffer.alloc()`, `Buffer.allocUnsafe()`,
`Buffer.from()`, and `Buffer.allocUnsafeSlow()` APIs for v4.

Some backported tests are disabled, but those are not related to the
new API.

Note that `Buffer.from(arrayBuffer[, byteOffset [, length]])` is not
supported in v4.x, only `Buffer.from(arrayBuffer)` is.

Refs: #4682
Refs: #5833
Refs: #7475
PR-URL: #7562
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Nikolai Vavilov <[email protected]>
@MylesBorins
Copy link
Contributor

landed in 350e8c6...34776ec

@MylesBorins MylesBorins closed this Jul 8, 2016
@ChALkeR
Copy link
Member Author

ChALkeR commented Jul 8, 2016

@thealphanerd Thanks!

@seishun seishun mentioned this pull request Jul 8, 2016
Closed
2 tasks
MylesBorins pushed a commit that referenced this pull request Jul 11, 2016
@addaleax
buffer: ignore negative allocation lengths
ee2f08f 
Treat negative length arguments to `Buffer()`/`allocUnsafe()`
as if they were zero so the allocation does not affect the
pool’s offset.

Fixes: #7047

Refs: #7051
Refs: #7221
Refs: #7475
PR-URL: #7562
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Nikolai Vavilov <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jul 11, 2016
@ChALkeR
buffer: backport new buffer constructor APIs to v4.x
b4a2791 
This backports the new `Buffer.alloc()`, `Buffer.allocUnsafe()`,
`Buffer.from()`, and `Buffer.allocUnsafeSlow()` APIs for v4.

Some backported tests are disabled, but those are not related to the
new API.

Note that `Buffer.from(arrayBuffer[, byteOffset [, length]])` is not
supported in v4.x, only `Buffer.from(arrayBuffer)` is.

Refs: #4682
Refs: #5833
Refs: #7475
PR-URL: #7562
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Nikolai Vavilov <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jul 12, 2016
Closed
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
@addaleax
buffer: ignore negative allocation lengths
022439c 
Treat negative length arguments to `Buffer()`/`allocUnsafe()`
as if they were zero so the allocation does not affect the
pool’s offset.

Fixes: #7047

Refs: #7051
Refs: #7221
Refs: #7475
PR-URL: #7562
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Nikolai Vavilov <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
@ChALkeR
buffer: backport new buffer constructor APIs to v4.x
ef3687f 
This backports the new `Buffer.alloc()`, `Buffer.allocUnsafe()`,
`Buffer.from()`, and `Buffer.allocUnsafeSlow()` APIs for v4.

Some backported tests are disabled, but those are not related to the
new API.

Note that `Buffer.from(arrayBuffer[, byteOffset [, length]])` is not
supported in v4.x, only `Buffer.from(arrayBuffer)` is.

Refs: #4682
Refs: #5833
Refs: #7475
PR-URL: #7562
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Nikolai Vavilov <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
2016-07-12, Version 4.5.0 'Argon' (LTS)
3103a79 
Notable Changes:

This list is not yet complete. Please comment in the thread with
commits you think should be included. Descriptions will also be
updated in a later release candidate.

Semver Minor:

* buffer:
 * backport new buffer constructor APIs to v4.x
   (Сковорода Никита Андреевич)
   #7562
 * ignore negative allocation lengths (Anna Henningsen)
   #7562
 * backport --zero-fill-buffers cli option (James M Snell)
   #5745
* build:
  * add Intel Vtune profiling support (Chunyang Dai)
    #5527
* repl:
  * copying tabs shouldn't trigger completion (Eugene Obrezkov)
    #5958
* src:
  * add node::FreeEnvironment public API (Cheng Zhao)
    #3098
* test:
  * run v8 tests from node tree (Bryon Leung)
    #4704
* V8:
  * backport 9c927d0f01 from V8 upstream (Myles Borins)
    #7451
  * cherry-pick 68e89fb from v8's upstream (Fedor Indutny)
    #3779

Semver Patch:

* **libuv**:
  * upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé)
    #6796
  * upgrade libuv to 1.9.0 (Saúl Ibarra Corretgé)
    #5994
@MylesBorins MylesBorins mentioned this pull request Jul 12, 2016
Merged
MylesBorins pushed a commit that referenced this pull request Jul 13, 2016
2016-07-12, Version 4.5.0 'Argon' (LTS)
79dd0df 
Notable Changes:

This list is not yet complete. Please comment in the thread with
commits you think should be included. Descriptions will also be
updated in a later release candidate.

Semver Minor:

* buffer:
 * backport new buffer constructor APIs to v4.x
   (Сковорода Никита Андреевич)
   #7562
 * ignore negative allocation lengths (Anna Henningsen)
   #7562
 * backport --zero-fill-buffers cli option (James M Snell)
   #5745
* build:
  * add Intel Vtune profiling support (Chunyang Dai)
    #5527
* repl:
  * copying tabs shouldn't trigger completion (Eugene Obrezkov)
    #5958
* src:
  * add node::FreeEnvironment public API (Cheng Zhao)
    #3098
* test:
  * run v8 tests from node tree (Bryon Leung)
    #4704
* V8:
  * Add post portem data to imrpove object inspection and function's
    context variables inspection (Fedor Indutny)
    #3779

Semver Patch:

* **libuv**:
  * upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé)
    #6796
  * upgrade libuv to 1.9.0 (Saúl Ibarra Corretgé)
    #5994
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
@addaleax
buffer: ignore negative allocation lengths
725c8c4 
Treat negative length arguments to `Buffer()`/`allocUnsafe()`
as if they were zero so the allocation does not affect the
pool’s offset.

Fixes: #7047

Refs: #7051
Refs: #7221
Refs: #7475
PR-URL: #7562
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Nikolai Vavilov <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
@ChALkeR
buffer: backport new buffer constructor APIs to v4.x
12d9769 
This backports the new `Buffer.alloc()`, `Buffer.allocUnsafe()`,
`Buffer.from()`, and `Buffer.allocUnsafeSlow()` APIs for v4.

Some backported tests are disabled, but those are not related to the
new API.

Note that `Buffer.from(arrayBuffer[, byteOffset [, length]])` is not
supported in v4.x, only `Buffer.from(arrayBuffer)` is.

Refs: #4682
Refs: #5833
Refs: #7475
PR-URL: #7562
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Nikolai Vavilov <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
@addaleax
buffer: ignore negative allocation lengths
200429e 
Treat negative length arguments to `Buffer()`/`allocUnsafe()`
as if they were zero so the allocation does not affect the
pool’s offset.

Fixes: #7047

Refs: #7051
Refs: #7221
Refs: #7475
PR-URL: #7562
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Nikolai Vavilov <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
@ChALkeR
buffer: backport new buffer constructor APIs to v4.x
7090481 
This backports the new `Buffer.alloc()`, `Buffer.allocUnsafe()`,
`Buffer.from()`, and `Buffer.allocUnsafeSlow()` APIs for v4.

Some backported tests are disabled, but those are not related to the
new API.

Note that `Buffer.from(arrayBuffer[, byteOffset [, length]])` is not
supported in v4.x, only `Buffer.from(arrayBuffer)` is.

Refs: #4682
Refs: #5833
Refs: #7475
PR-URL: #7562
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Nikolai Vavilov <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
2016-07-14, Version 4.5.0 'Argon' (LTS)
1dbc727 
Notable Changes:

This list is not yet complete. Please comment in the thread with
commits you think should be included. Descriptions will also be
updated in a later release candidate.

Semver Minor:

* buffer:
 * backport new buffer constructor APIs to v4.x
   (Сковорода Никита Андреевич)
   #7562
 * backport --zero-fill-buffers cli option (James M Snell)
   #5745
* build:
  * add Intel Vtune profiling support (Chunyang Dai)
    #5527
* repl:
  * copying tabs shouldn't trigger completion (Eugene Obrezkov)
    #5958
* src:
  * add node::FreeEnvironment public API (Cheng Zhao)
    #3098
* test:
  * run v8 tests from node tree (Bryon Leung)
    #4704
* V8:
  * Add post portem data to imrpove object inspection and function's
    context variables inspection (Fedor Indutny)
    #3779

Semver Patch:

* **buffer**:
  * ignore negative allocation lengths (Anna Henningsen)
    #7562
* **libuv**:
  * upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé)
    #6796
  * upgrade libuv to 1.9.0 (Saúl Ibarra Corretgé)
    #5994
* **npm**:
  * upgrade to 2.15.9 (Kat Marchán)
    #7692
@ChALkeR ChALkeR mentioned this pull request Jul 29, 2016
Closed
MylesBorins pushed a commit that referenced this pull request Aug 15, 2016
2016-07-14, Version 4.5.0 'Argon' (LTS)
20ebcf5 
Notable Changes:

Semver Minor:

* buffer:
 * backport new buffer constructor APIs to v4.x
   (Сковорода Никита Андреевич)
   #7562
 * backport --zero-fill-buffers cli option (James M Snell)
   #5745
* build:
  * add Intel Vtune profiling support (Chunyang Dai)
    #5527
* repl:
  * copying tabs shouldn't trigger completion (Eugene Obrezkov)
    #5958
* src:
  * add node::FreeEnvironment public API (Cheng Zhao)
    #3098
* test:
  * run v8 tests from node tree (Bryon Leung)
    #4704
* V8:
  * Add post mortem data to improve object inspection and function's
    context variables inspection (Fedor Indutny)
    #3779

Semver Patch:

* **buffer**:
  * ignore negative allocation lengths (Anna Henningsen)
    #7562
* **crypto**:
  * update root certificates (Ben Noordhuis)
    #7363
* **libuv**:
  * upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé)
    #6796
  * upgrade libuv to 1.9.0 (Saúl Ibarra Corretgé)
    #5994
* **npm**:
  * upgrade to 2.15.9 (Kat Marchán)
    #7692
MylesBorins pushed a commit that referenced this pull request Aug 16, 2016
2016-08-15, Version 4.5.0 'Argon' (LTS)
50577fa 
Notable Changes:

Semver Minor:

* buffer:
 * backport new buffer constructor APIs to v4.x
   (Сковорода Никита Андреевич)
   #7562
 * backport --zero-fill-buffers cli option (James M Snell)
   #5745
* build:
  * add Intel Vtune profiling support (Chunyang Dai)
    #5527
* repl:
  * copying tabs shouldn't trigger completion (Eugene Obrezkov)
    #5958
* src:
  * add node::FreeEnvironment public API (Cheng Zhao)
    #3098
* test:
  * run v8 tests from node tree (Bryon Leung)
    #4704
* V8:
  * Add post mortem data to improve object inspection and function's
    context variables inspection (Fedor Indutny)
    #3779

Semver Patch:

* **buffer**:
  * ignore negative allocation lengths (Anna Henningsen)
    #7562
* **crypto**:
  * update root certificates (Ben Noordhuis)
    #7363
* **libuv**:
  * upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé)
    #6796
  * upgrade libuv to 1.9.0 (Saúl Ibarra Corretgé)
    #5994
* **npm**:
  * upgrade to 2.15.9 (Kat Marchán)
    #7692
MylesBorins pushed a commit that referenced this pull request Aug 16, 2016
2016-08-15, Version 4.5.0 'Argon' (LTS)
e7bd4d2 
Notable Changes:

Semver Minor:

* buffer:
 * backport new buffer constructor APIs to v4.x
   (Сковорода Никита Андреевич)
   #7562
 * backport --zero-fill-buffers cli option (James M Snell)
   #5745
* build:
  * add Intel Vtune profiling support (Chunyang Dai)
    #5527
* repl:
  * copying tabs shouldn't trigger completion (Eugene Obrezkov)
    #5958
* src:
  * add node::FreeEnvironment public API (Cheng Zhao)
    #3098
* test:
  * run v8 tests from node tree (Bryon Leung)
    #4704
* V8:
  * Add post mortem data to improve object inspection and function's
    context variables inspection (Fedor Indutny)
    #3779

Semver Patch:

* **buffer**:
  * ignore negative allocation lengths (Anna Henningsen)
    #7562
* **crypto**:
  * update root certificates (Ben Noordhuis)
    #7363
* **libuv**:
  * upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé)
    #6796
  * upgrade libuv to 1.9.0 (Saúl Ibarra Corretgé)
    #5994
* **npm**:
  * upgrade to 2.15.9 (Kat Marchán)
    #7692
macedigital added a commit to macedigital/gulp-sri-hash that referenced this pull request Mar 13, 2017
@macedigital
fix: Buffer.from(<string>) requires nodejs 4.5+ (#2)
32604ba 
Add version detection and use deprecated `new Buffer(<string>)`, if the
new Buffer API isn't supported by runtime.

See nodejs/node#7562.
@cxam cxam mentioned this pull request Aug 2, 2017
Merged
@SheetJSDev SheetJSDev mentioned this pull request Jun 21, 2018
Closed
@phureewat29 phureewat29 mentioned this pull request Oct 25, 2018
Merged
@lukastribus lukastribus mentioned this pull request May 13, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
buffer Issues and PRs related to the buffer subsystem. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ChALkeR @rvagg @jasnell @MylesBorins @mscdex @addaleax