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

src: remove usage of AllocatedBuffer from src/node_buffer.cc #39941

Conversation

RaisinTen
Copy link
Contributor

Since AllocatedBuffer is just a thin wrapper around v8::BackingStore, we
should prefer using v8::BackingStore directly.

Refs:

// TODO(addaleax): We may want to start phasing this out as it's only a
// thin wrapper around v8::BackingStore at this point

Signed-off-by: Darshan Sen darshan.sen@postman.com

@nodejs-github-bot nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Aug 30, 2021
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

src/node_buffer.cc Outdated Show resolved Hide resolved
Since AllocatedBuffer is just a thin wrapper around v8::BackingStore, we
should prefer using v8::BackingStore directly.

Refs: https://github.com/nodejs/node/blob/52abf271c563ddffdc93b444ea05e5347a7f2784/src/allocated_buffer.h#L30-L31

Signed-off-by: Darshan Sen <darshan.sen@postman.com>
@RaisinTen RaisinTen force-pushed the src/remove-usage-of-AllocatedBuffer-from-src/buffer.cc branch from 6ca23b2 to 2d93910 Compare August 30, 2021 09:04
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Aug 30, 2021

@RaisinTen RaisinTen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 30, 2021
@VoltrexKeyva VoltrexKeyva removed the needs-ci PRs that need a full CI run. label Aug 30, 2021
@jasnell
Copy link
Member

jasnell commented Sep 3, 2021

Landed in 7cc96b2

jasnell pushed a commit that referenced this pull request Sep 3, 2021
Since AllocatedBuffer is just a thin wrapper around v8::BackingStore, we
should prefer using v8::BackingStore directly.

Refs: https://github.com/nodejs/node/blob/52abf271c563ddffdc93b444ea05e5347a7f2784/src/allocated_buffer.h#L30-L31

Signed-off-by: Darshan Sen <darshan.sen@postman.com>

PR-URL: #39941
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell jasnell closed this Sep 3, 2021
@RaisinTen RaisinTen deleted the src/remove-usage-of-AllocatedBuffer-from-src/buffer.cc branch September 4, 2021 08:03
targos pushed a commit that referenced this pull request Sep 6, 2021
Since AllocatedBuffer is just a thin wrapper around v8::BackingStore, we
should prefer using v8::BackingStore directly.

Refs: https://github.com/nodejs/node/blob/52abf271c563ddffdc93b444ea05e5347a7f2784/src/allocated_buffer.h#L30-L31

Signed-off-by: Darshan Sen <darshan.sen@postman.com>

PR-URL: #39941
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Sep 6, 2021
Since AllocatedBuffer is just a thin wrapper around v8::BackingStore, we
should prefer using v8::BackingStore directly.

Refs: https://github.com/nodejs/node/blob/52abf271c563ddffdc93b444ea05e5347a7f2784/src/allocated_buffer.h#L30-L31

Signed-off-by: Darshan Sen <darshan.sen@postman.com>

PR-URL: #39941
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos mentioned this pull request Sep 6, 2021
RaisinTen added a commit to RaisinTen/node that referenced this pull request Mar 27, 2022
Refs: nodejs#39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Mar 31, 2022
Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
nodejs-github-bot pushed a commit that referenced this pull request Mar 31, 2022
ZeroPadDiffieHellmanSecret() is in an anonymous namespace, so it has
static linkage already.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
nodejs-github-bot pushed a commit that referenced this pull request Mar 31, 2022
Buffer::Length() dereferences the passed Local, so calling it when the
underlying pointer is a nullptr would lead to a crash. This fixes that
by returning early instead.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
RaisinTen added a commit to RaisinTen/node that referenced this pull request Apr 3, 2022
Refs: nodejs#39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>
juanarbol pushed a commit to juanarbol/node that referenced this pull request Apr 5, 2022
Refs: nodejs#39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs#42492
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
danielleadams pushed a commit that referenced this pull request Jun 27, 2022
ZeroPadDiffieHellmanSecret() is in an anonymous namespace, so it has
static linkage already.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
danielleadams pushed a commit that referenced this pull request Jun 27, 2022
Buffer::Length() dereferences the passed Local, so calling it when the
underlying pointer is a nullptr would lead to a crash. This fixes that
by returning early instead.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
danielleadams pushed a commit that referenced this pull request Jun 27, 2022
Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42589
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams pushed a commit that referenced this pull request Jun 27, 2022
Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42766
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams pushed a commit that referenced this pull request Jun 27, 2022
Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42852
Reviewed-By: Anna Henningsen <anna@addaleax.net>
danielleadams pushed a commit that referenced this pull request Jun 27, 2022
Since all its uses are now gone, it's time to say goodbye to
AllocatedBuffer.

Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #43008
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 11, 2022
Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 11, 2022
ZeroPadDiffieHellmanSecret() is in an anonymous namespace, so it has
static linkage already.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 11, 2022
Buffer::Length() dereferences the passed Local, so calling it when the
underlying pointer is a nullptr would lead to a crash. This fixes that
by returning early instead.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 11, 2022
Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 11, 2022
ZeroPadDiffieHellmanSecret() is in an anonymous namespace, so it has
static linkage already.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 11, 2022
Buffer::Length() dereferences the passed Local, so calling it when the
underlying pointer is a nullptr would lead to a crash. This fixes that
by returning early instead.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 11, 2022
Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42589
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jul 12, 2022
Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42766
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jul 12, 2022
Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42852
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 12, 2022
Since all its uses are now gone, it's time to say goodbye to
AllocatedBuffer.

Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #43008
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 31, 2022
Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 31, 2022
ZeroPadDiffieHellmanSecret() is in an anonymous namespace, so it has
static linkage already.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 31, 2022
Buffer::Length() dereferences the passed Local, so calling it when the
underlying pointer is a nullptr would lead to a crash. This fixes that
by returning early instead.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42492
Refs: #39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 31, 2022
Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42589
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jul 31, 2022
Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42766
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jul 31, 2022
Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #42852
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jul 31, 2022
Since all its uses are now gone, it's time to say goodbye to
AllocatedBuffer.

Refs: #39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #43008
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
Refs: nodejs/node#39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs/node#42492
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
ZeroPadDiffieHellmanSecret() is in an anonymous namespace, so it has
static linkage already.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs/node#42492
Refs: nodejs/node#39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
Buffer::Length() dereferences the passed Local, so calling it when the
underlying pointer is a nullptr would lead to a crash. This fixes that
by returning early instead.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs/node#42492
Refs: nodejs/node#39941
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
Refs: nodejs/node#39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs/node#42589
Reviewed-By: James M Snell <jasnell@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
Refs: nodejs/node#39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs/node#42766
Reviewed-By: James M Snell <jasnell@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
Refs: nodejs/node#39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs/node#42852
Reviewed-By: Anna Henningsen <anna@addaleax.net>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
Since all its uses are now gone, it's time to say goodbye to
AllocatedBuffer.

Refs: nodejs/node#39941
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs/node#43008
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants