From e2d3677cc2239a8530d33e3c5e24e65fff096801 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Tue, 23 Jun 2015 13:00:32 -0700 Subject: [PATCH] Improve the "device '(null)' not found" error. Now we'll say "no devices found" if you haven't set ANDROID_SERIAL and there's no device connected to default to. Also clean up the relevant code a little. Change-Id: Id254929629ce0888628d5ba8e67cd996ffbf9c8a --- adb/adb.cpp | 75 ++++++++++++++++++++--------------------------- adb/services.cpp | 2 +- adb/transport.cpp | 32 ++++++++++---------- 3 files changed, 48 insertions(+), 61 deletions(-) diff --git a/adb/adb.cpp b/adb/adb.cpp index 6a0540c7e..2e59634fd 100644 --- a/adb/adb.cpp +++ b/adb/adb.cpp @@ -835,6 +835,14 @@ int handle_forward_request(const char* service, TransportType type, const char* return 0; } +#if ADB_HOST +static int SendOkay(int fd, const std::string& s) { + SendOkay(fd); + SendProtocolString(fd, s); + return 0; +} +#endif + int handle_host_request(const char* service, TransportType type, const char* serial, int reply_fd, asocket* s) { if (strcmp(service, "kill") == 0) { @@ -845,7 +853,6 @@ int handle_host_request(const char* service, TransportType type, } #if ADB_HOST - atransport *transport = NULL; // "transport:" is used for switching transport with a specified serial number // "transport-usb:" is used for switching transport to the only USB transport // "transport-local:" is used for switching transport to the only local transport @@ -864,11 +871,10 @@ int handle_host_request(const char* service, TransportType type, serial = service; } - std::string error_msg = "unknown failure"; - transport = acquire_one_transport(kCsAny, type, serial, &error_msg); - - if (transport) { - s->transport = transport; + std::string error_msg; + atransport* t = acquire_one_transport(kCsAny, type, serial, &error_msg); + if (t != nullptr) { + s->transport = t; SendOkay(reply_fd); } else { SendFail(reply_fd, error_msg); @@ -883,9 +889,7 @@ int handle_host_request(const char* service, TransportType type, D("Getting device list...\n"); std::string device_list = list_transports(long_listing); D("Sending device list...\n"); - SendOkay(reply_fd); - SendProtocolString(reply_fd, device_list); - return 0; + return SendOkay(reply_fd, device_list); } return 1; } @@ -905,8 +909,7 @@ int handle_host_request(const char* service, TransportType type, snprintf(hostbuf, sizeof(hostbuf) - 1, "%s:5555", serial); serial = hostbuf; } - atransport *t = find_transport(serial); - + atransport* t = find_transport(serial); if (t) { unregister_transport(t); } else { @@ -914,52 +917,38 @@ int handle_host_request(const char* service, TransportType type, } } - SendOkay(reply_fd); - SendProtocolString(reply_fd, buffer); - return 0; + return SendOkay(reply_fd, buffer); } // returns our value for ADB_SERVER_VERSION if (!strcmp(service, "version")) { - SendOkay(reply_fd); - SendProtocolString(reply_fd, android::base::StringPrintf("%04x", ADB_SERVER_VERSION)); - return 0; + return SendOkay(reply_fd, android::base::StringPrintf("%04x", ADB_SERVER_VERSION)); } - if(!strncmp(service,"get-serialno",strlen("get-serialno"))) { - const char *out = "unknown"; - transport = acquire_one_transport(kCsAny, type, serial, NULL); - if (transport && transport->serial) { - out = transport->serial; - } - SendOkay(reply_fd); - SendProtocolString(reply_fd, out); - return 0; + // These always report "unknown" rather than the actual error, for scripts. + if (!strcmp(service, "get-serialno")) { + std::string ignored; + atransport* t = acquire_one_transport(kCsAny, type, serial, &ignored); + return SendOkay(reply_fd, (t && t->serial) ? t->serial : "unknown"); } - if(!strncmp(service,"get-devpath",strlen("get-devpath"))) { - const char *out = "unknown"; - transport = acquire_one_transport(kCsAny, type, serial, NULL); - if (transport && transport->devpath) { - out = transport->devpath; - } - SendOkay(reply_fd); - SendProtocolString(reply_fd, out); - return 0; + if (!strcmp(service, "get-devpath")) { + std::string ignored; + atransport* t = acquire_one_transport(kCsAny, type, serial, &ignored); + return SendOkay(reply_fd, (t && t->devpath) ? t->devpath : "unknown"); } + if (!strcmp(service, "get-state")) { + std::string ignored; + atransport* t = acquire_one_transport(kCsAny, type, serial, &ignored); + return SendOkay(reply_fd, t ? t->connection_state_name() : "unknown"); + } + // indicates a new emulator instance has started - if (!strncmp(service,"emulator:",9)) { + if (!strncmp(service, "emulator:", 9)) { int port = atoi(service+9); local_connect(port); /* we don't even need to send a reply */ return 0; } - - if(!strncmp(service,"get-state",strlen("get-state"))) { - transport = acquire_one_transport(kCsAny, type, serial, NULL); - SendOkay(reply_fd); - SendProtocolString(reply_fd, transport->connection_state_name()); - return 0; - } #endif // ADB_HOST int ret = handle_forward_request(service, type, serial, reply_fd); diff --git a/adb/services.cpp b/adb/services.cpp index 2fd75abe1..227f22acc 100644 --- a/adb/services.cpp +++ b/adb/services.cpp @@ -528,7 +528,7 @@ static void wait_for_state(int fd, void* cookie) std::string error_msg = "unknown error"; atransport* t = acquire_one_transport(sinfo->state, sinfo->transport_type, sinfo->serial, &error_msg); - if (t != 0) { + if (t != nullptr) { SendOkay(fd); } else { SendFail(fd, error_msg); diff --git a/adb/transport.cpp b/adb/transport.cpp index 379c702e8..274449bea 100644 --- a/adb/transport.cpp +++ b/adb/transport.cpp @@ -731,12 +731,12 @@ atransport* acquire_one_transport(ConnectionState state, TransportType type, int ambiguous = 0; retry: - if (error_out) *error_out = android::base::StringPrintf("device '%s' not found", serial); + *error_out = serial ? android::base::StringPrintf("device '%s' not found", serial) : "no devices found"; adb_mutex_lock(&transport_lock); for (auto t : transport_list) { if (t->connection_state == kCsNoPerm) { - if (error_out) *error_out = "insufficient permissions for device"; + *error_out = "insufficient permissions for device"; continue; } @@ -748,7 +748,7 @@ retry: qual_match(serial, "model:", t->model, true) || qual_match(serial, "device:", t->device, false)) { if (result) { - if (error_out) *error_out = "more than one device"; + *error_out = "more than one device"; ambiguous = 1; result = NULL; break; @@ -758,7 +758,7 @@ retry: } else { if (type == kTransportUsb && t->type == kTransportUsb) { if (result) { - if (error_out) *error_out = "more than one device"; + *error_out = "more than one device"; ambiguous = 1; result = NULL; break; @@ -766,7 +766,7 @@ retry: result = t; } else if (type == kTransportLocal && t->type == kTransportLocal) { if (result) { - if (error_out) *error_out = "more than one emulator"; + *error_out = "more than one emulator"; ambiguous = 1; result = NULL; break; @@ -774,7 +774,7 @@ retry: result = t; } else if (type == kTransportAny) { if (result) { - if (error_out) *error_out = "more than one device/emulator"; + *error_out = "more than one device/emulator"; ambiguous = 1; result = NULL; break; @@ -787,33 +787,31 @@ retry: if (result) { if (result->connection_state == kCsUnauthorized) { - if (error_out) { - *error_out = "device unauthorized.\n"; - char* ADB_VENDOR_KEYS = getenv("ADB_VENDOR_KEYS"); - *error_out += "This adbd's $ADB_VENDOR_KEYS is "; - *error_out += ADB_VENDOR_KEYS ? ADB_VENDOR_KEYS : "not set"; - *error_out += "; try 'adb kill-server' if that seems wrong.\n"; - *error_out += "Otherwise check for a confirmation dialog on your device."; - } + *error_out = "device unauthorized.\n"; + char* ADB_VENDOR_KEYS = getenv("ADB_VENDOR_KEYS"); + *error_out += "This adbd's $ADB_VENDOR_KEYS is "; + *error_out += ADB_VENDOR_KEYS ? ADB_VENDOR_KEYS : "not set"; + *error_out += "; try 'adb kill-server' if that seems wrong.\n"; + *error_out += "Otherwise check for a confirmation dialog on your device."; result = NULL; } /* offline devices are ignored -- they are either being born or dying */ if (result && result->connection_state == kCsOffline) { - if (error_out) *error_out = "device offline"; + *error_out = "device offline"; result = NULL; } /* check for required connection state */ if (result && state != kCsAny && result->connection_state != state) { - if (error_out) *error_out = "invalid device state"; + *error_out = "invalid device state"; result = NULL; } } if (result) { /* found one that we can take */ - if (error_out) *error_out = "success"; + *error_out = "success"; } else if (state != kCsAny && (serial || !ambiguous)) { adb_sleep_ms(1000); goto retry;