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

ssl: Update outdated "openssl-only" comments for supported backends #3985

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
2 participants
@gevaerts
Copy link
Contributor

commented Jun 4, 2019

These are for features that used to be openssl-only but were expanded over
time.

void *ssl_ctx, /* actually an
OpenSSL SSL_CTX */
void *ssl_ctx, /* actually an OpenSSL
or WolfSSL SSL_CTX,

This comment has been minimized.

Copy link
@captain-caveman2k

captain-caveman2k Jun 4, 2019

Member

Is there space here within our 80 character limit to align the first character of each addition comment line with the start to actually?

This comment has been minimized.

Copy link
@gevaerts

gevaerts Jun 4, 2019

Author Contributor

For these lines, yes, but the "mbedtls_ssl_config */" line would be too long then. I'll have just that one be "wrong"

in second argument. The function must be matching the
curl_ssl_ctx_callback proto. */
/* Set the ssl context callback function, currently only for OpenSSL or
WolfSSL ssl_ctx or mbedTLS mbedtls_ssl_config in second argument.

This comment has been minimized.

Copy link
@captain-caveman2k

captain-caveman2k Jun 4, 2019

Member

I would insert the old "oxford comma" between ssl_ctx and or, and a the between in and second.

This comment has been minimized.

Copy link
@gevaerts

gevaerts Jun 4, 2019

Author Contributor

Yes, good point (or comma!)

curl_ssl_ctx_callback proto. */
/* Set the ssl context callback function, currently only for OpenSSL or
WolfSSL ssl_ctx or mbedTLS mbedtls_ssl_config in second argument.
The function must be matching the curl_ssl_ctx_callback proto. */

This comment has been minimized.

Copy link
@captain-caveman2k

captain-caveman2k Jun 4, 2019

Member

I think the sentence reads better if be matching is replaced with match and proto is expanded to prototype.

This comment has been minimized.

Copy link
@gevaerts

gevaerts Jun 4, 2019

Author Contributor

True, and worth changing while we're at it.

@captain-caveman2k captain-caveman2k changed the title Adjust some outdated "openssl-only" comments to add more backends. ssl: Update outdated "openssl-only" comments for supported backends Jun 4, 2019

@gevaerts gevaerts force-pushed the gevaerts:commentfix branch from 1814021 to fcadd44 Jun 4, 2019

ssl: Update outdated "openssl-only" comments for supported backends
These are for features that used to be openssl-only but were expanded over
time.

@gevaerts gevaerts force-pushed the gevaerts:commentfix branch from fcadd44 to d10fba6 Jun 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.