Ensure handle_ fits in FD_SETSIZE.

base/sync_socket_posix.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/sync_socket.h"
 
 #include <errno.h>
 #include <fcntl.h>
 #include <limits.h>
 #include <stdio.h>
@@ skipping 44 matching lines @@
   DCHECK_NE(socket_a, socket_b);
   DCHECK_EQ(socket_a->handle_, kInvalidHandle);
   DCHECK_EQ(socket_b->handle_, kInvalidHandle);
 
 #if defined(OS_MACOSX)
   int nosigpipe = 1;
 #endif  // defined(OS_MACOSX)
 
   Handle handles[2] = { kInvalidHandle, kInvalidHandle };
   if (socketpair(AF_UNIX, SOCK_STREAM, 0, handles) != 0)
     goto cleanup;

[jar (doing other things), 2013/11/01 00:00:13]

nit: If goto's bug you as much as they bother me... consider putting this into
a:

 do { } while(0) 

loop (start just before line 64, and close curly at line 83), and then replace
with "break" here and in line 74 with goto.

[DaleCurtis, 2013/11/01 00:15:59]

Better: moved into anonymous function.

[jar (doing other things), 2013/11/01 00:22:28]

Perfect ;-)

 
 #if defined(OS_MACOSX)
   // On OSX an attempt to read or write to a closed socket may generate a
   // SIGPIPE rather than returning -1.  setsockopt will shut this off.
   if (0 != setsockopt(handles[0], SOL_SOCKET, SO_NOSIGPIPE,
                       &nosigpipe, sizeof nosigpipe) ||
       0 != setsockopt(handles[1], SOL_SOCKET, SO_NOSIGPIPE,
                       &nosigpipe, sizeof nosigpipe)) {
     goto cleanup;
   }
 #endif
 
   // Copy the handles out for successful return.
   socket_a->handle_ = handles[0];
   socket_b->handle_ = handles[1];

[jar (doing other things), 2013/11/01 00:00:13]

Wouldn't it be more interesting to test where the setting took place, assuming
it was not random external corruption, such as:

CHECK_LT(handles[0], FD_SETSIZE);
CHECK_LT(handles[1], FD_SETSIZE);

[DaleCurtis, 2013/11/01 00:15:59]

I considered this, but the handle only needs to be less than FD_SETSIZE when
used with an fd_set in ReceiveWithTimeout(); which is currently only done in the
browser process.  It's seemingly conceivable that the socketpair may have a
handle in the renderer process which is >= FD_SETSIZE and one in the browser
process which is < FD_SETSIZE.

 
   return true;
 
  cleanup:
   if (handles[0] != kInvalidHandle) {
     if (HANDLE_EINTR(close(handles[0])) < 0)
       DPLOG(ERROR) << "close";
   }
   if (handles[1] != kInvalidHandle) {
     if (HANDLE_EINTR(close(handles[1])) < 0)
@@ skipping 28 matching lines @@
   return 0;
 }
 
 size_t SyncSocket::ReceiveWithTimeout(void* buffer,
                                       size_t length,
                                       TimeDelta timeout) {
   ThreadRestrictions::AssertIOAllowed();
   DCHECK_GT(length, 0u);
   DCHECK_LE(length, kMaxMessageLength);
   DCHECK_NE(handle_, kInvalidHandle);
-  DCHECK_LT(handle_, FD_SETSIZE);
+  CHECK_LT(handle_, FD_SETSIZE);
 
   // Only timeouts greater than zero and less than one second are allowed.
   DCHECK_GT(timeout.InMicroseconds(), 0);
   DCHECK_LT(timeout.InMicroseconds(),
             base::TimeDelta::FromSeconds(1).InMicroseconds());
 
   // Track the start time so we can reduce the timeout as data is read.
   TimeTicks start_time = TimeTicks::Now();
   const TimeTicks finish_time = start_time + timeout;
 
@@ skipping 78 matching lines @@
   return len;
 }
 
 // static
 bool CancelableSyncSocket::CreatePair(CancelableSyncSocket* socket_a,
                                       CancelableSyncSocket* socket_b) {
   return SyncSocket::CreatePair(socket_a, socket_b);
 }
 
 }  // namespace base