Skip to content

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Dec 8, 2017

I've not been able to find any reason for calling
BIO_set_shutdown(bio, 1). This is done by default for the following
versions of OpenSSL:

https://github.com/openssl/openssl/blob/OpenSSL_1_1_0/crypto/bio/bio_lib.c#L26
https://github.com/openssl/openssl/blob/OpenSSL_1_0_1/crypto/bio/bio_lib.c#L90
https://github.com/openssl/openssl/blob/OpenSSL_1_0_2/crypto/bio/bio_lib.c#L88
https://github.com/openssl/openssl/blob/OpenSSL_1_0_0/crypto/bio/bio_lib.c#L90

This commit removes the call and the comment.

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

crypto

I've not been able to find any reason for calling
BIO_set_shutdown(bio, 1). This is done by default for the following
versions of OpenSSL:

https://github.com/openssl/openssl/blob/OpenSSL_1_1_0/
crypto/bio/bio_lib.c#L26

https://github.com/openssl/openssl/blob/OpenSSL_1_0_1/
crypto/bio/bio_lib.c#L90

https://github.com/openssl/openssl/blob/OpenSSL_1_0_2/
crypto/bio/bio_lib.c#L88

https://github.com/openssl/openssl/blob/OpenSSL_1_0_0/
crypto/bio/bio_lib.c#L90

This commit removes the call and the comment.
@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 Dec 8, 2017
@danbev
Copy link
Contributor Author

danbev commented Dec 8, 2017

Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

If you want the background: it's a signal to openssl's built-in bios to free or close the underlying resource (file, socket, memory, etc) when freeing the bio.

@danbev
Copy link
Contributor Author

danbev commented Dec 11, 2017

If you want the background: it's a signal to openssl's built-in bios to free or close the underlying resource (file, socket, memory, etc) when freeing the bio.

Thanks!

@danbev
Copy link
Contributor Author

danbev commented Dec 11, 2017

Landed in b36d4e7

@danbev danbev closed this Dec 11, 2017
danbev added a commit to danbev/node that referenced this pull request Dec 11, 2017
I've not been able to find any reason for calling
BIO_set_shutdown(bio, 1). This is done by default for the following
versions of OpenSSL:

https://github.com/openssl/openssl/blob/OpenSSL_1_1_0/
crypto/bio/bio_lib.c#L26

https://github.com/openssl/openssl/blob/OpenSSL_1_0_1/
crypto/bio/bio_lib.c#L90

https://github.com/openssl/openssl/blob/OpenSSL_1_0_2/
crypto/bio/bio_lib.c#L88

https://github.com/openssl/openssl/blob/OpenSSL_1_0_0/
crypto/bio/bio_lib.c#L90

This commit removes the call and the comment.

PR-URL: nodejs#17542
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@danbev danbev deleted the crypto-remove-shutdown branch December 11, 2017 06:30
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
I've not been able to find any reason for calling
BIO_set_shutdown(bio, 1). This is done by default for the following
versions of OpenSSL:

https://github.com/openssl/openssl/blob/OpenSSL_1_1_0/
crypto/bio/bio_lib.c#L26

https://github.com/openssl/openssl/blob/OpenSSL_1_0_1/
crypto/bio/bio_lib.c#L90

https://github.com/openssl/openssl/blob/OpenSSL_1_0_2/
crypto/bio/bio_lib.c#L88

https://github.com/openssl/openssl/blob/OpenSSL_1_0_0/
crypto/bio/bio_lib.c#L90

This commit removes the call and the comment.

PR-URL: #17542
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
I've not been able to find any reason for calling
BIO_set_shutdown(bio, 1). This is done by default for the following
versions of OpenSSL:

https://github.com/openssl/openssl/blob/OpenSSL_1_1_0/
crypto/bio/bio_lib.c#L26

https://github.com/openssl/openssl/blob/OpenSSL_1_0_1/
crypto/bio/bio_lib.c#L90

https://github.com/openssl/openssl/blob/OpenSSL_1_0_2/
crypto/bio/bio_lib.c#L88

https://github.com/openssl/openssl/blob/OpenSSL_1_0_0/
crypto/bio/bio_lib.c#L90

This commit removes the call and the comment.

PR-URL: #17542
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Dec 12, 2017
@gibfahn
Copy link
Member

gibfahn commented Dec 20, 2017

Should this be backported to v6.x or v8.x-staging? If yes please follow the guide and raise a backport PR, if no let me know or add the dont-land-on label.

danbev added a commit to danbev/node that referenced this pull request Dec 20, 2017
I've not been able to find any reason for calling
BIO_set_shutdown(bio, 1). This is done by default for the following
versions of OpenSSL:

https://github.com/openssl/openssl/blob/OpenSSL_1_1_0/
crypto/bio/bio_lib.c#L26

https://github.com/openssl/openssl/blob/OpenSSL_1_0_1/
crypto/bio/bio_lib.c#L90

https://github.com/openssl/openssl/blob/OpenSSL_1_0_2/
crypto/bio/bio_lib.c#L88

https://github.com/openssl/openssl/blob/OpenSSL_1_0_0/
crypto/bio/bio_lib.c#L90

This commit removes the call and the comment.

PR-URL: nodejs#17542
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 17, 2018
I've not been able to find any reason for calling
BIO_set_shutdown(bio, 1). This is done by default for the following
versions of OpenSSL:

https://github.com/openssl/openssl/blob/OpenSSL_1_1_0/
crypto/bio/bio_lib.c#L26

https://github.com/openssl/openssl/blob/OpenSSL_1_0_1/
crypto/bio/bio_lib.c#L90

https://github.com/openssl/openssl/blob/OpenSSL_1_0_2/
crypto/bio/bio_lib.c#L88

https://github.com/openssl/openssl/blob/OpenSSL_1_0_0/
crypto/bio/bio_lib.c#L90

This commit removes the call and the comment.

Backport-PR-URL: #17784
PR-URL: #17542
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 17, 2018
I've not been able to find any reason for calling
BIO_set_shutdown(bio, 1). This is done by default for the following
versions of OpenSSL:

https://github.com/openssl/openssl/blob/OpenSSL_1_1_0/
crypto/bio/bio_lib.c#L26

https://github.com/openssl/openssl/blob/OpenSSL_1_0_1/
crypto/bio/bio_lib.c#L90

https://github.com/openssl/openssl/blob/OpenSSL_1_0_2/
crypto/bio/bio_lib.c#L88

https://github.com/openssl/openssl/blob/OpenSSL_1_0_0/
crypto/bio/bio_lib.c#L90

This commit removes the call and the comment.

Backport-PR-URL: #17784
PR-URL: #17542
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2018
MylesBorins pushed a commit that referenced this pull request Feb 11, 2018
I've not been able to find any reason for calling
BIO_set_shutdown(bio, 1). This is done by default for the following
versions of OpenSSL:

https://github.com/openssl/openssl/blob/OpenSSL_1_1_0/
crypto/bio/bio_lib.c#L26

https://github.com/openssl/openssl/blob/OpenSSL_1_0_1/
crypto/bio/bio_lib.c#L90

https://github.com/openssl/openssl/blob/OpenSSL_1_0_2/
crypto/bio/bio_lib.c#L88

https://github.com/openssl/openssl/blob/OpenSSL_1_0_0/
crypto/bio/bio_lib.c#L90

This commit removes the call and the comment.

Backport-PR-URL: #17784
PR-URL: #17542
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 12, 2018
I've not been able to find any reason for calling
BIO_set_shutdown(bio, 1). This is done by default for the following
versions of OpenSSL:

https://github.com/openssl/openssl/blob/OpenSSL_1_1_0/
crypto/bio/bio_lib.c#L26

https://github.com/openssl/openssl/blob/OpenSSL_1_0_1/
crypto/bio/bio_lib.c#L90

https://github.com/openssl/openssl/blob/OpenSSL_1_0_2/
crypto/bio/bio_lib.c#L88

https://github.com/openssl/openssl/blob/OpenSSL_1_0_0/
crypto/bio/bio_lib.c#L90

This commit removes the call and the comment.

Backport-PR-URL: #17784
PR-URL: #17542
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 13, 2018
I've not been able to find any reason for calling
BIO_set_shutdown(bio, 1). This is done by default for the following
versions of OpenSSL:

https://github.com/openssl/openssl/blob/OpenSSL_1_1_0/
crypto/bio/bio_lib.c#L26

https://github.com/openssl/openssl/blob/OpenSSL_1_0_1/
crypto/bio/bio_lib.c#L90

https://github.com/openssl/openssl/blob/OpenSSL_1_0_2/
crypto/bio/bio_lib.c#L88

https://github.com/openssl/openssl/blob/OpenSSL_1_0_0/
crypto/bio/bio_lib.c#L90

This commit removes the call and the comment.

Backport-PR-URL: #17784
PR-URL: #17542
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 22, 2018
I've not been able to find any reason for calling
BIO_set_shutdown(bio, 1). This is done by default for the following
versions of OpenSSL:

https://github.com/openssl/openssl/blob/OpenSSL_1_1_0/
crypto/bio/bio_lib.c#L26

https://github.com/openssl/openssl/blob/OpenSSL_1_0_1/
crypto/bio/bio_lib.c#L90

https://github.com/openssl/openssl/blob/OpenSSL_1_0_2/
crypto/bio/bio_lib.c#L88

https://github.com/openssl/openssl/blob/OpenSSL_1_0_0/
crypto/bio/bio_lib.c#L90

This commit removes the call and the comment.

PR-URL: #17542
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jun 14, 2018
I've not been able to find any reason for calling
BIO_set_shutdown(bio, 1). This is done by default for the following
versions of OpenSSL:

https://github.com/openssl/openssl/blob/OpenSSL_1_1_0/
crypto/bio/bio_lib.c#L26

https://github.com/openssl/openssl/blob/OpenSSL_1_0_1/
crypto/bio/bio_lib.c#L90

https://github.com/openssl/openssl/blob/OpenSSL_1_0_2/
crypto/bio/bio_lib.c#L88

https://github.com/openssl/openssl/blob/OpenSSL_1_0_0/
crypto/bio/bio_lib.c#L90

This commit removes the call and the comment.

PR-URL: #17542
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jul 9, 2018
rvagg pushed a commit that referenced this pull request Aug 16, 2018
I've not been able to find any reason for calling
BIO_set_shutdown(bio, 1). This is done by default for the following
versions of OpenSSL:

https://github.com/openssl/openssl/blob/OpenSSL_1_1_0/
crypto/bio/bio_lib.c#L26

https://github.com/openssl/openssl/blob/OpenSSL_1_0_1/
crypto/bio/bio_lib.c#L90

https://github.com/openssl/openssl/blob/OpenSSL_1_0_2/
crypto/bio/bio_lib.c#L88

https://github.com/openssl/openssl/blob/OpenSSL_1_0_0/
crypto/bio/bio_lib.c#L90

This commit removes the call and the comment.

PR-URL: #17542
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
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