adb: clean up handle_host_request.
Previously, we were returning the result of SendOkay/SendFail in a few
places after handling a host request, which is incorrect for two
reasons. First, the return type of SendOkay/SendFail is bool, and
handle_host_request was expected to return 0 on success. Second, we
don't care if the SendOkay fails; if we got to that point, we're done
with the request, regardless of whether we succeeded to report our
result. The result of this was a spurious failure result reported after
the initial result, which was ignored by the adb client.
Test: manually straced adb server
Test: python test_adb.py
Test: python test_device.py
Change-Id: I7d45ba527e1faccbbae5b15e7a0d1557b0a84858
diff --git a/adb/adb.cpp b/adb/adb.cpp
index a8fe736..8e028f4 100644
--- a/adb/adb.cpp
+++ b/adb/adb.cpp
@@ -1044,7 +1044,7 @@
return 0;
}
-int handle_host_request(const char* service, TransportType type, const char* serial,
+bool handle_host_request(const char* service, TransportType type, const char* serial,
TransportId transport_id, int reply_fd, asocket* s) {
if (strcmp(service, "kill") == 0) {
fprintf(stderr, "adb server killed by remote request\n");
@@ -1070,7 +1070,7 @@
transport_id = strtoll(service, const_cast<char**>(&service), 10);
if (*service != '\0') {
SendFail(reply_fd, "invalid transport id");
- return 1;
+ return true;
}
} else if (!strncmp(service, "transport-usb", strlen("transport-usb"))) {
type = kTransportUsb;
@@ -1088,10 +1088,13 @@
if (t != nullptr) {
s->transport = t;
SendOkay(reply_fd);
+
+ // We succesfully handled the device selection, but there's another request coming.
+ return false;
} else {
SendFail(reply_fd, error);
+ return true;
}
- return 1;
}
// return a list of all connected devices
@@ -1101,9 +1104,9 @@
D("Getting device list...");
std::string device_list = list_transports(long_listing);
D("Sending device list...");
- return SendOkay(reply_fd, device_list);
+ SendOkay(reply_fd, device_list);
}
- return 1;
+ return true;
}
if (!strcmp(service, "reconnect-offline")) {
@@ -1119,7 +1122,7 @@
response.resize(response.size() - 1);
}
SendOkay(reply_fd, response);
- return 0;
+ return true;
}
if (!strcmp(service, "features")) {
@@ -1130,7 +1133,7 @@
} else {
SendFail(reply_fd, error);
}
- return 0;
+ return true;
}
if (!strcmp(service, "host-features")) {
@@ -1141,7 +1144,7 @@
}
features.insert(kFeaturePushSync);
SendOkay(reply_fd, FeatureSetToString(features));
- return 0;
+ return true;
}
// remove TCP transport
@@ -1149,7 +1152,8 @@
const std::string address(service + 11);
if (address.empty()) {
kick_all_tcp_devices();
- return SendOkay(reply_fd, "disconnected everything");
+ SendOkay(reply_fd, "disconnected everything");
+ return true;
}
std::string serial;
@@ -1157,21 +1161,24 @@
int port = DEFAULT_ADB_LOCAL_TRANSPORT_PORT;
std::string error;
if (!android::base::ParseNetAddress(address, &host, &port, &serial, &error)) {
- return SendFail(reply_fd, android::base::StringPrintf("couldn't parse '%s': %s",
- address.c_str(), error.c_str()));
+ SendFail(reply_fd, android::base::StringPrintf("couldn't parse '%s': %s",
+ address.c_str(), error.c_str()));
+ return true;
}
atransport* t = find_transport(serial.c_str());
if (t == nullptr) {
- return SendFail(reply_fd, android::base::StringPrintf("no such device '%s'",
- serial.c_str()));
+ SendFail(reply_fd, android::base::StringPrintf("no such device '%s'", serial.c_str()));
+ return true;
}
kick_transport(t);
- return SendOkay(reply_fd, android::base::StringPrintf("disconnected %s", address.c_str()));
+ SendOkay(reply_fd, android::base::StringPrintf("disconnected %s", address.c_str()));
+ return true;
}
// Returns our value for ADB_SERVER_VERSION.
if (!strcmp(service, "version")) {
- return SendOkay(reply_fd, android::base::StringPrintf("%04x", ADB_SERVER_VERSION));
+ SendOkay(reply_fd, android::base::StringPrintf("%04x", ADB_SERVER_VERSION));
+ return true;
}
// These always report "unknown" rather than the actual error, for scripts.
@@ -1179,28 +1186,31 @@
std::string error;
atransport* t = acquire_one_transport(type, serial, transport_id, nullptr, &error);
if (t) {
- return SendOkay(reply_fd, !t->serial.empty() ? t->serial : "unknown");
+ SendOkay(reply_fd, !t->serial.empty() ? t->serial : "unknown");
} else {
- return SendFail(reply_fd, error);
+ SendFail(reply_fd, error);
}
+ return true;
}
if (!strcmp(service, "get-devpath")) {
std::string error;
atransport* t = acquire_one_transport(type, serial, transport_id, nullptr, &error);
if (t) {
- return SendOkay(reply_fd, !t->devpath.empty() ? t->devpath : "unknown");
+ SendOkay(reply_fd, !t->devpath.empty() ? t->devpath : "unknown");
} else {
- return SendFail(reply_fd, error);
+ SendFail(reply_fd, error);
}
+ return true;
}
if (!strcmp(service, "get-state")) {
std::string error;
atransport* t = acquire_one_transport(type, serial, transport_id, nullptr, &error);
if (t) {
- return SendOkay(reply_fd, t->connection_state_name());
+ SendOkay(reply_fd, t->connection_state_name());
} else {
- return SendFail(reply_fd, error);
+ SendFail(reply_fd, error);
}
+ return true;
}
// Indicates a new emulator instance has started.
@@ -1208,7 +1218,7 @@
int port = atoi(service+9);
local_connect(port);
/* we don't even need to send a reply */
- return 0;
+ return true;
}
if (!strcmp(service, "reconnect")) {
@@ -1219,7 +1229,8 @@
response =
"reconnecting " + t->serial_name() + " [" + t->connection_state_name() + "]\n";
}
- return SendOkay(reply_fd, response);
+ SendOkay(reply_fd, response);
+ return true;
}
if (handle_forward_request(service,
@@ -1228,10 +1239,10 @@
error);
},
reply_fd)) {
- return 0;
+ return true;
}
- return -1;
+ return false;
}
static auto& init_mutex = *new std::mutex();
diff --git a/adb/adb.h b/adb/adb.h
index e6af780..f434e2d 100644
--- a/adb/adb.h
+++ b/adb/adb.h
@@ -210,8 +210,8 @@
#define USB_FFS_ADB_IN USB_FFS_ADB_EP(ep2)
#endif
-int handle_host_request(const char* service, TransportType type, const char* serial,
- TransportId transport_id, int reply_fd, asocket* s);
+bool handle_host_request(const char* service, TransportType type, const char* serial,
+ TransportId transport_id, int reply_fd, asocket* s);
void handle_online(atransport* t);
void handle_offline(atransport* t);
diff --git a/adb/sockets.cpp b/adb/sockets.cpp
index 69b5180..1534792 100644
--- a/adb/sockets.cpp
+++ b/adb/sockets.cpp
@@ -685,13 +685,9 @@
if (service) {
asocket* s2;
- /* some requests are handled immediately -- in that
- ** case the handle_host_request() routine has sent
- ** the OKAY or FAIL message and all we have to do
- ** is clean up.
- */
- if (handle_host_request(service, type, serial, transport_id, s->peer->fd, s) == 0) {
- /* XXX fail message? */
+ // Some requests are handled immediately -- in that case the handle_host_request() routine
+ // has sent the OKAY or FAIL message and all we have to do is clean up.
+ if (handle_host_request(service, type, serial, transport_id, s->peer->fd, s)) {
D("SS(%d): handled host service '%s'", s->id, service);
goto fail;
}