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

src: in-source comments and minor TLS cleanups #25713

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@sam-github
Copy link
Member

sam-github commented Jan 25, 2019

Renamed some internal C++ methods and properties for consistency, and
commented SSL I/O.

  • Rename waiting_new_session_ after is_waiting_new_session(), instead of
    using reverse naming (new_session_wait_).
  • Make TLSWrap::ClearIn() return void, the value is never used.
  • Fix a getTicketKeys() cut-n-paste error. Since it doesn't use the
    arguments, remove them from the js wrapper.

This is semver-patch, none of the changes are visible, not even at the binding layer.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
@addaleax
Copy link
Member

addaleax left a comment

Thank you!

@sam-github sam-github force-pushed the sam-github:tls-cleanups branch from 0c4bb44 to 1a50638 Jan 25, 2019

@sam-github

This comment has been minimized.

Copy link
Member Author

sam-github commented Jan 25, 2019

Fixed the spelling ofNewSessionDoneDb ("Cb") in the commit message, repushed.

@mhdawson
Copy link
Member

mhdawson left a comment

LGTM

@sam-github sam-github force-pushed the sam-github:tls-cleanups branch from 1a50638 to 99e558d Jan 25, 2019

@cjihrig cjihrig referenced this pull request Jan 25, 2019

Merged

tls: support changing credentials dynamically #23644

4 of 4 tasks complete
Show resolved Hide resolved src/node_crypto.cc Outdated
Show resolved Hide resolved src/tls_wrap.cc Outdated
Show resolved Hide resolved src/tls_wrap.h Outdated
src: in-source comments and minor TLS cleanups
Renamed some internal C++ methods and properties for consistency, and
commented SSL I/O.

- Rename waiting_new_session_ after is_waiting_new_session(), instead of
  using reverse naming (new_session_wait_), and change "waiting" to
  "awaiting".
- Make TLSWrap::ClearIn() return void, the value is never used.
- Fix a getTicketKeys() cut-n-paste error. Since it doesn't use the
  arguments, remove them from the js wrapper.
- Remove call of setTicketKeys(getTicketKeys()), its a no-op.

@sam-github sam-github force-pushed the sam-github:tls-cleanups branch from cdf9d71 to cbdcaea Jan 28, 2019

@addaleax

This comment has been minimized.

Copy link
Member

addaleax commented Jan 28, 2019

@addaleax addaleax closed this Jan 28, 2019

pull bot pushed a commit to SimenB/node that referenced this pull request Jan 28, 2019

src: in-source comments and minor TLS cleanups
Renamed some internal C++ methods and properties for consistency, and
commented SSL I/O.

- Rename waiting_new_session_ after is_waiting_new_session(), instead of
  using reverse naming (new_session_wait_), and change "waiting" to
  "awaiting".
- Make TLSWrap::ClearIn() return void, the value is never used.
- Fix a getTicketKeys() cut-n-paste error. Since it doesn't use the
  arguments, remove them from the js wrapper.
- Remove call of setTicketKeys(getTicketKeys()), its a no-op.

PR-URL: nodejs#25713
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

addaleax added a commit that referenced this pull request Jan 28, 2019

src: in-source comments and minor TLS cleanups
Renamed some internal C++ methods and properties for consistency, and
commented SSL I/O.

- Rename waiting_new_session_ after is_waiting_new_session(), instead of
  using reverse naming (new_session_wait_), and change "waiting" to
  "awaiting".
- Make TLSWrap::ClearIn() return void, the value is never used.
- Fix a getTicketKeys() cut-n-paste error. Since it doesn't use the
  arguments, remove them from the js wrapper.
- Remove call of setTicketKeys(getTicketKeys()), its a no-op.

PR-URL: #25713
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

@sam-github sam-github deleted the sam-github:tls-cleanups branch Jan 29, 2019

@sam-github

This comment has been minimized.

Copy link
Member Author

sam-github commented Jan 29, 2019

Thanks for landing @addaleax

@targos targos referenced this pull request Jan 29, 2019

Merged

v11.9.0 proposal #25802

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment