Bug at getsockopt when TCP_NODELAY is used as parameter

[jdzstz]

While compiling varnish 3.0.5, I have detected a possible bug at 
getsockopt when TCP_NODELAY is used as parameter. The issue is caused 
because at cygwin it returns a BOOL, instead returning INT value like at 
Linux.

At varnish 3.0.5, the following code at "sock_test(int fd)" uses getsockopt:
    - 
https://www.varnish-cache.org/trac/browser/bin/varnishd/cache_acceptor.c?rev=dec9098a2ac760d7ba4765a9a1a1091103b6eb05

     l = sizeof tcp_nodelay;
     i = getsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &tcp_nodelay, &l);
     if (i) {
         VTCP_Assert(i);
         return;
     }
     assert(l == sizeof tcp_nodelay);
     if (!tcp_nodelay)
         need_tcpnodelay = 1;


The problem is that at cygwin, this ASSERT fails:
     -     assert(l == sizeof tcp_nodelay);

Because before function call, the initial tcp_nodelay variable size is 
INT, but after funtion call, it returns a pointer to a BOOL variable, so 
assert (sizeof INT == sizeof BOOL) fails.

After some investigations I have seen that at windows, its getsockopt 
function, returns BOOL for TCP_NODELAY:
     - 
http://msdn.microsoft.com/en-us/library/windows/desktop/ms738544%28v=vs.85%29.aspx

But at Unix, the correct behaviour is to return INT:
     - http://www.yolinux.com/TUTORIALS/Sockets.html

I suppose that the cygwin getsockopt internally calls to Windows 
getsockopt, and no type conversion of variables is done. I think the 
solution would be to wrap getsockopt response and change it from bool 
size variable to a int size variable.

As a temporary workaround for varnish 3.0.5 I have changed the variable 
types to bool, but I think It would be better to directly fix it at 
cygwin layer, to avoid modifications to original Linux code:

--- origsrc/varnish-3.0.5/bin/varnishd/cache_acceptor.c 2013-12-02 
08:48:15.000000000 +0100
+++ src/varnish-3.0.5/bin/varnishd/cache_acceptor.c 2014-04-17 
21:31:12.555512600 +0200
@@ -36,6 +36,9 @@
  #include <string.h>
  #include <stdlib.h>
  #include <unistd.h>
+#if defined(__CYGWIN__)
+#include <stdbool.h>
+#endif

  #include <sys/uio.h>
  #include <sys/types.h>
@@ -105,7 +108,12 @@ sock_test(int fd)
      struct linger lin;
      struct timeval tv;
      socklen_t l;
-    int i, tcp_nodelay;
+    int i;
+    #if defined(__CYGWIN__)
+    bool tcp_nodelay;
+    #else
+    int tcp_nodelay;
+    #endif

      l = sizeof lin;
      i = getsockopt(fd, SOL_SOCKET, SO_LINGER, &lin, &l);
@@ -172,7 +180,11 @@ VCA_Prep(struct sess *sp)
  {
      char addr[VTCP_ADDRBUFSIZE];
      char port[VTCP_PORTBUFSIZE];
+    #if defined(__CYGWIN__)
+    bool tcp_nodelay = true;
+    #else
      int tcp_nodelay = 1;
+    #endif

      VTCP_name(sp->sockaddr, sp->sockaddrlen,
          addr, sizeof addr, port, sizeof port);


Thank you
Jorge








---
This email is free from viruses and malware because avast! Antivirus protection is active.
http://www.avast.com


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple