-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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: declare int return type for set_field #17468
Conversation
The |
Ah I see that now, thanks for clarifying! Is there a reason for not declaring the return type for the function pointer? I've added a commit with what I mean. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a suggestion.
src/node_crypto.cc
Outdated
@@ -5107,12 +5107,13 @@ void DiffieHellman::SetKey(const v8::FunctionCallbackInfo<v8::Value>& args, | |||
BN_bin2bn(reinterpret_cast<unsigned char*>(Buffer::Data(args[0])), | |||
Buffer::Length(args[0]), nullptr); | |||
CHECK_NE(num, nullptr); | |||
set_field(dh->dh, num); | |||
USE(set_field(dh->dh, num)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you're threading through the return value, you might as well CHECK_EQ(1, set_field(dh->dh, num));
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, I'll add the check. Thanks
This commit updates the set_field function pointer to return an int, and also updates the lambdas with a return statement.
acbe9e6
to
0a1bfc1
Compare
Landed in abc2801 |
This commit updates the set_field function pointer to return an int, and also updates the lambdas with a return statement. PR-URL: #17468 Reviewed-By: Ben Noordhuis <[email protected]>
This commit updates the set_field function pointer to return an int, and also updates the lambdas with a return statement. PR-URL: #17468 Reviewed-By: Ben Noordhuis <[email protected]>
This commit updates the set_field function pointer to return an int, and also updates the lambdas with a return statement. PR-URL: #17468 Reviewed-By: Ben Noordhuis <[email protected]>
Should this be backported to |
This commit updates the set_field function pointer to return an int, and
also updates the lambdas with a return statement.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
src