Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

quic: prevent copying ngtcp2_cid #48561

Merged
merged 1 commit into from
Jun 28, 2023

Conversation

tniessen
Copy link
Member

The existing code explicitly uses the CID(const ngtcp2_cid*) constructor that does not copy the given ngtcp2_cid, but the range-based for loop still unnecessarily copies the ngtcp2_cid. Use auto& to avoid copying.

The existing code explicitly uses the CID(const ngtcp2_cid*) constructor
that does not copy the given ngtcp2_cid, but the range-based for loop
still unnecessarily copies the ngtcp2_cid. Use auto& to avoid copying.
@tniessen tniessen added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. quic Issues and PRs related to the QUIC implementation / HTTP/3. request-ci Add this label to start a Jenkins CI on a PR. labels Jun 26, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 26, 2023
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

@tniessen tniessen added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Jun 28, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 28, 2023
@nodejs-github-bot nodejs-github-bot merged commit 546797f into nodejs:main Jun 28, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 546797f

@RafaelGSS
Copy link
Member

This PR depends on #47927 which didn't land on v20.x.

@RafaelGSS RafaelGSS added dont-land-on-v16.x dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. labels Jul 3, 2023
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
The existing code explicitly uses the CID(const ngtcp2_cid*) constructor
that does not copy the given ngtcp2_cid, but the range-based for loop
still unnecessarily copies the ngtcp2_cid. Use auto& to avoid copying.

PR-URL: nodejs#48561
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
The existing code explicitly uses the CID(const ngtcp2_cid*) constructor
that does not copy the given ngtcp2_cid, but the range-based for loop
still unnecessarily copies the ngtcp2_cid. Use auto& to avoid copying.

PR-URL: nodejs#48561
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
author ready PRs 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++. dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. needs-ci PRs that need a full CI run. quic Issues and PRs related to the QUIC implementation / HTTP/3.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants