Re: ws2_32: socket keepalive for OSX

[Bruno Jesus]

Hi, Matt, thanks for the patch, here is some feedback.

Please use a 4 space identation like the code style in this function.
> +#elif defined(SO_KEEPALIVE) && defined(TCP_KEEPALIVE)

SO_KEEPALIVE is defined for sure (already used outside #ifdef) so
there is no need to test.
> - /* BSDish platforms like Darwin */
> - else if (keepalive)
> - {
> - if (setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, (void*)&keepalive, sizeof(int)) == 1)
> - status = WSAEINVAL;

SO_KEEPALIVE value was already set by a previous call before the
#ifdef. Is there a reason to repeat it? Also you are testing the
return value for 1 instead of -1.
> - else if (setsockopt(fd, IPPROTO_TCP, TCP_KEEPALIVE, (void*)&keepintvl, sizeof(int)) == -1)
> - status = WSAEINVAL;
> - }

Please note that in the rest of the code it is used (void *), not (void*).

Best wishes,
Bruno

On Wed, Jul 29, 2015 at 10:30 PM, Matt Durgavich
 wrote:
> Set socket options for keepalive on OSX tcp connections.
> No tests at this time.
>
> dlls/ws2_32/socket.c | 9 +++++++++
>  1 file changed, 9 insertions(+)