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

[Rémi Denis-Courmont]

On March 10, 2017 2:48:44 PM GMT+02:00, Dennis Hamester <dhamester at jusst.de> wrote:
>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.
>> 

It depends on TCP support via BSD socket, either as defined by POSIX or by Winsock. VLC assumes either of those two are available.
-- 
Rémi Denis-Courmont