Ensure handle_ fits in FD_SETSIZE.

Ensure handle_ fits in FD_SETSIZE.

Speculative attempt to diagnose stack corruption issues.

BUG=313843
TEST=none

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=232449

[jar (doing other things), 2013-11-01 00:00:12]

comments below.... and I'd like you to consider them.

Please either answer "why not" or make the changes, and then LGTM.

Thanks.

https://codereview.chromium.org/54993006/diff/1/base/sync_socket_posix.cc
File base/sync_socket_posix.cc (right):

https://codereview.chromium.org/54993006/diff/1/base/sync_socket_posix.cc#new...
base/sync_socket_posix.cc:65: goto cleanup;
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.

https://codereview.chromium.org/54993006/diff/1/base/sync_socket_posix.cc#new...
base/sync_socket_posix.cc:80: socket_b->handle_ = handles[1];
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]

Look good?

https://codereview.chromium.org/54993006/diff/1/base/sync_socket_posix.cc
File base/sync_socket_posix.cc (right):

https://codereview.chromium.org/54993006/diff/1/base/sync_socket_posix.cc#new...
base/sync_socket_posix.cc:65: goto cleanup;
On 2013/11/01 00:00:13, jar wrote:
> 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.
> 

Better: moved into anonymous function.

https://codereview.chromium.org/54993006/diff/1/base/sync_socket_posix.cc#new...
base/sync_socket_posix.cc:80: socket_b->handle_ = handles[1];
On 2013/11/01 00:00:13, jar wrote:
> 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);

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.

https://codereview.chromium.org/54993006/diff/60001/base/sync_socket_posix.cc
File base/sync_socket_posix.cc (right):

https://codereview.chromium.org/54993006/diff/60001/base/sync_socket_posix.cc...
base/sync_socket_posix.cc:44: if (handle != SyncSocket::kInvalidHandle &&
close(handle) < 0) {
Removed the EINTR here per http://crbug.com/269623 too.

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

lgtm

https://codereview.chromium.org/54993006/diff/1/base/sync_socket_posix.cc
File base/sync_socket_posix.cc (right):

https://codereview.chromium.org/54993006/diff/1/base/sync_socket_posix.cc#new...
base/sync_socket_posix.cc:65: goto cleanup;
On 2013/11/01 00:15:59, DaleCurtis wrote:
> On 2013/11/01 00:00:13, jar wrote:
> > 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.
> > 
> 
> Better: moved into anonymous function.

Perfect ;-)

[commit-bot: I haz the power, 2013-11-01 00:38:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/54993006/60001

[commit-bot: I haz the power, 2013-11-01 04:47:58]

Retried try job too often on win7_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-11-01 16:49:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/54993006/60001

[commit-bot: I haz the power, 2013-11-01 19:58:14]

Change committed as 232449