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

crypto: return CHECK_OK in VerifyCallback #13241

Closed
wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented May 26, 2017

VerifyCallback returns 1 in two locations but CHECK_CERT_REVOKED in a
third return statment. This commit suggests that CHECK_OK is used
instead of 1. CHECK_OK is also used as the return value in
CheckWhitelistedServerCert so it seems to be consitent change to make.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

crypto

VerifyCallback returns 1 in two locations but CHECK_CERT_REVOKED in a
third return statment. This commit suggests that CHECK_OK is used
instead of 1. CHECK_OK is also used as the return value in
CheckWhitelistedServerCert so it seems to be consitent change to make.
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. labels May 26, 2017
@danbev
Copy link
Contributor Author

danbev commented May 26, 2017

@refack
Copy link
Contributor

refack commented May 27, 2017

OSX fail is a new flaky: #13252

danbev added a commit to danbev/node that referenced this pull request May 29, 2017
VerifyCallback returns 1 in two locations but CHECK_CERT_REVOKED in a
third return statment. This commit suggests that CHECK_OK is used
instead of 1. CHECK_OK is also used as the return value in
CheckWhitelistedServerCert so it seems to be consitent change to make.

PR-URL: nodejs#13241
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@danbev
Copy link
Contributor Author

danbev commented May 29, 2017

Landed in b3fa3fc

@danbev danbev closed this May 29, 2017
@danbev danbev deleted the crypto_use_CHECK_OK branch May 29, 2017 03:32
jasnell pushed a commit that referenced this pull request May 29, 2017
VerifyCallback returns 1 in two locations but CHECK_CERT_REVOKED in a
third return statment. This commit suggests that CHECK_OK is used
instead of 1. CHECK_OK is also used as the return value in
CheckWhitelistedServerCert so it seems to be consitent change to make.

PR-URL: #13241
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
3 tasks
MylesBorins pushed a commit that referenced this pull request Jul 17, 2017
VerifyCallback returns 1 in two locations but CHECK_CERT_REVOKED in a
third return statment. This commit suggests that CHECK_OK is used
instead of 1. CHECK_OK is also used as the return value in
CheckWhitelistedServerCert so it seems to be consitent change to make.

PR-URL: #13241
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jul 18, 2017
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++. crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants