From 381cfa9a8bc262dcd823a8bb6adc189595a2fe7d Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Thu, 23 Jul 2015 17:12:58 -0700 Subject: [PATCH] Report getaddrinfo failures correctly. Also move us off the "convenience" function because you can't get useful error reporting from it. Change-Id: I5fcc6a6d762f5f60906980a7835f01a35045be65 --- adb/adb.h | 6 ++++-- adb/adb_client.cpp | 19 ++++++++++++++----- adb/adb_listeners.cpp | 1 + adb/adb_utils.cpp | 17 +++++++++++++++++ adb/adb_utils.h | 2 ++ adb/console.cpp | 7 ++++--- adb/jdwp_service.cpp | 1 + adb/services.cpp | 16 ++++++++++------ adb/sysdeps.h | 6 ++---- adb/sysdeps_win32.cpp | 30 ++++++++++-------------------- adb/transport_local.cpp | 17 +++++++++-------- include/cutils/sockets.h | 2 +- libcutils/socket_network_client.c | 11 ++++++++--- 13 files changed, 83 insertions(+), 52 deletions(-) diff --git a/adb/adb.h b/adb/adb.h index 357be513f..309b0e9bc 100644 --- a/adb/adb.h +++ b/adb/adb.h @@ -22,6 +22,8 @@ #include +#include + #include "adb_trace.h" #include "fdevent.h" @@ -357,8 +359,8 @@ void put_apacket(apacket *p); void local_init(int port); -int local_connect(int port); -int local_connect_arbitrary_ports(int console_port, int adb_port); +void local_connect(int port); +int local_connect_arbitrary_ports(int console_port, int adb_port, std::string* error); /* usb host/client interface */ void usb_init(); diff --git a/adb/adb_client.cpp b/adb/adb_client.cpp index 75e888d49..418662c4d 100644 --- a/adb/adb_client.cpp +++ b/adb/adb_client.cpp @@ -33,8 +33,10 @@ #include #include +#include #include "adb_io.h" +#include "adb_utils.h" static TransportType __adb_transport = kTransportAny; static const char* __adb_serial = NULL; @@ -152,13 +154,20 @@ int _adb_connect(const std::string& service, std::string* error) { int fd; if (__adb_server_name) { - fd = socket_network_client(__adb_server_name, __adb_server_port, SOCK_STREAM); + std::string reason; + fd = network_connect(__adb_server_name, __adb_server_port, SOCK_STREAM, 0, &reason); + if (fd == -1) { + *error = android::base::StringPrintf("can't connect to %s:%d: %s", + __adb_server_name, __adb_server_port, + reason.c_str()); + return -2; + } } else { fd = socket_loopback_client(__adb_server_port, SOCK_STREAM); - } - if (fd < 0) { - *error = perror_str("cannot connect to daemon"); - return -2; + if (fd == -1) { + *error = perror_str("cannot connect to daemon"); + return -2; + } } if (memcmp(&service[0],"host",4) != 0 && switch_socket_transport(fd, error)) { diff --git a/adb/adb_listeners.cpp b/adb/adb_listeners.cpp index a15c513fe..bb45022cb 100644 --- a/adb/adb_listeners.cpp +++ b/adb/adb_listeners.cpp @@ -20,6 +20,7 @@ #include #include +#include #include "sysdeps.h" #include "transport.h" diff --git a/adb/adb_utils.cpp b/adb/adb_utils.cpp index e2af045b1..3846c2112 100644 --- a/adb/adb_utils.cpp +++ b/adb/adb_utils.cpp @@ -18,6 +18,7 @@ #include "adb_utils.h" +#include #include #include #include @@ -28,6 +29,7 @@ #include #include #include +#include #include "adb_trace.h" #include "sysdeps.h" @@ -158,3 +160,18 @@ bool parse_host_and_port(const std::string& address, << " (" << *canonical_address << ")"; return true; } + +int network_connect(const std::string& host, int port, int type, int timeout, std::string* error) { + int getaddrinfo_error = 0; + int fd = socket_network_client_timeout(host.c_str(), port, type, timeout, &getaddrinfo_error); + if (fd != -1) { + return fd; + } + if (getaddrinfo_error != 0) { + // TODO: not thread safe on Win32. + *error = gai_strerror(getaddrinfo_error); + } else { + *error = strerror(errno); + } + return -1; +} diff --git a/adb/adb_utils.h b/adb/adb_utils.h index e0aa1ba73..d1a3f5f8f 100644 --- a/adb/adb_utils.h +++ b/adb/adb_utils.h @@ -39,4 +39,6 @@ bool parse_host_and_port(const std::string& address, std::string* host, int* port, std::string* error); +int network_connect(const std::string& host, int port, int type, int timeout, std::string* error); + #endif diff --git a/adb/console.cpp b/adb/console.cpp index 07079609c..b7f53458e 100644 --- a/adb/console.cpp +++ b/adb/console.cpp @@ -18,9 +18,10 @@ #include -#include "base/file.h" -#include "base/logging.h" -#include "base/strings.h" +#include +#include +#include +#include #include "adb.h" #include "adb_client.h" diff --git a/adb/jdwp_service.cpp b/adb/jdwp_service.cpp index a2e0f88fd..06e778029 100644 --- a/adb/jdwp_service.cpp +++ b/adb/jdwp_service.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include diff --git a/adb/services.cpp b/adb/services.cpp index 2e3ad98a5..82efb1c04 100644 --- a/adb/services.cpp +++ b/adb/services.cpp @@ -39,6 +39,7 @@ #include #include #include +#include #if !ADB_HOST #include "cutils/android_reboot.h" @@ -436,7 +437,8 @@ int service_to_fd(const char *name) disable_tcp_nagle(ret); } else { #if ADB_HOST - ret = socket_network_client(name + 1, port, SOCK_STREAM); + std::string error; + ret = network_connect(name + 1, port, SOCK_STREAM, 0, &error); #else return -1; #endif @@ -555,10 +557,11 @@ static void connect_device(const std::string& address, std::string* response) { return; } - int fd = socket_network_client_timeout(host.c_str(), port, SOCK_STREAM, 10); + std::string error; + int fd = network_connect(host.c_str(), port, SOCK_STREAM, 10, &error); if (fd == -1) { *response = android::base::StringPrintf("unable to connect to %s: %s", - serial.c_str(), strerror(errno)); + serial.c_str(), error.c_str()); return; } @@ -612,12 +615,13 @@ void connect_emulator(const std::string& port_spec, std::string* response) { } // Preconditions met, try to connect to the emulator. - if (!local_connect_arbitrary_ports(console_port, adb_port)) { + std::string error; + if (!local_connect_arbitrary_ports(console_port, adb_port, &error)) { *response = android::base::StringPrintf("Connected to emulator on ports %d,%d", console_port, adb_port); } else { - *response = android::base::StringPrintf("Could not connect to emulator on ports %d,%d", - console_port, adb_port); + *response = android::base::StringPrintf("Could not connect to emulator on ports %d,%d: %s", + console_port, adb_port, error.c_str()); } } diff --git a/adb/sysdeps.h b/adb/sysdeps.h index 5efdab5ba..c051eb324 100644 --- a/adb/sysdeps.h +++ b/adb/sysdeps.h @@ -24,6 +24,8 @@ # undef _WIN32 #endif +#include + /* * TEMP_FAILURE_RETRY is defined by some, but not all, versions of * . (Alas, it is not as standard as we'd hoped!) So, if it's @@ -187,9 +189,6 @@ extern void* load_file(const char* pathname, unsigned* psize); /* normally provided by */ extern int socket_loopback_client(int port, int type); -extern int socket_network_client(const char *host, int port, int type); -extern int socket_network_client_timeout(const char *host, int port, int type, - int timeout); extern int socket_loopback_server(int port, int type); extern int socket_inaddr_any_server(int port, int type); @@ -274,7 +273,6 @@ static __inline__ int adb_is_absolute_host_path( const char* path ) #else /* !_WIN32 a.k.a. Unix */ #include "fdevent.h" -#include #include #include #include diff --git a/adb/sysdeps_win32.cpp b/adb/sysdeps_win32.cpp index bdc602775..91e14953f 100644 --- a/adb/sysdeps_win32.cpp +++ b/adb/sysdeps_win32.cpp @@ -668,55 +668,45 @@ int socket_loopback_server(int port, int type) } -int socket_network_client(const char *host, int port, int type) -{ +int socket_network_client_timeout(const char *host, int port, int type, int timeout, + std::string* error) { FH f = _fh_alloc( &_fh_socket_class ); - struct hostent *hp; - struct sockaddr_in addr; - SOCKET s; + if (!f) return -1; - if (!f) - return -1; + if (!_winsock_init) _init_winsock(); - if (!_winsock_init) - _init_winsock(); - - hp = gethostbyname(host); + hostent* hp = gethostbyname(host); if(hp == 0) { _fh_close(f); return -1; } + sockaddr_in addr; memset(&addr, 0, sizeof(addr)); addr.sin_family = hp->h_addrtype; addr.sin_port = htons(port); memcpy(&addr.sin_addr, hp->h_addr, hp->h_length); - s = socket(hp->h_addrtype, type, 0); + SOCKET s = socket(hp->h_addrtype, type, 0); if(s == INVALID_SOCKET) { _fh_close(f); return -1; } f->fh_socket = s; + // TODO: implement timeouts for Windows. + if(connect(s, (struct sockaddr *) &addr, sizeof(addr)) < 0) { _fh_close(f); return -1; } snprintf( f->name, sizeof(f->name), "%d(net-client:%s%d)", _fh_to_int(f), type != SOCK_STREAM ? "udp:" : "", port ); - D( "socket_network_client: host '%s' port %d type %s => fd %d\n", host, port, type != SOCK_STREAM ? "udp" : "tcp", _fh_to_int(f) ); + D( "socket_network_client_timeout: host '%s' port %d type %s => fd %d\n", host, port, type != SOCK_STREAM ? "udp" : "tcp", _fh_to_int(f) ); return _fh_to_int(f); } -int socket_network_client_timeout(const char *host, int port, int type, int timeout) -{ - // TODO: implement timeouts for Windows. - return socket_network_client(host, port, type); -} - - int socket_inaddr_any_server(int port, int type) { FH f = _fh_alloc( &_fh_socket_class ); diff --git a/adb/transport_local.cpp b/adb/transport_local.cpp index 1adc69e13..0dc9581dd 100644 --- a/adb/transport_local.cpp +++ b/adb/transport_local.cpp @@ -26,6 +26,7 @@ #include #include +#include #if !ADB_HOST #include "cutils/properties.h" @@ -33,6 +34,7 @@ #include "adb.h" #include "adb_io.h" +#include "adb_utils.h" #if ADB_HOST /* we keep a list of opened transports. The atransport struct knows to which @@ -83,19 +85,18 @@ static int remote_write(apacket *p, atransport *t) return 0; } - -int local_connect(int port) { - return local_connect_arbitrary_ports(port-1, port); +void local_connect(int port) { + std::string dummy; + local_connect_arbitrary_ports(port-1, port, &dummy); } -int local_connect_arbitrary_ports(int console_port, int adb_port) -{ - int fd = -1; +int local_connect_arbitrary_ports(int console_port, int adb_port, std::string* error) { + int fd = -1; #if ADB_HOST const char *host = getenv("ADBHOST"); if (host) { - fd = socket_network_client(host, adb_port, SOCK_STREAM); + fd = network_connect(host, adb_port, SOCK_STREAM, 0, error); } #endif if (fd < 0) { @@ -126,7 +127,7 @@ static void *client_socket_thread(void *x) /* this is only done when ADB starts up. later, each new emulator */ /* will send a message to ADB to indicate that is is starting up */ for ( ; count > 0; count--, port += 2 ) { - (void) local_connect(port); + local_connect(port); } #endif return 0; diff --git a/include/cutils/sockets.h b/include/cutils/sockets.h index f8076ca45..07d1351a9 100644 --- a/include/cutils/sockets.h +++ b/include/cutils/sockets.h @@ -77,7 +77,7 @@ static inline int android_get_control_socket(const char *name) extern int socket_loopback_client(int port, int type); extern int socket_network_client(const char *host, int port, int type); extern int socket_network_client_timeout(const char *host, int port, int type, - int timeout); + int timeout, int* getaddrinfo_error); extern int socket_loopback_server(int port, int type); extern int socket_local_server(const char *name, int namespaceId, int type); extern int socket_local_server_bind(int s, const char *name, int namespaceId); diff --git a/libcutils/socket_network_client.c b/libcutils/socket_network_client.c index 261025425..3610f1b62 100644 --- a/libcutils/socket_network_client.c +++ b/libcutils/socket_network_client.c @@ -41,7 +41,10 @@ static int toggle_O_NONBLOCK(int s) { // Connect to the given host and port. // 'timeout' is in seconds (0 for no timeout). // Returns a file descriptor or -1 on error. -int socket_network_client_timeout(const char* host, int port, int type, int timeout) { +// On error, check *getaddrinfo_error (for use with gai_strerror) first; +// if that's 0, use errno instead. +int socket_network_client_timeout(const char* host, int port, int type, int timeout, + int* getaddrinfo_error) { struct addrinfo hints; memset(&hints, 0, sizeof(hints)); hints.ai_family = AF_UNSPEC; @@ -51,7 +54,8 @@ int socket_network_client_timeout(const char* host, int port, int type, int time snprintf(port_str, sizeof(port_str), "%d", port); struct addrinfo* addrs; - if (getaddrinfo(host, port_str, &hints, &addrs) != 0) { + *getaddrinfo_error = getaddrinfo(host, port_str, &hints, &addrs); + if (getaddrinfo_error != 0) { return -1; } @@ -116,5 +120,6 @@ int socket_network_client_timeout(const char* host, int port, int type, int time } int socket_network_client(const char* host, int port, int type) { - return socket_network_client_timeout(host, port, type, 0); + int getaddrinfo_error; + return socket_network_client_timeout(host, port, type, 0, &getaddrinfo_error); }