[FFmpeg-devel] [PATCH] lavf/tls_openssl: Support building with LibreSSL

[wm4]

On Sat, 28 Jan 2017 13:01:54 +0000
Mark Thompson <sw at jkqxz.net> wrote:

> On 28/01/17 11:28, wm4 wrote:
> > On Fri, 27 Jan 2017 19:53:50 +0000
> > Mark Thompson <sw at jkqxz.net> wrote:
> >   
> >> On 27/01/17 19:15, Marek Behun wrote:  
> >>> On Fri, 27 Jan 2017 18:41:09 +0000
> >>> Mark Thompson <sw at jkqxz.net> wrote:
> >>>     
> >>>> On 27/01/17 17:31, Marek Behún wrote:    
> >>>>> Use the LIBRESSL_VERSION_NUMBER macro to determine if building with
> >>>>> LibreSSL instead of OpenSSL. This is pretty straightforward, since
> >>>>> it is enough to add this check to existing #if macros.
> >>>>>
> >>>>> Signed-off-by: Marek Behun <kabel at blackhole.sk>
> >>>>> ---
> >>>>>  libavformat/tls_openssl.c | 12 ++++++------
> >>>>>  1 file changed, 6 insertions(+), 6 deletions(-)
> >>>>>
> >>>>> diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
> >>>>> index 3d9768a..cf1a62e 100644
> >>>>> --- a/libavformat/tls_openssl.c
> >>>>> +++ b/libavformat/tls_openssl.c
> >>>>> @@ -43,7 +43,7 @@ typedef struct TLSContext {
> >>>>>      TLSShared tls_shared;
> >>>>>      SSL_CTX *ctx;
> >>>>>      SSL *ssl;
> >>>>> -#if OPENSSL_VERSION_NUMBER >= 0x1010000fL
> >>>>> +#if OPENSSL_VERSION_NUMBER >= 0x1010000fL
> >>>>> && !defined(LIBRESSL_VERSION_NUMBER)      
> >>>>
> >>>> I don't understand what this is trying to do.
> >>>>
> >>>> Does LibreSSL support the OpenSSL 1.1.0 API:
> >>>>
> >>>> If yes, why would the additional check be needed?
> >>>>
> >>>> If no, isn't this doing nothing because the first check would be
> >>>> false?    
> >>>
> >>> LibreSSL defines OPENSSL_VERSION_NUMBER to >=0x2000000, thus
> >>> OPENSSL_VERSION_NUMBER is always greater than 0x1010000, but LibreSSL
> >>> does not support 1.1.0 API.    
> >>
> >> Er, right, so it just lies and leaves it to user programs to sort it out.  How nice.
> >>
> >> Looking back, I can see this has been discussed before:
> >> <https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2016-October/201960.html>
> >> <https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2016-December/203998.html>
> >>
> >> That (beyond the disapprobation towards libressl for being naughty) looks like people would prefer the test to be in configure rather than copying the nontrivial #if condition everywhere?  
> > 
> > Maybe LibreSSL should fix this upstream.
> > 
> > They're doing an extreme disservice to everyone by breaking every
> > single downstream program.
> > 
> > I'd even go as far as saying we shouldn't bother with LibreSSL if
> > trying to keep compatibility is going to be a mess this huge.  
> 
> If it becomes more inconvenient to do so, yes. 

> (At that point probably just clone tls_openssl.c to tls_libressl.c and
> let them diverge if support is still wanted.)

Well, I suspect LibreSSL will continue their "low quality"
compatibility with OpenSSL, so it will remain a cat and mouse game.

> On the other hand, I think the now-proposed change (configure-detected) is positive even ignoring the existence of LibreSSL, since it moves a whole set of repeated version-conditional #ifs into one configure variable.

Yes, until next time another part of the API breaks.

Anyway, just to make it clear: I don't mind the currently proposed
patch.