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

@Zenju Zenju 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

@bagder
Copy link
Member

bagder 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
Copy link
Contributor Author

Zenju commented Jul 10, 2019

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

@jay
Copy link
Member

jay commented Jul 11, 2019

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

@bagder
Copy link
Member

bagder commented Jul 12, 2019

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

@bagder
Copy link
Member

bagder commented Jul 14, 2019

Thanks!

@bagder bagder closed this in 7e8f191 Jul 14, 2019
caraitto pushed a commit to caraitto/curl that referenced this pull request Jul 23, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 12, 2019
@Zenju Zenju deleted the patch-4 branch November 7, 2022 14:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

None yet

3 participants