GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — buffer: backport --zero-fill-buffers command line option by jasnell · Pull Request #5744 · nodejs/node

Conversation

@jasnell

@jasnell jasnell commented Mar 16, 2016

Copy link
Copy Markdown
Member

Pull Request check-list

  • Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to [CONTRIBUTING.md][0]?
  • If this change fixes a bug (or a performance problem), is a regression
    test (or a benchmark) included?
  • Is a documentation update included (if this change modifies
    existing APIs, or introduces new ones)?

Affected core subsystem(s)

buffer

Description of change

This backports the --zero-fill-buffers command line flag introduced in master. When used, all Buffer and SlowBuffer instances will zero fill by default.

This does not backport any of the other Buffer API or behavior changes.

Note: My intent is to open similar backports for --zero-fill-buffer in v4, v0.12 and v0.10

/cc @trevnorris

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@jasnell jasnell 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. v5.x labels Mar 16, 2016
@jasnell

jasnell commented Mar 17, 2016

Copy link
Copy Markdown
Member Author

@nodejs/ctc
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fishrock123

Fishrock123 commented Mar 17, 2016

Copy link
Copy Markdown
Contributor

Uh, why not land the actual patches with the new APIs?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell

jasnell commented Mar 17, 2016

Copy link
Copy Markdown
Member Author

Because it's technically a semver-major even tho it's a soft deprecation and there's been no decision about pulling all of the APIs back. I'm certainly open to the discussion but we'd need to decide how far back to backport
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ChALkeR

ChALkeR commented Mar 17, 2016

Copy link
Copy Markdown
Member

We can land those without the soft deprecation, but it should be discussed in a separate issue, probably.

+1 to these PRs that implement the --zero-fill-buffers flag (for all versions). I did not yet review the code, will do that later today.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell

jasnell commented Mar 17, 2016

Copy link
Copy Markdown
Member Author

I'm +1 with backporting the new APIs to v5 without the soft deprecation.
The @nodejs/lts WG and @nodejs/ctc would need to discuss and decide if it makes sense to backport the new APIs to v4/v0.12/v0.10.

At the very least, however, let's get the --zero-fill-buffers flag landed in each.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fishrock123

Fishrock123 commented Mar 17, 2016

Copy link
Copy Markdown
Contributor

Everything except the deprecation should (i.e. needs to) land in v5.x -- the deprecation should have been in a separate commit.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell

jasnell commented Mar 17, 2016

Copy link
Copy Markdown
Member Author

That would be easy enough to do. I'll get the v5 PR updated this morning.
Any opinions about backporting the APIs to v4?
On Mar 17, 2016 7:10 AM, "Jeremiah Senkpiel" [email protected]
wrote:

Everything except the deprecation should (i.e. needs to) land in v5.x --
the deprecation should have been in a separate commit.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#5744 (comment)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ChALkeR

ChALkeR commented Mar 17, 2016

Copy link
Copy Markdown
Member

@jasnell it would be good to include the new API in the next v4 minor, imo.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell jasnell added the lts-agenda label Mar 17, 2016
@jasnell

jasnell commented Mar 17, 2016

Copy link
Copy Markdown
Member Author

Adding lts-agenda so the @nodejs/lts WG can discuss backporting the APIs to v4
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell jasnell mentioned this pull request Mar 17, 2016
buffer: backport new buffer constructor APIs #5763
Closed
4 tasks
@ChALkeR

ChALkeR commented Mar 19, 2016

Copy link
Copy Markdown
Member

LGTM
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
1 similar comment
@trevnorris

trevnorris commented Mar 19, 2016

Copy link
Copy Markdown
Contributor

LGTM
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell jasnell mentioned this pull request Mar 21, 2016
LTS Meeting - 2016-03-21 nodejs/Release#89
Closed
@jasnell

jasnell commented Mar 21, 2016

Copy link
Copy Markdown
Member Author

CI: https://ci.nodejs.org/job/node-test-pull-request/2001/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell jasnell removed the lts-agenda label Mar 23, 2016
@jasnell

jasnell commented Mar 23, 2016

Copy link
Copy Markdown
Member Author

Discussion by the CTC today was to get this landed in v5 but hold off backporting further.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell
buffer: backport --zero-fill-buffers command line option
c64d0b0 
This backports the --zero-fill-buffers command line flag introduced
in master. When used, all Buffer and SlowBuffer instances will zero
fill by default.

This does *not* backport any of the other Buffer API or behavior
changes.
@jasnell
jasnell force-pushed the v5.x-zero-fill-buffers branch from b30fefd to c64d0b0 Compare March 23, 2016 21:46
@jasnell

jasnell commented Mar 23, 2016

Copy link
Copy Markdown
Member Author

@Fishrock123 ... updated this to fix up doc/node.1 and doc/api/cli.markdown. PTAL
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell

jasnell commented Mar 23, 2016

Copy link
Copy Markdown
Member Author

New CI: https://ci.nodejs.org/job/node-test-pull-request/2047/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell

jasnell commented Mar 23, 2016

Copy link
Copy Markdown
Member Author

CI is green except for one unrelated failure.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
jasnell added a commit that referenced this pull request Mar 24, 2016
@jasnell
buffer: backport --zero-fill-buffers command line option
3c02727 
This backports the --zero-fill-buffers command line flag introduced
in master. When used, all Buffer and SlowBuffer instances will zero
fill by default.

This does *not* backport any of the other Buffer API or behavior
changes.

PR-URL: #5744
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
@jasnell

jasnell commented Mar 24, 2016

Copy link
Copy Markdown
Member Author

Landed in v5.x in 3c02727
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell jasnell closed this Mar 24, 2016
@evanlucas evanlucas mentioned this pull request Mar 31, 2016
v5.10.0 Release Proposal #5970
Closed
evanlucas added a commit that referenced this pull request Mar 31, 2016
@evanlucas
2016-03-31 Version 5.10.0 (Stable) Release
9cd6e98 
Notable changes:

* buffer:
  * make byteLength work with ArrayBuffer & DataView (Jackson Tian)
[#5255](#5255)
  * backport --zero-fill-buffers command line option (James M Snell)
[#5744](#5744)
  * backport new buffer constructor APIs (James M Snell)
[#5763](#5763)
  * add swap16() and swap32() methods (James M Snell)
[#5724](#5724)
* fs: add the fs.mkdtemp() function. (Florian MARGAINE)
[#5333](#5333)
* net: emit host in lookup event (HUANG Wei)
[#5598](#5598)
* node: --no-browser-globals configure flag (Fedor Indutny)
[#5853](#5853)
* npm: Upgrade to v3.8.3. (Forrest L Norvell)
* repl: support standalone blocks (Prince J Wesley)
[#5581](#5581)
* src: override v8 thread defaults using cli options (Tom Gallacher)
[#4344](#4344)
evanlucas added a commit that referenced this pull request Mar 31, 2016
@evanlucas
2016-03-31 Version 5.10.0 (Stable) Release
1bb2c4b 
Notable changes:

* buffer:
  * make byteLength work with ArrayBuffer & DataView (Jackson Tian)
[#5255](#5255)
  * backport --zero-fill-buffers command line option (James M Snell)
[#5744](#5744)
  * backport new buffer constructor APIs (James M Snell)
[#5763](#5763)
  * add swap16() and swap32() methods (James M Snell)
[#5724](#5724)
* fs: add the fs.mkdtemp() function. (Florian MARGAINE)
[#5333](#5333)
* net: emit host in lookup event (HUANG Wei)
[#5598](#5598)
* node: --no-browser-globals configure flag (Fedor Indutny)
[#5853](#5853)
* npm: Upgrade to v3.8.3. Fixes a security flaw in the use of
authentication tokens in HTTP requests that would allow an attacker to
set up a server that could collect tokens from users of the command-line
interface. Authentication tokens have previously been sent with every
request made by the CLI for logged-in users, regardless of the destination
of the request. This update fixes this by only including those tokens
for requests made against the registry or registries used for the
current install. (Forrest L Norvell)
[npm#6](npm#6)
* repl: support standalone blocks (Prince J Wesley)
[#5581](#5581)
* src: override v8 thread defaults using cli options (Tom Gallacher)
[#4344](#4344)
evanlucas added a commit that referenced this pull request Apr 1, 2016
@evanlucas
2016-03-31 Version 5.10.0 (Stable) Release
79ea8c3 
Notable changes:

* buffer:
  * make byteLength work with ArrayBuffer & DataView (Jackson Tian)
[#5255](#5255)
  * backport --zero-fill-buffers command line option (James M Snell)
[#5744](#5744)
  * backport new buffer constructor APIs (James M Snell)
[#5763](#5763)
  * add swap16() and swap32() methods (James M Snell)
[#5724](#5724)
* fs: add the fs.mkdtemp() function. (Florian MARGAINE)
[#5333](#5333)
* net: emit host in lookup event (HUANG Wei)
[#5598](#5598)
* node: --no-browser-globals configure flag (Fedor Indutny)
[#5853](#5853)
* npm: Upgrade to v3.8.3. Fixes a security flaw in the use of
authentication tokens in HTTP requests that would allow an attacker to
set up a server that could collect tokens from users of the command-line
interface. Authentication tokens have previously been sent with every
request made by the CLI for logged-in users, regardless of the destination
of the request. This update fixes this by only including those tokens
for requests made against the registry or registries used for the
current install. (Forrest L Norvell)
[npm#6](npm#6)
* repl: support standalone blocks (Prince J Wesley)
[#5581](#5581)
* src: override v8 thread defaults using cli options (Tom Gallacher)
[#4344](#4344)
evanlucas added a commit that referenced this pull request Apr 1, 2016
@evanlucas
2016-03-31 Version 5.10.0 (Stable) Release
64c87e2 
Notable changes:

* buffer:
  * make byteLength work with ArrayBuffer & DataView (Jackson Tian)
[#5255](#5255)
  * backport --zero-fill-buffers command line option (James M Snell)
[#5744](#5744)
  * backport new buffer constructor APIs (James M Snell)
[#5763](#5763)
  * add swap16() and swap32() methods (James M Snell)
[#5724](#5724)
* fs: add the fs.mkdtemp() function. (Florian MARGAINE)
[#5333](#5333)
* net: emit host in lookup event (HUANG Wei)
[#5598](#5598)
* node: --no-browser-globals configure flag (Fedor Indutny)
[#5853](#5853)
* npm: Upgrade to v3.8.3. Fixes a security flaw in the use of
authentication tokens in HTTP requests that would allow an attacker to
set up a server that could collect tokens from users of the command-line
interface. Authentication tokens have previously been sent with every
request made by the CLI for logged-in users, regardless of the destination
of the request. This update fixes this by only including those tokens
for requests made against the registry or registries used for the
current install. (Forrest L Norvell)
[npm#6](npm#6)
* repl: support standalone blocks (Prince J Wesley)
[#5581](#5581)
* src: override v8 thread defaults using cli options (Tom Gallacher)
[#4344](#4344)

PR-URL: #5970
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

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.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@jasnell @Fishrock123 @ChALkeR @trevnorris
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.