Fix adb -d/-e error reporting.
If -d/-e fail, get-serialno and friends will now report an error
and return a failure status code on exit.
Also fix the behavior of -d/-e with $ANDROID_SERIAL --- -d/-e
should override $ANDROID_SERIAL, not the other way round.
I'm deleting my own comment here about always returning "unknown"
for scripts. I can't find any evidence that there are scripts
relying on that, so I think my comment meant "I fear that there
are scripts doing so".
Bug: http://b/24403699
Change-Id: Ie13a751f1137abcfe0cc6c46a0630ba5e02db676
diff --git a/adb.cpp b/adb.cpp
index 1c1683e..3dcf282 100644
--- a/adb.cpp
+++ b/adb.cpp
@@ -904,7 +904,7 @@
}
std::string error_msg;
- atransport* transport = acquire_one_transport(kCsAny, type, serial, &error_msg);
+ atransport* transport = acquire_one_transport(type, serial, nullptr, &error_msg);
if (!transport) {
SendFail(reply_fd, error_msg);
return 1;
@@ -990,13 +990,13 @@
serial = service;
}
- std::string error_msg;
- atransport* t = acquire_one_transport(kCsAny, type, serial, &error_msg);
+ std::string error;
+ atransport* t = acquire_one_transport(type, serial, nullptr, &error);
if (t != nullptr) {
s->transport = t;
SendOkay(reply_fd);
} else {
- SendFail(reply_fd, error_msg);
+ SendFail(reply_fd, error);
}
return 1;
}
@@ -1014,12 +1014,12 @@
}
if (!strcmp(service, "features")) {
- std::string error_msg;
- atransport* t = acquire_one_transport(kCsAny, type, serial, &error_msg);
+ std::string error;
+ atransport* t = acquire_one_transport(type, serial, nullptr, &error);
if (t != nullptr) {
SendOkay(reply_fd, FeatureSetToString(t->features()));
} else {
- SendFail(reply_fd, error_msg);
+ SendFail(reply_fd, error);
}
return 0;
}
@@ -1049,29 +1049,41 @@
return SendOkay(reply_fd, android::base::StringPrintf("disconnected %s", address.c_str()));
}
- // returns our value for ADB_SERVER_VERSION
+ // Returns our value for ADB_SERVER_VERSION.
if (!strcmp(service, "version")) {
return SendOkay(reply_fd, android::base::StringPrintf("%04x", ADB_SERVER_VERSION));
}
// 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");
+ std::string error;
+ atransport* t = acquire_one_transport(type, serial, nullptr, &error);
+ if (t) {
+ return SendOkay(reply_fd, t->serial ? t->serial : "unknown");
+ } else {
+ return SendFail(reply_fd, error);
+ }
}
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");
+ std::string error;
+ atransport* t = acquire_one_transport(type, serial, nullptr, &error);
+ if (t) {
+ return SendOkay(reply_fd, t->devpath ? t->devpath : "unknown");
+ } else {
+ return SendFail(reply_fd, error);
+ }
}
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");
+ std::string error;
+ atransport* t = acquire_one_transport(type, serial, nullptr, &error);
+ if (t) {
+ return SendOkay(reply_fd, t->connection_state_name());
+ } else {
+ return SendFail(reply_fd, error);
+ }
}
- // indicates a new emulator instance has started
+ // Indicates a new emulator instance has started.
if (!strncmp(service, "emulator:", 9)) {
int port = atoi(service+9);
local_connect(port);
diff --git a/adb_client.cpp b/adb_client.cpp
index 984910d..fa8ce9e 100644
--- a/adb_client.cpp
+++ b/adb_client.cpp
@@ -209,9 +209,8 @@
adb_close(fd);
if (sscanf(&version_string[0], "%04x", &version) != 1) {
- *error = android::base::StringPrintf(
- "cannot parse version string: %s",
- version_string.c_str());
+ *error = android::base::StringPrintf("cannot parse version string: %s",
+ version_string.c_str());
return -1;
}
} else {
diff --git a/commandline.cpp b/commandline.cpp
index 5e5ca7f..0531cf9 100644
--- a/commandline.cpp
+++ b/commandline.cpp
@@ -1093,8 +1093,6 @@
}
// TODO: also try TARGET_PRODUCT/TARGET_DEVICE as a hint
- const char* serial = getenv("ANDROID_SERIAL");
-
/* Validate and assign the server port */
const char* server_port_str = getenv("ANDROID_ADB_SERVER_PORT");
int server_port = DEFAULT_ADB_PORT;
@@ -1108,7 +1106,9 @@
}
}
- /* modifiers and flags */
+ // We need to check for -d and -e before we look at $ANDROID_SERIAL.
+ const char* serial = nullptr;
+
while (argc > 0) {
if (!strcmp(argv[0],"server")) {
is_server = 1;
@@ -1199,6 +1199,11 @@
argv++;
}
+ // If none of -d, -e, or -s were specified, try $ANDROID_SERIAL.
+ if (transport_type == kTransportAny && serial == nullptr) {
+ serial = getenv("ANDROID_SERIAL");
+ }
+
adb_set_transport(transport_type, serial);
adb_set_tcp_specifics(server_port);
diff --git a/services.cpp b/services.cpp
index e832b1e..e24b470 100644
--- a/services.cpp
+++ b/services.cpp
@@ -363,23 +363,31 @@
ConnectionState state;
};
-static void wait_for_state(int fd, void* cookie)
-{
+static void wait_for_state(int fd, void* cookie) {
state_info* sinfo = reinterpret_cast<state_info*>(cookie);
D("wait_for_state %d", sinfo->state);
- std::string error_msg = "unknown error";
- atransport* t = acquire_one_transport(sinfo->state, sinfo->transport_type, sinfo->serial,
- &error_msg);
- if (t != nullptr) {
- SendOkay(fd);
- } else {
- SendFail(fd, error_msg);
+ while (true) {
+ bool is_ambiguous = false;
+ std::string error = "unknown error";
+ atransport* t = acquire_one_transport(sinfo->transport_type, sinfo->serial,
+ &is_ambiguous, &error);
+ if (t != nullptr && t->connection_state == sinfo->state) {
+ SendOkay(fd);
+ break;
+ } else if (!is_ambiguous) {
+ adb_sleep_ms(1000);
+ // Try again...
+ } else {
+ SendFail(fd, error);
+ break;
+ }
}
- if (sinfo->serial)
+ if (sinfo->serial) {
free(sinfo->serial);
+ }
free(sinfo);
adb_close(fd);
D("wait_for_state is done");
diff --git a/sockets.cpp b/sockets.cpp
index 8562496..f8c2f64 100644
--- a/sockets.cpp
+++ b/sockets.cpp
@@ -671,8 +671,8 @@
{
unsigned len;
#if ADB_HOST
- char *service = NULL;
- char* serial = NULL;
+ char *service = nullptr;
+ char* serial = nullptr;
TransportType type = kTransportAny;
#endif
@@ -739,7 +739,7 @@
type = kTransportAny;
service += strlen("host:");
} else {
- service = NULL;
+ service = nullptr;
}
if (service) {
@@ -782,7 +782,7 @@
SendOkay(s->peer->fd);
s->peer->ready = local_socket_ready;
- s->peer->shutdown = NULL;
+ s->peer->shutdown = nullptr;
s->peer->close = local_socket_close;
s->peer->peer = s2;
s2->peer = s->peer;
@@ -795,12 +795,10 @@
return 0;
}
#else /* !ADB_HOST */
- if (s->transport == NULL) {
+ if (s->transport == nullptr) {
std::string error_msg = "unknown failure";
- s->transport =
- acquire_one_transport(kCsAny, kTransportAny, NULL, &error_msg);
-
- if (s->transport == NULL) {
+ s->transport = acquire_one_transport(kTransportAny, nullptr, nullptr, &error_msg);
+ if (s->transport == nullptr) {
SendFail(s->peer->fd, error_msg);
goto fail;
}
@@ -822,7 +820,7 @@
** tear down
*/
s->peer->ready = local_socket_ready_notify;
- s->peer->shutdown = NULL;
+ s->peer->shutdown = nullptr;
s->peer->close = local_socket_close_notify;
s->peer->peer = 0;
/* give him our transport and upref it */
diff --git a/transport.cpp b/transport.cpp
index 501a39a..f8cf63a 100644
--- a/transport.cpp
+++ b/transport.cpp
@@ -653,22 +653,28 @@
return !*to_test;
}
-atransport* acquire_one_transport(ConnectionState state, TransportType type,
- const char* serial, std::string* error_out) {
- atransport *result = NULL;
- int ambiguous = 0;
+atransport* acquire_one_transport(TransportType type, const char* serial,
+ bool* is_ambiguous, std::string* error_out) {
+ atransport* result = nullptr;
-retry:
- *error_out = serial ? android::base::StringPrintf("device '%s' not found", serial) : "no devices found";
+ if (serial) {
+ *error_out = android::base::StringPrintf("device '%s' not found", serial);
+ } else if (type == kTransportLocal) {
+ *error_out = "no emulators found";
+ } else if (type == kTransportAny) {
+ *error_out = "no devices/emulators found";
+ } else {
+ *error_out = "no devices found";
+ }
adb_mutex_lock(&transport_lock);
- for (auto t : transport_list) {
+ for (const auto& t : transport_list) {
if (t->connection_state == kCsNoPerm) {
*error_out = "insufficient permissions for device";
continue;
}
- /* check for matching serial number */
+ // Check for matching serial number.
if (serial) {
if ((t->serial && !strcmp(serial, t->serial)) ||
(t->devpath && !strcmp(serial, t->devpath)) ||
@@ -677,8 +683,8 @@
qual_match(serial, "device:", t->device, false)) {
if (result) {
*error_out = "more than one device";
- ambiguous = 1;
- result = NULL;
+ if (is_ambiguous) *is_ambiguous = true;
+ result = nullptr;
break;
}
result = t;
@@ -687,24 +693,24 @@
if (type == kTransportUsb && t->type == kTransportUsb) {
if (result) {
*error_out = "more than one device";
- ambiguous = 1;
- result = NULL;
+ if (is_ambiguous) *is_ambiguous = true;
+ result = nullptr;
break;
}
result = t;
} else if (type == kTransportLocal && t->type == kTransportLocal) {
if (result) {
*error_out = "more than one emulator";
- ambiguous = 1;
- result = NULL;
+ if (is_ambiguous) *is_ambiguous = true;
+ result = nullptr;
break;
}
result = t;
} else if (type == kTransportAny) {
if (result) {
*error_out = "more than one device/emulator";
- ambiguous = 1;
- result = NULL;
+ if (is_ambiguous) *is_ambiguous = true;
+ result = nullptr;
break;
}
result = t;
@@ -713,37 +719,26 @@
}
adb_mutex_unlock(&transport_lock);
- if (result) {
- if (result->connection_state == kCsUnauthorized) {
- *error_out = "device unauthorized.\n";
- char* ADB_VENDOR_KEYS = getenv("ADB_VENDOR_KEYS");
- *error_out += "This adb server's $ADB_VENDOR_KEYS is ";
- *error_out += ADB_VENDOR_KEYS ? ADB_VENDOR_KEYS : "not set";
- *error_out += "\n";
- *error_out += "Try 'adb kill-server' if that seems wrong.\n";
- *error_out += "Otherwise check for a confirmation dialog on your device.";
- result = NULL;
- }
+ // Don't return unauthorized devices; the caller can't do anything with them.
+ if (result && result->connection_state == kCsUnauthorized) {
+ *error_out = "device unauthorized.\n";
+ char* ADB_VENDOR_KEYS = getenv("ADB_VENDOR_KEYS");
+ *error_out += "This adb server's $ADB_VENDOR_KEYS is ";
+ *error_out += ADB_VENDOR_KEYS ? ADB_VENDOR_KEYS : "not set";
+ *error_out += "\n";
+ *error_out += "Try 'adb kill-server' if that seems wrong.\n";
+ *error_out += "Otherwise check for a confirmation dialog on your device.";
+ result = nullptr;
+ }
- /* offline devices are ignored -- they are either being born or dying */
- if (result && result->connection_state == kCsOffline) {
- *error_out = "device offline";
- result = NULL;
- }
-
- /* check for required connection state */
- if (result && state != kCsAny && result->connection_state != state) {
- *error_out = "invalid device state";
- result = NULL;
- }
+ // Don't return offline devices; the caller can't do anything with them.
+ if (result && result->connection_state == kCsOffline) {
+ *error_out = "device offline";
+ result = nullptr;
}
if (result) {
- /* found one that we can take */
*error_out = "success";
- } else if (state != kCsAny && (serial || !ambiguous)) {
- adb_sleep_ms(1000);
- goto retry;
}
return result;
diff --git a/transport.h b/transport.h
index dfe8875..f41a8d4 100644
--- a/transport.h
+++ b/transport.h
@@ -122,12 +122,13 @@
/*
* Obtain a transport from the available transports.
- * If state is != kCsAny, only transports in that state are considered.
- * If serial is non-NULL then only the device with that serial will be chosen.
- * If no suitable transport is found, error is set.
+ * If serial is non-null then only the device with that serial will be chosen.
+ * If multiple devices/emulators would match, *is_ambiguous (if non-null)
+ * is set to true and nullptr returned.
+ * If no suitable transport is found, error is set and nullptr returned.
*/
-atransport* acquire_one_transport(ConnectionState state, TransportType type,
- const char* serial, std::string* error_out);
+atransport* acquire_one_transport(TransportType type, const char* serial,
+ bool* is_ambiguous, std::string* error_out);
void kick_transport(atransport* t);
void update_transports(void);