buffer: harden SlowBuffer creation by ZYSzys · Pull Request #26272 · nodejs/node (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed

ZYSzys wants to merge2 commits intonodejs:masterfromzys-contrib:buffer-harden-slowbuffer

Conversation8 Commits2 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

ZYSzys

Now SlowBuffer creation throw misleading Error(which should be ERR_INVALID_ARG_TYPE IMHO) in our newest master since #26162 landed.

screen shot 2019-02-23 at 5 25 41 pm

So harden SlowBuffer creation again, make SlowBuffer function be consistent with the comment of Buffer.allocUnsafeSlow at the same time:

/**
* Equivalent to SlowBuffer(num), by default creates a non-zero-filled
* Buffer instance that is not allocated off the pre-initialized pool.
* If `--zero-fill-buffers` is set, will zero-fill the buffer.
*/
Buffer.allocUnsafeSlow = function allocUnsafeSlow(size) {
assertSize(size);
return createUnsafeBuffer(size);
};
Checklist

@addaleax addaleax added the semver-major

PRs that contain breaking changes and should be released in the next major version.

label

Feb 23, 2019

bnoordhuis

jasnell

Fishrock123

BridgeAR

@BridgeAR

mcollina

Choose a reason for hiding this comment

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

LGTM

BridgeAR

@ZYSzys

@ZYSzys

BridgeAR

Choose a reason for hiding this comment

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

Still LG

@BridgeAR

@BridgeAR BridgeAR added the author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

label

Mar 10, 2019

@danbev

danbev pushed a commit that referenced this pull request

Mar 11, 2019

@ZYSzys @danbev

PR-URL: #26272 Reviewed-By: Ben Noordhuis info@bnoordhuis.nl Reviewed-By: James M Snell jasnell@gmail.com Reviewed-By: Jeremiah Senkpiel fishrock123@rocketmail.com Reviewed-By: Matteo Collina matteo.collina@gmail.com Reviewed-By: Ruben Bridgewater ruben@bridgewater.de

@ZYSzys ZYSzys deleted the buffer-harden-slowbuffer branch

March 11, 2019 08:55

BethGriggs added a commit that referenced this pull request

Apr 22, 2019

@BethGriggs

Notable changes:

PR-URL: #26930

BethGriggs added a commit that referenced this pull request

Apr 23, 2019

@BethGriggs

Notable changes:

PR-URL: #26930

BethGriggs added a commit that referenced this pull request

Apr 23, 2019

@BethGriggs

Notable changes:

PR-URL: #26930

This was referenced

Apr 23, 2019

This was referenced

Apr 23, 2019

Reviewers

@mcollina mcollina mcollina approved these changes

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@jasnell jasnell jasnell approved these changes

@Fishrock123 Fishrock123 Fishrock123 approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

Labels

author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

buffer

Issues and PRs related to the buffer subsystem.

semver-major

PRs that contain breaking changes and should be released in the next major version.