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

quic: fix up coverity warning in quic/session.cc #49865

Closed
wants to merge 1 commit into from

Conversation

mhdawson
Copy link
Member

  • add CHECK around SocketAddress::New like we have in other places as suggested by Coverity scan

- add CHECK around SocketAddress::New like we have in other
  places as suggested by Coverity scan

Signed-off-by: Michael Dawson <midawson@redhat.com>
@nodejs-github-bot nodejs-github-bot 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. labels Sep 25, 2023
@mhdawson
Copy link
Member Author

Coverity report:

02    case AF_INET: {
1303      auto ipv4 = preferredAddress->ipv4();
1304      if (ipv4.has_value()) {
1305        if (ipv4->address.empty() || ipv4->port == 0) return;
      	CID 318341 (#1 of 2): Unchecked return value (CHECKED_RETURN) [[select issue](https://scan9.scan.coverity.com/defectInstanceId=9078846&fileInstanceId=114681342&mergedDefectId=318341)]
1306        SocketAddress::New(AF_INET,
1307                           std::string(ipv4->address).c_str(),
1308                           ipv4->port,
1309                           &remote_address_);
1310        preferredAddress->Use(ipv4.value());
1311      }
1312      break;
1313    }
1314    case AF_INET6: {
1315      auto ipv6 = preferredAddress->ipv6();
    	3. Condition ipv6.has_value(), taking true branch.
1316      if (ipv6.has_value()) {
    	4. Condition ipv6->address.empty(), taking false branch.
    	5. Condition ipv6->port == 0, taking false branch.
1317        if (ipv6->address.empty() || ipv6->port == 0) return;
    	
CID 318341 (#2 of 2): Unchecked return value (CHECKED_RETURN)
6. check_return: Calling New without checking return value (as is done elsewhere 9 out of 11 times).
1318        SocketAddress::New(AF_INET,
1319                           std::string(ipv6->address).c_str(),
1320                           ipv6->port,
1321                           &remote_address_);
1322        preferredAddress->Use(ipv6.value());
1323      }
1324      br

@mhdawson mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 28, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 28, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

mhdawson added a commit that referenced this pull request Oct 12, 2023
- add CHECK around SocketAddress::New like we have in other
  places as suggested by Coverity scan

Signed-off-by: Michael Dawson <midawson@redhat.com>

PR-URL: #49865
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@mhdawson
Copy link
Member Author

Landed in 971af4b

@mhdawson mhdawson closed this Oct 12, 2023
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
- add CHECK around SocketAddress::New like we have in other
  places as suggested by Coverity scan

Signed-off-by: Michael Dawson <midawson@redhat.com>

PR-URL: nodejs#49865
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@targos targos added 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 Nov 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants