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

Define HAVE_SSL_GET_SHUTDOWN when available (Windows) #4100

Closed
wants to merge 6 commits into from

Conversation

@Zenju
Copy link
Contributor

commented Jul 9, 2019

Hi,

Just noticed, that the following code fragment in lib/vtls/openssl.c is disabled in the Windows build because HAVE_SSL_GET_SHUTDOWN is undefined, but shouldn't be:

#ifdef HAVE_SSL_GET_SHUTDOWN
            switch (SSL_get_shutdown(BACKEND->handle))
            {
                case SSL_SENT_SHUTDOWN:
                    infof(data, "SSL_get_shutdown() returned SSL_SENT_SHUTDOWN\n");
                    break;
                case SSL_RECEIVED_SHUTDOWN:
                    infof(data, "SSL_get_shutdown() returned SSL_RECEIVED_SHUTDOWN\n");
                    break;
                case SSL_SENT_SHUTDOWN|SSL_RECEIVED_SHUTDOWN:
                    infof(data, "SSL_get_shutdown() returned SSL_SENT_SHUTDOWN|"
                          "SSL_RECEIVED__SHUTDOWN\n");
                    break;
            }
#endif

I'm not sure in which OpenSSL version SSL_get_shutdown() was first introduced, but according to the docs it is at least available beginning with 1.0.2.

The patch follows the implementation strategy of other HAVE_* defines found in openssl.c, like HAVE_X509_GET0_SIGNATURE. If this is not the right approach, feel free to change.

Best, Zenju

Zenju added some commits Jul 9, 2019

@bagder

This comment has been minimized.

Copy link
Member

commented Jul 10, 2019

So then maybe also remove the check for it from configure.ac? It seems wrong to have two different checks for it...

@Zenju

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2019

I assume the HAVE_SSL_GET_SHUTDOWN mentioned in packages\vms\config_h.com should be scraped as well?

@jay

This comment has been minimized.

Copy link
Member

commented Jul 11, 2019

@wb8tyw can you tell us how does this PR affect VMS builds? Can we remove this for example?

@bagder

This comment has been minimized.

Copy link
Member

commented Jul 12, 2019

With the define handled in vtls/openssl.c, no platform should need to handle it special...

@bagder

This comment has been minimized.

Copy link
Member

commented Jul 14, 2019

Thanks!

@bagder bagder closed this in 7e8f191 Jul 14, 2019

caraitto added a commit to caraitto/curl that referenced this pull request Jul 23, 2019

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