adb: fix register_socket_transport related double-closes.
Multiple codepaths were closing the fd they passed into
register_socket_transport on failure, which would close the fd itself.
Switch things over to unique_fd to make it clear that we don't actually
have to close on failure.
Test: mma
Change-Id: I2d9bdcb1142c24931d970f99ebdf9a8051daf05c
diff --git a/adb/adb.h b/adb/adb.h
index 7e9af9e..26b5fa1 100644
--- a/adb/adb.h
+++ b/adb/adb.h
@@ -133,7 +133,7 @@
int adb_server_main(int is_daemon, const std::string& socket_spec, int ack_reply_fd);
/* initialize a transport object's func pointers and state */
-int init_socket_transport(atransport* t, int s, int port, int local);
+int init_socket_transport(atransport* t, unique_fd s, int port, int local);
void init_usb_transport(atransport* t, usb_handle* usb);
std::string getEmulatorSerialString(int console_port);
diff --git a/adb/transport.cpp b/adb/transport.cpp
index 638940c..793c283 100644
--- a/adb/transport.cpp
+++ b/adb/transport.cpp
@@ -1157,7 +1157,7 @@
}
#endif // ADB_HOST
-int register_socket_transport(int s, const char* serial, int port, int local,
+int register_socket_transport(unique_fd s, const char* serial, int port, int local,
atransport::ReconnectCallback reconnect) {
atransport* t = new atransport(std::move(reconnect), kCsOffline);
@@ -1167,8 +1167,8 @@
serial = buf;
}
- D("transport: %s init'ing for socket %d, on port %d", serial, s, port);
- if (init_socket_transport(t, s, port, local) < 0) {
+ D("transport: %s init'ing for socket %d, on port %d", serial, s.get(), port);
+ if (init_socket_transport(t, std::move(s), port, local) < 0) {
delete t;
return -1;
}
diff --git a/adb/transport.h b/adb/transport.h
index e9c9d37..1844ae8 100644
--- a/adb/transport.h
+++ b/adb/transport.h
@@ -364,7 +364,7 @@
void connect_device(const std::string& address, std::string* response);
/* cause new transports to be init'd and added to the list */
-int register_socket_transport(int s, const char* serial, int port, int local,
+int register_socket_transport(unique_fd s, const char* serial, int port, int local,
atransport::ReconnectCallback reconnect);
// This should only be used for transports with connection_state == kCsNoPerm.
diff --git a/adb/transport_local.cpp b/adb/transport_local.cpp
index 122ad12..9398ceb 100644
--- a/adb/transport_local.cpp
+++ b/adb/transport_local.cpp
@@ -122,12 +122,12 @@
// invoked if the atransport* has already been setup. This eventually
// calls atransport->SetConnection() with a newly created Connection*
// that will in turn send the CNXN packet.
- return init_socket_transport(t, fd.release(), port, 0) >= 0;
+ return init_socket_transport(t, std::move(fd), port, 0) >= 0;
};
- int ret = register_socket_transport(fd.release(), serial.c_str(), port, 0, std::move(reconnect));
+ int ret =
+ register_socket_transport(std::move(fd), serial.c_str(), port, 0, std::move(reconnect));
if (ret < 0) {
- adb_close(fd);
if (ret == -EALREADY) {
*response = android::base::StringPrintf("already connected to %s", serial.c_str());
} else {
@@ -162,7 +162,7 @@
close_on_exec(fd.get());
disable_tcp_nagle(fd.get());
std::string serial = getEmulatorSerialString(console_port);
- if (register_socket_transport(fd.release(), serial.c_str(), adb_port, 1,
+ if (register_socket_transport(std::move(fd), serial.c_str(), adb_port, 1,
[](atransport*) { return false; }) == 0) {
return 0;
}
@@ -242,34 +242,31 @@
#else // ADB_HOST
static void server_socket_thread(int port) {
- int serverfd, fd;
+ unique_fd serverfd;
adb_thread_setname("server socket");
D("transport: server_socket_thread() starting");
- serverfd = -1;
- for(;;) {
- if(serverfd == -1) {
- std::string error;
- serverfd = network_inaddr_any_server(port, SOCK_STREAM, &error);
- if(serverfd < 0) {
- D("server: cannot bind socket yet: %s", error.c_str());
- std::this_thread::sleep_for(1s);
- continue;
- }
- close_on_exec(serverfd);
+ while (serverfd == -1) {
+ std::string error;
+ serverfd.reset(network_inaddr_any_server(port, SOCK_STREAM, &error));
+ if (serverfd < 0) {
+ D("server: cannot bind socket yet: %s", error.c_str());
+ std::this_thread::sleep_for(1s);
+ continue;
}
+ close_on_exec(serverfd.get());
+ }
+ while (true) {
D("server: trying to get new connection from %d", port);
- fd = adb_socket_accept(serverfd, nullptr, nullptr);
- if(fd >= 0) {
- D("server: new connection on fd %d", fd);
- close_on_exec(fd);
- disable_tcp_nagle(fd);
- std::string serial = android::base::StringPrintf("host-%d", fd);
- if (register_socket_transport(fd, serial.c_str(), port, 1,
- [](atransport*) { return false; }) != 0) {
- adb_close(fd);
- }
+ unique_fd fd(adb_socket_accept(serverfd, nullptr, nullptr));
+ if (fd >= 0) {
+ D("server: new connection on fd %d", fd.get());
+ close_on_exec(fd.get());
+ disable_tcp_nagle(fd.get());
+ std::string serial = android::base::StringPrintf("host-%d", fd.get());
+ register_socket_transport(std::move(fd), serial.c_str(), port, 1,
+ [](atransport*) { return false; });
}
}
D("transport: server_socket_thread() exiting");
@@ -330,7 +327,6 @@
/* 'ok' reply from the adb QEMUD service. */
static const char _ok_resp[] = "ok";
- int fd;
char tmp[256];
char con_name[32];
@@ -341,7 +337,7 @@
snprintf(con_name, sizeof(con_name), "pipe:qemud:adb:%d", port);
/* Connect to the adb QEMUD service. */
- fd = qemu_pipe_open(con_name);
+ unique_fd fd(qemu_pipe_open(con_name));
if (fd < 0) {
/* This could be an older version of the emulator, that doesn't
* implement adb QEMUD service. Fall back to the old TCP way. */
@@ -350,31 +346,28 @@
return;
}
- for(;;) {
+ while (true) {
/*
* Wait till the host creates a new connection.
*/
/* Send the 'accept' request. */
- if (WriteFdExactly(fd, _accept_req, strlen(_accept_req))) {
+ if (WriteFdExactly(fd.get(), _accept_req, strlen(_accept_req))) {
/* Wait for the response. In the response we expect 'ok' on success,
* or 'ko' on failure. */
- if (!ReadFdExactly(fd, tmp, 2) || memcmp(tmp, _ok_resp, 2)) {
+ if (!ReadFdExactly(fd.get(), tmp, 2) || memcmp(tmp, _ok_resp, 2)) {
D("Accepting ADB host connection has failed.");
- adb_close(fd);
} else {
/* Host is connected. Register the transport, and start the
* exchange. */
- std::string serial = android::base::StringPrintf("host-%d", fd);
- if (register_socket_transport(fd, serial.c_str(), port, 1,
- [](atransport*) { return false; }) != 0 ||
- !WriteFdExactly(fd, _start_req, strlen(_start_req))) {
- adb_close(fd);
- }
+ std::string serial = android::base::StringPrintf("host-%d", fd.get());
+ WriteFdExactly(fd.get(), _start_req, strlen(_start_req));
+ register_socket_transport(std::move(fd), serial.c_str(), port, 1,
+ [](atransport*) { return false; });
}
/* Prepare for accepting of the next ADB host connection. */
- fd = qemu_pipe_open(con_name);
+ fd.reset(qemu_pipe_open(con_name));
if (fd < 0) {
D("adb service become unavailable.");
return;
@@ -475,10 +468,9 @@
}
#endif
-int init_socket_transport(atransport* t, int s, int adb_port, int local) {
+int init_socket_transport(atransport* t, unique_fd fd, int adb_port, int local) {
int fail = 0;
- unique_fd fd(s);
t->type = kTransportLocal;
#if ADB_HOST