Skip to content

src: remove unnecessary function call #27143

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

Closed

Conversation

tniessen
Copy link
Member

@tniessen tniessen commented Apr 9, 2019

Personally, I prefer a single call to .release() over .get() followed by .release() without using the return value since the former is closer to "move semantics". As always, feel free to disagree :-)

Refs: #27092

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

@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 Apr 9, 2019
@addaleax
Copy link
Member

addaleax commented Apr 9, 2019

This does not have the same behaviour in the return false; case, though … Is this still correct, i.e. do we have a memory leak right now?

@tniessen
Copy link
Member Author

tniessen commented Apr 9, 2019

I assumed OpenSSL took ownership either way, but I checked and you are right, it doesn't, so this won't work.

@tniessen tniessen closed this Apr 9, 2019
# for free to join this conversation on GitHub. Already have an account? # 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.

4 participants