Skip to content

quic: do not dereference shared_ptr after move - #47294

Closed
tniessen wants to merge 1 commit into
nodejs:mainfrom
tniessen:src-quic-store-move
Closed

quic: do not dereference shared_ptr after move#47294
tniessen wants to merge 1 commit into
nodejs:mainfrom
tniessen:src-quic-store-move

Conversation

@tniessen

Copy link
Copy Markdown
Member

The stored pointer is assumed to be nullptr after std::move.

The stored pointer is assumed to be nullptr after std::move.
@tniessentniessen added c++ Issues and PRs that require attention from people who are familiar with C++. quic Issues and PRs related to the QUIC implementation / HTTP/3. labels Mar 29, 2023
@tniessen
tniessen requested a review from jasnellMarch 29, 2023 12:08
@nodejs-github-botnodejs-github-bot added dont-land-on-v14.x needs-ci PRs that need a full CI run. labels Mar 29, 2023
Comment threadsrc/quic/data.cc
CHECK_LE(offset_, store->ByteLength());
CHECK_LE(length_, store->ByteLength() - offset_);
CHECK_LE(offset_, store_->ByteLength());
CHECK_LE(length_, store_->ByteLength() - offset_);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

oh! ha, good catch.

@jasnelljasnell added the fast-track PRs that do not need to wait for 48 hours to land. label Mar 29, 2023
@github-actions

Copy link
Copy Markdown
Contributor

Fast-track has been requested by @jasnell. Please 👍 to approve.

@jasnell
jasnell requested review from Qard and anonrigMarch 29, 2023 13:27
@anonriganonrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Mar 29, 2023
@jasnelljasnell removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 29, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@jasnell

Copy link
Copy Markdown
Member

Landed in 9b104be

jasnell pushed a commit that referenced this pull request Mar 29, 2023
The stored pointer is assumed to be nullptr after std::move.
PR-URL: #47294
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@jasnelljasnell closed this Mar 29, 2023
@RafaelGSS

Copy link
Copy Markdown
Member

This PR depends on #47263 which was flagged as "dont-land-on-v19.x". Therefore, I'm adding the dont-land-x labels here too.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.c++Issues and PRs that require attention from people who are familiar with C++.fast-trackPRs that do not need to wait for 48 hours to land.needs-ciPRs that need a full CI run.quicIssues and PRs related to the QUIC implementation / HTTP/3.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@tniessen@nodejs-github-bot@jasnell@RafaelGSS@anonrig