< Previous by Date Date Index Next by Date >
  Thread Index Next in Thread >

[reSIProcate] PATCH: UdpTransport.cxx MSG_PEEK


Hi,

This patch peeks the socket first to allocate the correct amount of memory for 
the incoming packet, or bail out before allocating any on encountering an 
error.

Tested on linux 2.6 + 2.4, It should work fine on any sane OS, but i suggest 
people give it a once over to confirm.

Regarding detecting truncated messages - well, we should in theory never see 
them now (in linux anyway), but i'm not betting that all os'es don't still 
push just the received chunk within the SO_RECVBUF size down to userspace.  
in that case i can't really see any way to detect it.

hmm, just checked.  stevens rekons the datagram might be truncated on some 
implementations [1] if the packet size is greater than SO_RECVBUF, but i've 
not seen this behaviour happen in a good few years on any platforms i develop 
on (*bsd, linux + solaris).

Which brings me on to another question: which platforms does the resip stack 
"officially" support, and patches need to ensure work on?

 ~ Theo

1 - tcp/ip illustrated vol 1, 11.10 Maximum UDP Datagram Size, p160 

-- 
Theo P. Zourzouvillys

People who enjoy waiving flags don't deserve to have one
  -- Santa's Ghetto 2004, Banksy
Index: UdpTransport.cxx
===================================================================
--- UdpTransport.cxx	(revision 5387)
+++ UdpTransport.cxx	(working copy)
@@ -81,50 +81,57 @@
       }
    }
    
-   // !jf! this may have to change - when we read a message that is too big
    if ( fdset.readyToRead(mFd) )
    {
-      //should this buffer be allocated on the stack and then copied out, as it
-      //needs to be deleted every time EWOULDBLOCK is encountered
-      // .dlb. can we determine the size of the buffer before we allocate?
-      // something about MSG_PEEK|MSG_TRUNC in Stevens..
+      // Allocate the buffer only after peeking at the socket's head and getting
+      // the next datagrams size.  Assuming the next datagram is really (which it
+      // should always be, allocate the buffer and read the data in.
+
       // .dlb. RFC3261 18.1.1 MUST accept 65K datagrams. would have to attempt to
       // adjust the UDP buffer as well...
-      char* buffer = new char[MaxBufferSize + 5];
+      // (theo) We need to find a cross platform way of workign out the max then, 
+      // and/or provide an API method for setting the buffer size, perhaps in just
+      // UdpTransport's constructor?
 
+      int preLen = ::recv(mFd, NULL, 0, MSG_NOSIGNAL|MSG_PEEK|MSG_TRUNC);
+
+      if (preLen == SOCKET_ERROR) 
+      {
+         int err = getErrno();
+         // In theory, we should never see an ESHOULDBLOCK/EAGAIN here, but i'm sure 
+         // some messed up OS will report it.
+         if (err != EWOULDBLOCK) 
+         {
+            error(err);
+         }
+         return;
+      }
+      else if (preLen == 0) {
+        // Again, this should never happen, but i'm sure it will one day.
+        InfoLog(<< "Received 0 byte datagram?!");
+        return;
+      }
+
+      char * buffer = new char[preLen + 1];
+
       // !jf! how do we tell if it discarded bytes 
       // !ah! we use the len-1 trick :-(
       Tuple tuple(mTuple);
       socklen_t slen = tuple.length();
       int len = recvfrom( mFd,
                           buffer,
-                          MaxBufferSize,
+                          preLen,
                           0 /*flags */,
                           &tuple.getMutableSockaddr(), 
                           &slen);
-      if ( len == SOCKET_ERROR )
-      {
-         int err = getErrno();
-         if ( err != EWOULDBLOCK  )
-         {
-            error( err );
-         }
-      }
 
-      if (len == 0 || len == SOCKET_ERROR)
+      if ( len != preLen )
       {
-         delete[] buffer; 
-         buffer=0;
+         delete[] buffer;
+         error( getErrno() );
          return;
       }
 
-      if (len+1 >= MaxBufferSize)
-      {
-         InfoLog(<<"Datagram exceeded max length "<<MaxBufferSize);
-         delete [] buffer; buffer=0;
-         return;
-      }
-
       //handle incoming CRLFCRLF keep-alive packets
       if (len == 4 &&
           strncmp(buffer, Symbols::CRLFCRLF, len) == 0)