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

[Julian Scheel]

On 10.03.2017 14:08, Rémi Denis-Courmont wrote:
> 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.

We'll add it to vlc_fixups.h in the NaCl patch series.
Discard this patch.