adb: improve network error info
- handle_forward_request
- Because we have detailed info about which syscall failed (at least
on Win32), use a more generic prefix of "cannot bind listener" followed
by the detailed info.
- install_listener
- Return string errors for a few errors even though I don't think any
callers actually output the string for those errors.
- Remove the printf since the callers print the message themselves.
- adb_main
- LOG(FATAL) calls abort() which on Windows calls the Windows Error
Reporting service which pops up a dialog asking if you want a
crashdump to be uploaded to Microsoft. So really, abort() is
designed for app bugs. Windows isn't the only one doing this, Chromium
also makes LOG(FATAL) crashdump-ready. Since an error here is not
necessarily an app-bug, use a 'normal' error output API like fatal()
which prints an error and just uses exit().
- sysdeps_win32.cpp
- When Winsock APIs fail, make the string clarify which API failed.
Use terse unix-style descriptions (like what you'd get from
cp/mv/dd/etc.).
- Don't trace WSAEWOULDBLOCK from recv() which is a normal occurrence.
- Add a comment about WSAEWOULDBLOCK => EAGAIN.
Change-Id: I58e47f49fa2f6c1b4b92a36d0c4bfe369b456f2a
Signed-off-by: Spencer Low <CompareAndSwap@gmail.com>
diff --git a/adb.cpp b/adb.cpp
index 821b785..f4e779c 100644
--- a/adb.cpp
+++ b/adb.cpp
@@ -847,7 +847,7 @@
case INSTALL_STATUS_OK: message = "success (!)"; break;
case INSTALL_STATUS_INTERNAL_ERROR: message = "internal error"; break;
case INSTALL_STATUS_CANNOT_BIND:
- message = android::base::StringPrintf("cannot bind to socket: %s",
+ message = android::base::StringPrintf("cannot bind listener: %s",
error.c_str());
break;
case INSTALL_STATUS_CANNOT_REBIND:
diff --git a/adb_listeners.cpp b/adb_listeners.cpp
index 1e7ce5d..8fb2d19 100644
--- a/adb_listeners.cpp
+++ b/adb_listeners.cpp
@@ -190,16 +190,19 @@
/* can't repurpose a smartsocket */
if(l->connect_to[0] == '*') {
+ *error = "cannot repurpose smartsocket";
return INSTALL_STATUS_INTERNAL_ERROR;
}
/* can't repurpose a listener if 'no_rebind' is true */
if (no_rebind) {
+ *error = "cannot rebind";
return INSTALL_STATUS_CANNOT_REBIND;
}
cto = strdup(connect_to);
if(cto == 0) {
+ *error = "cannot duplicate string";
return INSTALL_STATUS_INTERNAL_ERROR;
}
@@ -232,7 +235,6 @@
listener->fd = local_name_to_fd(listener->local_name, error);
if (listener->fd < 0) {
- printf("cannot bind '%s': %s\n", listener->local_name, error->c_str());
free(listener->local_name);
free(listener->connect_to);
free(listener);
diff --git a/client/main.cpp b/client/main.cpp
index 73acbb0..e0e7e77 100644
--- a/client/main.cpp
+++ b/client/main.cpp
@@ -154,7 +154,7 @@
std::string error;
std::string local_name = android::base::StringPrintf("tcp:%d", server_port);
if (install_listener(local_name, "*smartsocket*", nullptr, 0, &error)) {
- LOG(FATAL) << "Could not install *smartsocket* listener: " << error;
+ fatal("could not install *smartsocket* listener: %s", error.c_str());
}
// Inform our parent that we are up and running.
diff --git a/sysdeps_win32.cpp b/sysdeps_win32.cpp
index db552a2..80d41a4 100644
--- a/sysdeps_win32.cpp
+++ b/sysdeps_win32.cpp
@@ -554,6 +554,9 @@
// POSIX and socket error codes, so this can only meaningfully map so much.
switch ( err ) {
case 0: errno = 0; break;
+ // Mapping WSAEWOULDBLOCK to EAGAIN is absolutely critical because
+ // non-blocking sockets can cause an error code of WSAEWOULDBLOCK and
+ // callers check specifically for EAGAIN.
case WSAEWOULDBLOCK: errno = EAGAIN; break;
case WSAEINTR: errno = EINTR; break;
case WSAEFAULT: errno = EFAULT; break;
@@ -619,8 +622,12 @@
int result = recv(f->fh_socket, reinterpret_cast<char*>(buf), len, 0);
if (result == SOCKET_ERROR) {
const DWORD err = WSAGetLastError();
- D("recv fd %d failed: %s\n", _fh_to_int(f),
- SystemErrorCodeToString(err).c_str());
+ // WSAEWOULDBLOCK is normal with a non-blocking socket, so don't trace
+ // that to reduce spam and confusion.
+ if (err != WSAEWOULDBLOCK) {
+ D("recv fd %d failed: %s\n", _fh_to_int(f),
+ SystemErrorCodeToString(err).c_str());
+ }
_socket_set_errno(err);
result = -1;
}
@@ -698,14 +705,19 @@
s = socket(AF_INET, type, 0);
if(s == INVALID_SOCKET) {
- *error = SystemErrorCodeToString(WSAGetLastError());
- D("could not create socket: %s\n", error->c_str());
+ *error = android::base::StringPrintf("cannot create socket: %s",
+ SystemErrorCodeToString(WSAGetLastError()).c_str());
+ D("%s\n", error->c_str());
return -1;
}
f->fh_socket = s;
if(connect(s, (struct sockaddr *) &addr, sizeof(addr)) == SOCKET_ERROR) {
- *error = SystemErrorCodeToString(WSAGetLastError());
+ // Save err just in case inet_ntoa() or ntohs() changes the last error.
+ const DWORD err = WSAGetLastError();
+ *error = android::base::StringPrintf("cannot connect to %s:%u: %s",
+ inet_ntoa(addr.sin_addr), ntohs(addr.sin_port),
+ SystemErrorCodeToString(err).c_str());
D("could not connect to %s:%d: %s\n",
type != SOCK_STREAM ? "udp" : "tcp", port, error->c_str());
return -1;
@@ -747,31 +759,40 @@
// IPv4 and IPv6.
s = socket(AF_INET, type, 0);
if (s == INVALID_SOCKET) {
- *error = SystemErrorCodeToString(WSAGetLastError());
- D("could not create socket: %s\n", error->c_str());
+ *error = android::base::StringPrintf("cannot create socket: %s",
+ SystemErrorCodeToString(WSAGetLastError()).c_str());
+ D("%s\n", error->c_str());
return -1;
}
f->fh_socket = s;
+ // Note: SO_REUSEADDR on Windows allows multiple processes to bind to the
+ // same port, so instead use SO_EXCLUSIVEADDRUSE.
n = 1;
if (setsockopt(s, SOL_SOCKET, SO_EXCLUSIVEADDRUSE, (const char*)&n,
sizeof(n)) == SOCKET_ERROR) {
- *error = SystemErrorCodeToString(WSAGetLastError());
- D("setsockopt level %d optname %d failed: %s\n",
- SOL_SOCKET, SO_EXCLUSIVEADDRUSE, error->c_str());
+ *error = android::base::StringPrintf(
+ "cannot set socket option SO_EXCLUSIVEADDRUSE: %s",
+ SystemErrorCodeToString(WSAGetLastError()).c_str());
+ D("%s\n", error->c_str());
return -1;
}
- if(bind(s, (struct sockaddr *) &addr, sizeof(addr)) == SOCKET_ERROR) {
- *error = SystemErrorCodeToString(WSAGetLastError());
+ if (bind(s, (struct sockaddr *) &addr, sizeof(addr)) == SOCKET_ERROR) {
+ // Save err just in case inet_ntoa() or ntohs() changes the last error.
+ const DWORD err = WSAGetLastError();
+ *error = android::base::StringPrintf("cannot bind to %s:%u: %s",
+ inet_ntoa(addr.sin_addr), ntohs(addr.sin_port),
+ SystemErrorCodeToString(err).c_str());
D("could not bind to %s:%d: %s\n",
type != SOCK_STREAM ? "udp" : "tcp", port, error->c_str());
return -1;
}
if (type == SOCK_STREAM) {
if (listen(s, LISTEN_BACKLOG) == SOCKET_ERROR) {
- *error = SystemErrorCodeToString(WSAGetLastError());
+ *error = android::base::StringPrintf("cannot listen on socket: %s",
+ SystemErrorCodeToString(WSAGetLastError()).c_str());
D("could not listen on %s:%d: %s\n",
type != SOCK_STREAM ? "udp" : "tcp", port, error->c_str());
return -1;
@@ -822,9 +843,10 @@
// with GetProcAddress("GetAddrInfoW").
#endif
if (getaddrinfo(host.c_str(), port_str, &hints, &addrinfo_ptr) != 0) {
- *error = SystemErrorCodeToString(WSAGetLastError());
- D("could not resolve host '%s' and port %s: %s\n", host.c_str(),
- port_str, error->c_str());
+ *error = android::base::StringPrintf(
+ "cannot resolve host '%s' and port %s: %s", host.c_str(),
+ port_str, SystemErrorCodeToString(WSAGetLastError()).c_str());
+ D("%s\n", error->c_str());
return -1;
}
std::unique_ptr<struct addrinfo, decltype(freeaddrinfo)*>
@@ -837,8 +859,9 @@
SOCKET s = socket(addrinfo->ai_family, addrinfo->ai_socktype,
addrinfo->ai_protocol);
if(s == INVALID_SOCKET) {
- *error = SystemErrorCodeToString(WSAGetLastError());
- D("could not create socket: %s\n", error->c_str());
+ *error = android::base::StringPrintf("cannot create socket: %s",
+ SystemErrorCodeToString(WSAGetLastError()).c_str());
+ D("%s\n", error->c_str());
return -1;
}
f->fh_socket = s;
@@ -846,7 +869,10 @@
// TODO: Implement timeouts for Windows. Seems like the default in theory
// (according to http://serverfault.com/a/671453) and in practice is 21 sec.
if(connect(s, addrinfo->ai_addr, addrinfo->ai_addrlen) == SOCKET_ERROR) {
- *error = SystemErrorCodeToString(WSAGetLastError());
+ // TODO: Use WSAAddressToString or inet_ntop on address.
+ *error = android::base::StringPrintf("cannot connect to %s:%s: %s",
+ host.c_str(), port_str,
+ SystemErrorCodeToString(WSAGetLastError()).c_str());
D("could not connect to %s:%s:%s: %s\n",
type != SOCK_STREAM ? "udp" : "tcp", host.c_str(), port_str,
error->c_str());