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

Remove redundant ifdef USE_OPENSSL #3269

Closed
wants to merge 2 commits into from
Closed

Conversation

pkubaj
Copy link
Contributor

@pkubaj pkubaj commented Nov 14, 2018

There's code that reads as follows:
#ifdef USE_OPENSSL
#ifdef USE_OPENSSL
#else
Remove the redundant USE_OPENSSL along with #else (it's not possible to reach it anyway).

There's code that reads as follows:
#ifdef USE_OPENSSL
#ifdef USE_OPENSSL
#else
Remove the redundant USE_OPENSSL along with #else (it's not possible to reach it anyway).
@bagder bagder added the TLS label Nov 14, 2018
@@ -52,8 +52,6 @@

#if !defined(USE_WINDOWS_SSPI) || defined(USE_WIN32_CRYPTO)

#ifdef USE_OPENSSL
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer if the inner USE_OPENSSL define was removed instead since that would keep the indentation. We can then re-indent lines 57-63 one step to the left aligning them with the other SSL backends.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We would also need to re-indent lines from 64 to 75. Is that ok?

Copy link
Member

Choose a reason for hiding this comment

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

No those can stay where they are, as they are correctly indented two spaces in underneath the topmost #ifdef USE_OPENSSL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is that ok?

# include <openssl/md5.h>
# include <openssl/ssl.h>
# include <openssl/rand.h>
# endif
Copy link
Member

Choose a reason for hiding this comment

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

This last #endif also needs to be removed.

@danielgustafsson
Copy link
Member

This is the version of the patch that I proposed: https://gist.github.com/danielgustafsson/ec1ed3552642d7e4657e656feb31dd29
Unless you, or any other reviewer, objects I will push that.

@danielgustafsson
Copy link
Member

The fixed up commit referenced above pushed to master, thanks for your contribution!

@lock lock bot locked as resolved and limited conversation to collaborators Feb 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

None yet

3 participants