GeezerVison

Copy link

Copy Markdown

Member

bricss and addaleax reacted with thumbs up emoji

tniessen force-pushed the src-memory-leaks-and-refactor-bytesource branch 5 times, most recently from 45a1378 to 2b6e52e Compare May 25, 2022 22:00

addaleax approved these changes Jun 10, 2022

View reviewed changes

RaisinTen approved these changes Jun 10, 2022

View reviewed changes

- Loading data for nodejs/node/pull/43202
✔  Done loading data for nodejs/node/pull/43202
----------------------------------- PR info ------------------------------------
Title      src: fix memory leaks and refactor ByteSource (#43202)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     tniessen:src-memory-leaks-and-refactor-bytesource -> nodejs:master
Labels     crypto, c++, lib / src, author ready, needs-ci
Commits    1
 - src: fix memory leaks and refactor ByteSource
Committers 1
 - Tobias Nießen 
PR-URL: https://github.com/nodejs/node/pull/43202
Reviewed-By: Anna Henningsen 
Reviewed-By: Darshan Sen 
Reviewed-By: James M Snell 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/43202
Reviewed-By: Anna Henningsen 
Reviewed-By: Darshan Sen 
Reviewed-By: James M Snell 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Tue, 24 May 2022 23:01:55 GMT
   ✔  Approvals: 3
   ✔  - Anna Henningsen (@addaleax): https://github.com/nodejs/node/pull/43202#pullrequestreview-1002220635
   ✔  - Darshan Sen (@RaisinTen) (TSC): https://github.com/nodejs/node/pull/43202#pullrequestreview-1002898142
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/43202#pullrequestreview-1003512265
   ✖  Last GitHub CI failed
   ℹ  Last Full PR CI on 2022-06-12T05:35:56Z: https://ci.nodejs.org/job/node-test-pull-request/44453/
- Querying data for job/node-test-pull-request/44453/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

Add ByteSource::Builder to replace the common MallocOpenSSL() +
ByteSource::Allocated() pattern.

Remove ByteSource::reset() that is unused.

Remove ByteSource::Resize() to make ByteSource truly immutable (until
moved away). Instead, ByteSource::Builder::release() takes an optional
size argument that truncates the resulting ByteSource.

Fix occurrences of MallocOpenSSL() that do not always free the allocated
memory by using the new ByteSource::Builder class instead.

Remove ByteSource::get() and replace uses with ByteSource::data().

Remove ReallocOpenSSL() because it likely only saves us a few bytes
whenever we use it.

PR-URL: #43202
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Avoid manual calls to MallocOpenSSL in ArrayBufferOrViewContents and
use the new ByteSource::Builder instead.

Refs: nodejs#43202

Avoid manual calls to MallocOpenSSL in ArrayBufferOrViewContents and
use the new ByteSource::Builder instead.

Refs: #43202

PR-URL: #43477
Reviewed-By: Darshan Sen <[email protected]>

Avoid manual calls to MallocOpenSSL in ArrayBufferOrViewContents and
use the new ByteSource::Builder instead.

Refs: #43202

PR-URL: #43477
Reviewed-By: Darshan Sen <[email protected]>

ByteSource::Allocated accepts a void pointer now, so we do not need to
cast the argument to a char pointer.

Refs: nodejs#43202

ByteSource::Allocated accepts a void pointer now, so we do not need to
cast the argument to a char pointer.

Refs: #43202

PR-URL: #44001
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Feng Yu <[email protected]>

ByteSource::Allocated accepts a void pointer now, so we do not need to
cast the argument to a char pointer.

Refs: #43202

PR-URL: #44001
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Feng Yu <[email protected]>

ByteSource::Allocated accepts a void pointer now, so we do not need to
cast the argument to a char pointer.

Refs: #43202

PR-URL: #44001
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Feng Yu <[email protected]>