[vlc-devel] [PATCH] tls: Fix build without netinet/tcp.h

[Dennis Hamester]

Well, I think we can agree, that the current call to setsockopt depends
on TCP_NODELAY being defined. And that is not generally true, so we
would like to guard the whole call with an #ifdef TCP_NODELAY.

Is that okay with you?

Best
Dennis

On 10.03.2017 13:21, Rémi Denis-Courmont wrote:
> On March 10, 2017 1:41:44 PM GMT+02:00, Julian Scheel <julian at jusst.de> wrote:
>> On 10.03.2017 12:27, Rémi Denis-Courmont wrote:
>>> On March 10, 2017 12:14:29 PM GMT+02:00, Julian Scheel
>> <julian at jusst.de> wrote:
>>>> It's not enough to define SOL_TCP if netinet/tcp.h is not available.
>> As
>>>> there is not toolchain agnostic value we could use to replace
>>>> TCP_NODELAY do only call setsockopt if netinet/tcp.h is available.
>>>>
>>>> Signed-off-by: Julian Scheel <julian at jusst.de>
>>>> ---
>>>> src/network/tls.c | 5 ++---
>>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/network/tls.c b/src/network/tls.c
>>>> index 3210cfe865..e76e59ca22 100644
>>>> --- a/src/network/tls.c
>>>> +++ b/src/network/tls.c
>>>> @@ -42,9 +42,6 @@
>>>> #ifdef HAVE_NETINET_TCP_H
>>>> # include <netinet/tcp.h>
>>>> #endif
>>>> -#ifndef SOL_TCP
>>>> -# define SOL_TCP IPPROTO_TCP
>>>> -#endif
>>>>
>>>> #include <vlc_common.h>
>>>> #include "libvlc.h"
>>>> @@ -440,8 +437,10 @@ static vlc_tls_t *vlc_tls_SocketAddrInfo(const
>>>> struct addrinfo *restrict info)
>>>>
>>>>   setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &(int){ 1 }, sizeof
>> (int));
>>>>
>>>> +#ifdef HAVE_NETINET_TCP_H
>>>> if (info->ai_socktype == SOCK_STREAM && info->ai_protocol ==
>>>> IPPROTO_TCP)
>>>>       setsockopt(fd, SOL_TCP, TCP_NODELAY, &(int){ 1 }, sizeof
>> (int));
>>>> +#endif
>>>>
>>>> vlc_tls_t *sk = vlc_tls_SocketAlloc(fd, info->ai_addr,
>>>> info->ai_addrlen);
>>>>     if (unlikely(sk == NULL))
>>>> --
>>>> 2.12.0
>>>>
>>>> _______________________________________________
>>>> vlc-devel mailing list
>>>> To unsubscribe or modify your subscription options:
>>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>
>>> Nack. This breaks Windows support
>>>
>>
>> Right, Windows declares TCP_NODELAY in ws2def.h, but you can't assume 
>> this to be true for any system not providing netinet/tcp.h.
>>
>> You break build on non-windows systems which don't have netinet/tcp.h 
>> with commit e5aa352, so Nack on that one.
>>
>> Anyway, would you be happy with putting the setsockopt call under
>> #ifdef 
>> TCP_NODELAY, keeping your SOL_TCP define. That way it will not change 
>> behaviour on Windows, but will still fix build on other systems.
>>
>> I'd be glad if instead of simply rejecting things you'd mind to give 
>> some thoughts on how you would like it to be.
>>
>> -Julian
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> 
> No, this does not depend on TCP_NODELAY being defined specifically in netinet/tcp.h nor does it depend on that header existing.
> 
> It just depends on BSD sockets with TCP/IP support. VLC has always been dependent on those two.
> 

-- 
Dennis Hamester
Software Development
-----------------------------------
jusst technologies GmbH

tel: +49 (0)40 1800 86 75
fax: +49 (0)40 1800 86 76
mail: dhamester at jusst.de

Ohlstedter Straße 12
22397 Hamburg
Deutschland

www.jusst.de

Vertretungsberechtigte Geschäftsführer: Julian Scheel, Wolfgang Scheel
Registergericht: Amtsgericht Hamburg
Registernummer: HRB 94300
USt-ID-Nr.: DE 243309917

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 862 bytes
Desc: OpenPGP digital signature
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170310/2fb8194d/attachment.sig>