tls: Conditionally use SSL_set_tlsext_host_name if available

diff mbox

This fixes building libavformat with OpenSSL on slightly older
distributions (e.g. CentOS 5.5).
---
 libavformat/tls.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

On 2/27/12 11:01 AM, Martin Storsjö wrote:
> This fixes building libavformat with OpenSSL on slightly older
> distributions (e.g. CentOS 5.5).
> ---
>   libavformat/tls.c |    2 ++
>   1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/libavformat/tls.c b/libavformat/tls.c
> index fb84fa8..a9e3b27 100644
> --- a/libavformat/tls.c
> +++ b/libavformat/tls.c
> @@ -177,8 +177,10 @@ static int tls_open(URLContext *h, const char *uri, int flags)
>           goto fail;
>       }
>       SSL_set_fd(c->ssl, c->fd);
> +#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
>       if (!numerichost)
>           SSL_set_tlsext_host_name(c->ssl, host);
> +#endif
>       while (1) {
>           ret = SSL_connect(c->ssl);
>           if (ret>  0)

Ok.

On Mon, 27 Feb 2012, Diego Biurrun wrote:

> On Mon, Feb 27, 2012 at 12:01:51PM +0200, Martin Storsjö wrote:
>> This fixes building libavformat with OpenSSL on slightly older
>> distributions (e.g. CentOS 5.5).
>> ---
>>  libavformat/tls.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> CentOS 6 is out.  I've been through the pains of supporting the old
> 5.5 version with nightmarishly outdated versions of everything at work,
> I really don't want to go through the pain again.  Do you think this is
> worth the trouble?  By the time this change has trickled down from HEAD
> to the distros, everybody will have updated and those that did not,
> should.

I occasionally build and test a larger setup including libavformat on such 
a CentOS machine, and that setup enables the openssl code - this patch 
fixes building that setup. So for my case, I wouldn't need to wait for 
this fix to be included in the distros themselves.

If you're opposed, I can live without this, but this particular patch is 
kinda nonintrusive, and I'd probably have written it in this way anyway if 
I had ran into the issue while making the tls stuff last year.

// Martin

On Mon, Feb 27, 2012 at 12:26:45PM +0200, Martin Storsjö wrote:
> On Mon, 27 Feb 2012, Diego Biurrun wrote:
> 
> >On Mon, Feb 27, 2012 at 12:01:51PM +0200, Martin Storsjö wrote:
> >>This fixes building libavformat with OpenSSL on slightly older
> >>distributions (e.g. CentOS 5.5).
> >>---
> >> libavformat/tls.c |    2 ++
> >> 1 files changed, 2 insertions(+), 0 deletions(-)
> >
> >CentOS 6 is out.  I've been through the pains of supporting the old
> >5.5 version with nightmarishly outdated versions of everything at work,
> >I really don't want to go through the pain again.  Do you think this is
> >worth the trouble?  By the time this change has trickled down from HEAD
> >to the distros, everybody will have updated and those that did not,
> >should.
> 
> I occasionally build and test a larger setup including libavformat
> on such a CentOS machine, and that setup enables the openssl code -
> this patch fixes building that setup. So for my case, I wouldn't
> need to wait for this fix to be included in the distros themselves.

So why not change the OpenSSL check to test for the availability of
that particular function?

> If you're opposed, I can live without this, but this particular
> patch is kinda nonintrusive, and I'd probably have written it in
> this way anyway if I had ran into the issue while making the tls
> stuff last year.

To be honest, I disagree slightly about the non-intrusiveness.
ifdefs are ifdefs and we should fight them tooth and nail.

Diego

On Mon, 27 Feb 2012, Diego Biurrun wrote:

> On Mon, Feb 27, 2012 at 12:26:45PM +0200, Martin Storsjö wrote:
>> On Mon, 27 Feb 2012, Diego Biurrun wrote:
>>
>>> On Mon, Feb 27, 2012 at 12:01:51PM +0200, Martin Storsjö wrote:
>>>> This fixes building libavformat with OpenSSL on slightly older
>>>> distributions (e.g. CentOS 5.5).
>>>> ---
>>>> libavformat/tls.c |    2 ++
>>>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>> CentOS 6 is out.  I've been through the pains of supporting the old
>>> 5.5 version with nightmarishly outdated versions of everything at work,
>>> I really don't want to go through the pain again.  Do you think this is
>>> worth the trouble?  By the time this change has trickled down from HEAD
>>> to the distros, everybody will have updated and those that did not,
>>> should.
>>
>> I occasionally build and test a larger setup including libavformat
>> on such a CentOS machine, and that setup enables the openssl code -
>> this patch fixes building that setup. So for my case, I wouldn't
>> need to wait for this fix to be included in the distros themselves.
>
> So why not change the OpenSSL check to test for the availability of
> that particular function?

That's of course also an option. (In this case, iirc this isn't a function 
but a macro, but we can check for the same define as the suggested ifdef 
does.)

>> If you're opposed, I can live without this, but this particular
>> patch is kinda nonintrusive, and I'd probably have written it in
>> this way anyway if I had ran into the issue while making the tls
>> stuff last year.
>
> To be honest, I disagree slightly about the non-intrusiveness.
> ifdefs are ifdefs and we should fight them tooth and nail.

Perhaps... But blocking using openssl at all just because we might want to 
use this optional feature if it is present feels like overkill to me, even 
if I know it is the norm of how we usually do things.

If it is less evil to you, I can move the ifdef to the header of the file 
to avoid having the ifdef inline.

// Martin