Issue 458563002: [NaCl SDK] Fix bug in TCPNode::Connect

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Issue 458563002: [NaCl SDK] Fix bug in TCPNode::Connect (Closed)

Created:
6 years, 4 months ago by binji

Modified:
6 years, 4 months ago

Reviewers:
noelallen1

CC:
chromium-reviews, binji+watch_chromium.org, Sam Clegg

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Project:
chromium

Visibility:
Public.

More Reviews

Description

[NaCl SDK] Fix bug in TCPNode::Connect Calling connect() would not fail if there was an error connecting. Instead it would return 0 (success) and fail on the first read/write. The fix is to check SocketNode::last_errno_ after the wait succeeds. BUG=none R=noelallen@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=288480

Patch Set 1 #

Created: 6 years, 4 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+16 lines, -0 lines)			Patch
	M	native_client_sdk/src/libraries/nacl_io/socket/tcp_node.cc	View		1 chunk	+6 lines, -0 lines	0 comments	Download
	M	native_client_sdk/src/tests/nacl_io_socket_test/socket_test.cc	View		1 chunk	+10 lines, -0 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

binji

Committed patchset #1 manually as r288480 (presubmit successful).

6 years, 4 months ago (2014-08-09 00:37:50 UTC) #3

manish.baj

6 years, 4 months ago (2014-08-09 19:27:12 UTC) #4

Message was sent while issue was closed.

On 2014/08/09 00:37:50, binji wrote:
> Committed patchset #1 manually as r288480 (presubmit successful).

This needs further work, IMO. Once connect() returns "connection refused", the
client can choose to wait and try connect() again. Say now the server is
listening. So connect should succeed. But in your fix it won't because
SSF_CONNECTING is set. Also, last_errno_ might need to be reset. I tried all
that, and it still didn't work for me. So there is something more that I am
missing.

Expand Messages | Collapse Messages