Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(563)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: net/socket/tcp_socket_win.cc

Issue 2539833003: Call WSAGetLastError immediately after the potentially failing call. (Closed)
Patch Set: Created 4 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/socket/tcp_socket_win.cc
diff --git a/net/socket/tcp_socket_win.cc b/net/socket/tcp_socket_win.cc
index d879a5ee95e7fdf62b22ee4f3ff17d545396db0b..aa64214dcec0d2075d313ce5964fffc9b18a59d0 100644
--- a/net/socket/tcp_socket_win.cc
+++ b/net/socket/tcp_socket_win.cc
@@ -40,7 +40,8 @@ const int kTCPKeepAliveSeconds = 45;
int SetSocketReceiveBufferSize(SOCKET socket, int32_t size) {
int rv = setsockopt(socket, SOL_SOCKET, SO_RCVBUF,
reinterpret_cast<const char*>(&size), sizeof(size));
- int net_error = (rv == 0) ? OK : MapSystemError(WSAGetLastError());
+ int os_error = WSAGetLastError();
+ int net_error = (rv == 0) ? OK : MapSystemError(os_error);
DCHECK(!rv) << "Could not set socket receive buffer size: " << net_error;
return net_error;
}
@@ -48,7 +49,8 @@ int SetSocketReceiveBufferSize(SOCKET socket, int32_t size) {
int SetSocketSendBufferSize(SOCKET socket, int32_t size) {
int rv = setsockopt(socket, SOL_SOCKET, SO_SNDBUF,
reinterpret_cast<const char*>(&size), sizeof(size));
- int net_error = (rv == 0) ? OK : MapSystemError(WSAGetLastError());
+ int os_error = WSAGetLastError();
eroman 2016/11/29 19:51:00 For these two cases, does the re-ordering make a d
Sigurður Ásgeirsson 2016/11/29 20:27:55 This is purely for consistency. Note that C++ can
+ int net_error = (rv == 0) ? OK : MapSystemError(os_error);
DCHECK(!rv) << "Could not set socket send buffer size: " << net_error;
return net_error;
}
@@ -67,8 +69,9 @@ bool SetTCPKeepAlive(SOCKET socket, BOOL enable, int delay_secs) {
int rv = WSAIoctl(socket, SIO_KEEPALIVE_VALS, &keepalive_vals,
sizeof(keepalive_vals), NULL, 0,
&bytes_returned, NULL, NULL);
+ int os_error = WSAGetLastError();
DCHECK(!rv) << "Could not enable TCP Keep-Alive for socket: " << socket
- << " [error: " << WSAGetLastError() << "].";
+ << " [error: " << os_error << "].";
// Disregard any failure in disabling nagle or enabling TCP Keep-Alive.
return rv == 0;
@@ -276,13 +279,15 @@ int TCPSocketWin::Open(AddressFamily family) {
socket_ = CreatePlatformSocket(ConvertAddressFamily(family), SOCK_STREAM,
IPPROTO_TCP);
+ int os_error = WSAGetLastError();
eroman 2016/11/29 19:51:00 Same issue as for base::SetNonBlocking() -- relies
Sigurður Ásgeirsson 2016/11/29 20:27:55 I don't know whether this works the same on other
if (socket_ == INVALID_SOCKET) {
PLOG(ERROR) << "CreatePlatformSocket() returned an error";
- return MapSystemError(WSAGetLastError());
+ return MapSystemError(os_error);
}
if (!base::SetNonBlocking(socket_)) {
- int result = MapSystemError(WSAGetLastError());
+ os_error = WSAGetLastError();
+ int result = MapSystemError(os_error);
Close();
return result;
}
@@ -299,7 +304,8 @@ int TCPSocketWin::AdoptConnectedSocket(SOCKET socket,
socket_ = socket;
if (!base::SetNonBlocking(socket_)) {
- int result = MapSystemError(WSAGetLastError());
+ int os_error = WSAGetLastError();
eroman 2016/11/29 19:51:00 Given this is called 3 times, I would suggest addi
Sigurður Ásgeirsson 2016/11/29 20:27:55 Done.
+ int result = MapSystemError(os_error);
Close();
return result;
}
@@ -317,7 +323,8 @@ int TCPSocketWin::AdoptListenSocket(SOCKET socket) {
socket_ = socket;
if (!base::SetNonBlocking(socket_)) {
- int result = MapSystemError(WSAGetLastError());
+ int os_error = WSAGetLastError();
+ int result = MapSystemError(os_error);
Close();
return result;
}
@@ -337,9 +344,10 @@ int TCPSocketWin::Bind(const IPEndPoint& address) {
return ERR_ADDRESS_INVALID;
int result = bind(socket_, storage.addr, storage.addr_len);
+ int os_error = WSAGetLastError();
if (result < 0) {
PLOG(ERROR) << "bind() returned an error";
- return MapSystemError(WSAGetLastError());
+ return MapSystemError(os_error);
}
return OK;
@@ -352,15 +360,17 @@ int TCPSocketWin::Listen(int backlog) {
DCHECK_EQ(accept_event_, WSA_INVALID_EVENT);
accept_event_ = WSACreateEvent();
+ int os_error = WSAGetLastError();
if (accept_event_ == WSA_INVALID_EVENT) {
PLOG(ERROR) << "WSACreateEvent()";
- return MapSystemError(WSAGetLastError());
+ return MapSystemError(os_error);
}
int result = listen(socket_, backlog);
+ os_error = WSAGetLastError();
if (result < 0) {
PLOG(ERROR) << "listen() returned an error";
- return MapSystemError(WSAGetLastError());
+ return MapSystemError(os_error);
}
return OK;
@@ -438,9 +448,10 @@ bool TCPSocketWin::IsConnected() const {
// Check if connection is alive.
char c;
int rv = recv(socket_, &c, 1, MSG_PEEK);
+ int os_error = WSAGetLastError();
if (rv == 0)
return false;
- if (rv == SOCKET_ERROR && WSAGetLastError() != WSAEWOULDBLOCK)
+ if (rv == SOCKET_ERROR && os_error != WSAEWOULDBLOCK)
return false;
return true;
@@ -459,9 +470,10 @@ bool TCPSocketWin::IsConnectedAndIdle() const {
// unexpectedly.
char c;
int rv = recv(socket_, &c, 1, MSG_PEEK);
+ int os_error = WSAGetLastError();
if (rv >= 0)
return false;
- if (WSAGetLastError() != WSAEWOULDBLOCK)
+ if (os_error != WSAEWOULDBLOCK)
return false;
return true;
@@ -498,6 +510,7 @@ int TCPSocketWin::Write(IOBuffer* buf,
DWORD num;
int rv = WSASend(socket_, &write_buffer, 1, &num, 0,
&core_->write_overlapped_, NULL);
+ int os_error = WSAGetLastError();
if (rv == 0) {
if (ResetEventIfSignaled(core_->write_overlapped_.hEvent)) {
rv = static_cast<int>(num);
@@ -514,7 +527,6 @@ int TCPSocketWin::Write(IOBuffer* buf,
return rv;
}
} else {
- int os_error = WSAGetLastError();
if (os_error != WSA_IO_PENDING) {
int net_error = MapSystemError(os_error);
net_log_.AddEvent(NetLogEventType::SOCKET_WRITE_ERROR,
@@ -535,8 +547,10 @@ int TCPSocketWin::GetLocalAddress(IPEndPoint* address) const {
DCHECK(address);
SockaddrStorage storage;
- if (getsockname(socket_, storage.addr, &storage.addr_len))
- return MapSystemError(WSAGetLastError());
+ if (getsockname(socket_, storage.addr, &storage.addr_len)) {
+ int os_error = WSAGetLastError();
+ return MapSystemError(os_error);
+ }
if (!address->FromSockAddr(storage.addr, storage.addr_len))
return ERR_ADDRESS_INVALID;
@@ -689,8 +703,9 @@ int TCPSocketWin::AcceptInternal(std::unique_ptr<TCPSocketWin>* socket,
IPEndPoint* address) {
SockaddrStorage storage;
int new_socket = accept(socket_, storage.addr, &storage.addr_len);
+ int os_error = WSAGetLastError();
if (new_socket < 0) {
- int net_error = MapSystemError(WSAGetLastError());
+ int net_error = MapSystemError(os_error);
if (net_error != ERR_IO_PENDING)
net_log_.EndEventWithNetErrorCode(NetLogEventType::TCP_ACCEPT, net_error);
return net_error;
@@ -763,11 +778,13 @@ int TCPSocketWin::DoConnect() {
return ERR_ADDRESS_INVALID;
int result;
+ int os_error;
{
// TODO(ricea): Remove ScopedTracker below once crbug.com/436634 is fixed.
tracked_objects::ScopedTracker tracking_profile(
FROM_HERE_WITH_EXPLICIT_FUNCTION("436634 connect()"));
result = connect(socket_, storage.addr, storage.addr_len);
+ os_error = WSAGetLastError();
}
if (!result) {
@@ -786,7 +803,6 @@ int TCPSocketWin::DoConnect() {
if (ResetEventIfSignaled(core_->read_overlapped_.hEvent))
return OK;
} else {
- int os_error = WSAGetLastError();
if (os_error != WSAEWOULDBLOCK) {
LOG(ERROR) << "connect failed: " << os_error;
connect_os_error_ = os_error;
@@ -834,9 +850,9 @@ void TCPSocketWin::LogConnectEnd(int net_error) {
socklen_t addrlen = sizeof(source_address);
int rv = getsockname(
socket_, reinterpret_cast<struct sockaddr*>(&source_address), &addrlen);
+ int os_error = WSAGetLastError();
if (rv != 0) {
- LOG(ERROR) << "getsockname() [rv: " << rv
- << "] error: " << WSAGetLastError();
+ LOG(ERROR) << "getsockname() [rv: " << rv << "] error: " << os_error;
NOTREACHED();
net_log_.EndEventWithNetErrorCode(NetLogEventType::TCP_CONNECT, rv);
return;
@@ -857,8 +873,8 @@ int TCPSocketWin::DoRead(IOBuffer* buf, int buf_len,
core_->non_blocking_reads_initialized_ = true;
}
int rv = recv(socket_, buf->data(), buf_len, 0);
+ int os_error = WSAGetLastError();
if (rv == SOCKET_ERROR) {
- int os_error = WSAGetLastError();
if (os_error != WSAEWOULDBLOCK) {
int net_error = MapSystemError(os_error);
net_log_.AddEvent(NetLogEventType::SOCKET_READ_ERROR,
@@ -887,6 +903,7 @@ void TCPSocketWin::DidCompleteConnect() {
WSANETWORKEVENTS events;
int rv;
+ int os_error = 0;
{
// TODO(pkasting): Remove ScopedTracker below once crbug.com/462784 is
// fixed.
@@ -894,11 +911,10 @@ void TCPSocketWin::DidCompleteConnect() {
FROM_HERE_WITH_EXPLICIT_FUNCTION(
"462784 TCPSocketWin::DidCompleteConnect -> WSAEnumNetworkEvents"));
rv = WSAEnumNetworkEvents(socket_, core_->read_overlapped_.hEvent, &events);
+ os_error = WSAGetLastError();
}
- int os_error = 0;
if (rv == SOCKET_ERROR) {
NOTREACHED();
- os_error = WSAGetLastError();
result = MapSystemError(os_error);
} else if (events.lNetworkEvents & FD_CONNECT) {
os_error = events.iErrorCode[FD_CONNECT_BIT];
@@ -928,10 +944,10 @@ void TCPSocketWin::DidCompleteWrite() {
BOOL ok = WSAGetOverlappedResult(socket_, &core_->write_overlapped_,
&num_bytes, FALSE, &flags);
WSAResetEvent(core_->write_overlapped_.hEvent);
+ int os_error = WSAGetLastError();
eroman 2016/11/29 19:51:00 Based on the conditional where this is used, I thi
Sigurður Ásgeirsson 2016/11/29 20:27:55 Done, though this probably does change semantics i
waiting_write_ = false;
int rv;
if (!ok) {
- int os_error = WSAGetLastError();
rv = MapSystemError(os_error);
net_log_.AddEvent(NetLogEventType::SOCKET_WRITE_ERROR,
CreateNetLogSocketErrorCallback(rv, os_error));
@@ -965,8 +981,9 @@ void TCPSocketWin::DidSignalRead() {
WSANETWORKEVENTS network_events;
int rv = WSAEnumNetworkEvents(socket_, core_->read_overlapped_.hEvent,
&network_events);
+ os_error = WSAGetLastError();
+
if (rv == SOCKET_ERROR) {
- os_error = WSAGetLastError();
rv = MapSystemError(os_error);
} else if (network_events.lNetworkEvents) {
// TODO(pkasting): Remove ScopedTracker below once crbug.com/462778 is
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698