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
base: master
from

Conversation

Projects
None yet
3 participants
@pkubaj
Contributor

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).

Remove redundant ifdef USE_OPENSSL
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 SSL/TLS label Nov 14, 2018

@bagder

bagder approved these changes Nov 14, 2018

@@ -52,8 +52,6 @@
#if !defined(USE_WINDOWS_SSPI) || defined(USE_WIN32_CRYPTO)
#ifdef USE_OPENSSL

This comment has been minimized.

@danielgustafsson

danielgustafsson Nov 14, 2018

Member

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.

This comment has been minimized.

@pkubaj

pkubaj Nov 14, 2018

Contributor

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

This comment has been minimized.

@danielgustafsson

danielgustafsson Nov 14, 2018

Member

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

This comment has been minimized.

@pkubaj

pkubaj Nov 14, 2018

Contributor

Is that ok?

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

This comment has been minimized.

@danielgustafsson

danielgustafsson Nov 14, 2018

Member

This last #endif also needs to be removed.

@danielgustafsson

This comment has been minimized.

Member

danielgustafsson commented Nov 19, 2018

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

This comment has been minimized.

Member

danielgustafsson commented Nov 20, 2018

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

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