From af0e220fb1fb4bc1bffee0f8475dbce282faa272 Mon Sep 17 00:00:00 2001 From: Cody Schuffelen Date: Wed, 2 Jan 2019 14:17:29 -0800 Subject: [PATCH] Combine tcp_connect and socket_spec_connect. This will make it easier to add new types of transports by combining the logic the adb server and adb daemon use to connect to things. Bug:121166534 Test: adb connect against a cuttlefish instance over the shm proxy. Change-Id: Ic7fc848c60a85eef968c3735838c87cb7fdaf38b --- adb/client/adb_client.cpp | 26 ++++++++++----------- adb/services.cpp | 9 ++++--- adb/socket_spec.cpp | 49 +++++++++++++++++++++------------------ adb/socket_spec.h | 8 +++++-- adb/socket_spec_test.cpp | 35 +++++++++++++++------------- adb/transport_local.cpp | 35 +++++++++------------------- 6 files changed, 79 insertions(+), 83 deletions(-) diff --git a/adb/client/adb_client.cpp b/adb/client/adb_client.cpp index eda4b778a..9a25d104c 100644 --- a/adb/client/adb_client.cpp +++ b/adb/client/adb_client.cpp @@ -144,53 +144,51 @@ static int _adb_connect(const std::string& service, std::string* error) { } std::string reason; - int fd = socket_spec_connect(__adb_server_socket_spec, &reason); - if (fd < 0) { + unique_fd fd; + if (!socket_spec_connect(&fd, __adb_server_socket_spec, nullptr, nullptr, &reason)) { *error = android::base::StringPrintf("cannot connect to daemon at %s: %s", __adb_server_socket_spec, reason.c_str()); return -2; } - if (memcmp(&service[0], "host", 4) != 0 && switch_socket_transport(fd, error)) { + if (memcmp(&service[0], "host", 4) != 0 && switch_socket_transport(fd.get(), error)) { return -1; } - if (!SendProtocolString(fd, service)) { + if (!SendProtocolString(fd.get(), service)) { *error = perror_str("write failure during connection"); - adb_close(fd); return -1; } - if (!adb_status(fd, error)) { - adb_close(fd); + if (!adb_status(fd.get(), error)) { return -1; } - D("_adb_connect: return fd %d", fd); - return fd; + D("_adb_connect: return fd %d", fd.get()); + return fd.release(); } bool adb_kill_server() { D("adb_kill_server"); std::string reason; - int fd = socket_spec_connect(__adb_server_socket_spec, &reason); - if (fd < 0) { + unique_fd fd; + if (!socket_spec_connect(&fd, __adb_server_socket_spec, nullptr, nullptr, &reason)) { fprintf(stderr, "cannot connect to daemon at %s: %s\n", __adb_server_socket_spec, reason.c_str()); return true; } - if (!SendProtocolString(fd, "host:kill")) { + if (!SendProtocolString(fd.get(), "host:kill")) { fprintf(stderr, "error: write failure during connection: %s\n", strerror(errno)); return false; } // The server might send OKAY, so consume that. char buf[4]; - ReadFdExactly(fd, buf, 4); + ReadFdExactly(fd.get(), buf, 4); // Now that no more data is expected, wait for socket orderly shutdown or error, indicating // server death. - ReadOrderlyShutdown(fd); + ReadOrderlyShutdown(fd.get()); return true; } diff --git a/adb/services.cpp b/adb/services.cpp index 863665774..73fed0932 100644 --- a/adb/services.cpp +++ b/adb/services.cpp @@ -72,24 +72,23 @@ unique_fd create_service_thread(const char* service_name, std::function= 0) { close_on_exec(ret); } - return ret; + return ret.release(); } #if ADB_HOST diff --git a/adb/socket_spec.cpp b/adb/socket_spec.cpp index 4cddc8444..cc67b6b9f 100644 --- a/adb/socket_spec.cpp +++ b/adb/socket_spec.cpp @@ -67,7 +67,7 @@ static auto& kLocalSocketTypes = *new std::unordered_mapreset(network_loopback_client(port_value, SOCK_STREAM, error)); } else { #if ADB_HOST - result = network_connect(hostname, port, SOCK_STREAM, 0, error); + fd->reset(network_connect(hostname, port_value, SOCK_STREAM, 0, error)); #else // Disallow arbitrary connections in adbd. *error = "adbd does not support arbitrary tcp connections"; - return -1; + return false; #endif } - if (result >= 0) { - disable_tcp_nagle(result); + if (fd->get() > 0) { + disable_tcp_nagle(fd->get()); + if (port) { + *port = port_value; + } + return true; } - return result; + return false; } for (const auto& it : kLocalSocketTypes) { std::string prefix = it.first + ":"; - if (spec.starts_with(prefix)) { + if (address.starts_with(prefix)) { if (!it.second.available) { *error = StringPrintf("socket type %s is unavailable on this platform", it.first.c_str()); - return -1; + return false; } - return network_local_client(&spec[prefix.length()], it.second.socket_namespace, - SOCK_STREAM, error); + fd->reset(network_local_client(&address[prefix.length()], it.second.socket_namespace, + SOCK_STREAM, error)); + return true; } } *error = "unknown socket specification: "; - *error += spec; - return -1; + *error += address; + return false; } int socket_spec_listen(std::string_view spec, std::string* error, int* resolved_tcp_port) { if (spec.starts_with("tcp:")) { std::string hostname; int port; - if (!parse_tcp_socket_spec(spec, &hostname, &port, error)) { + if (!parse_tcp_socket_spec(spec, &hostname, &port, nullptr, error)) { return -1; } diff --git a/adb/socket_spec.h b/adb/socket_spec.h index 5b0697322..687d751f6 100644 --- a/adb/socket_spec.h +++ b/adb/socket_spec.h @@ -17,14 +17,18 @@ #pragma once #include +#include + +#include "adb_unique_fd.h" // Returns true if the argument starts with a plausible socket prefix. bool is_socket_spec(std::string_view spec); bool is_local_socket_spec(std::string_view spec); -int socket_spec_connect(std::string_view spec, std::string* error); +bool socket_spec_connect(unique_fd* fd, std::string_view address, int* port, std::string* serial, + std::string* error); int socket_spec_listen(std::string_view spec, std::string* error, int* resolved_tcp_port = nullptr); // Exposed for testing. bool parse_tcp_socket_spec(std::string_view spec, std::string* hostname, int* port, - std::string* error); + std::string* serial, std::string* error); diff --git a/adb/socket_spec_test.cpp b/adb/socket_spec_test.cpp index 40ce21cea..f5ec0f15c 100644 --- a/adb/socket_spec_test.cpp +++ b/adb/socket_spec_test.cpp @@ -21,34 +21,37 @@ #include TEST(socket_spec, parse_tcp_socket_spec) { - std::string hostname, error; + std::string hostname, error, serial; int port; - EXPECT_TRUE(parse_tcp_socket_spec("tcp:5037", &hostname, &port, &error)); + EXPECT_TRUE(parse_tcp_socket_spec("tcp:5037", &hostname, &port, &serial, &error)); EXPECT_EQ("", hostname); EXPECT_EQ(5037, port); + EXPECT_EQ("", serial); // Bad ports: - EXPECT_FALSE(parse_tcp_socket_spec("tcp:", &hostname, &port, &error)); - EXPECT_FALSE(parse_tcp_socket_spec("tcp:-1", &hostname, &port, &error)); - EXPECT_FALSE(parse_tcp_socket_spec("tcp:65536", &hostname, &port, &error)); + EXPECT_FALSE(parse_tcp_socket_spec("tcp:", &hostname, &port, &serial, &error)); + EXPECT_FALSE(parse_tcp_socket_spec("tcp:-1", &hostname, &port, &serial, &error)); + EXPECT_FALSE(parse_tcp_socket_spec("tcp:65536", &hostname, &port, &serial, &error)); - EXPECT_TRUE(parse_tcp_socket_spec("tcp:localhost:1234", &hostname, &port, &error)); + EXPECT_TRUE(parse_tcp_socket_spec("tcp:localhost:1234", &hostname, &port, &serial, &error)); EXPECT_EQ("localhost", hostname); EXPECT_EQ(1234, port); + EXPECT_EQ("localhost:1234", serial); - EXPECT_FALSE(parse_tcp_socket_spec("tcp:localhost", &hostname, &port, &error)); - EXPECT_FALSE(parse_tcp_socket_spec("tcp:localhost:", &hostname, &port, &error)); - EXPECT_FALSE(parse_tcp_socket_spec("tcp:localhost:-1", &hostname, &port, &error)); - EXPECT_FALSE(parse_tcp_socket_spec("tcp:localhost:65536", &hostname, &port, &error)); + EXPECT_FALSE(parse_tcp_socket_spec("tcp:localhost", &hostname, &port, &serial, &error)); + EXPECT_FALSE(parse_tcp_socket_spec("tcp:localhost:", &hostname, &port, &serial, &error)); + EXPECT_FALSE(parse_tcp_socket_spec("tcp:localhost:-1", &hostname, &port, &serial, &error)); + EXPECT_FALSE(parse_tcp_socket_spec("tcp:localhost:65536", &hostname, &port, &serial, &error)); // IPv6: - EXPECT_TRUE(parse_tcp_socket_spec("tcp:[::1]:1234", &hostname, &port, &error)); + EXPECT_TRUE(parse_tcp_socket_spec("tcp:[::1]:1234", &hostname, &port, &serial, &error)); EXPECT_EQ("::1", hostname); EXPECT_EQ(1234, port); + EXPECT_EQ("[::1]:1234", serial); - EXPECT_FALSE(parse_tcp_socket_spec("tcp:[::1]", &hostname, &port, &error)); - EXPECT_FALSE(parse_tcp_socket_spec("tcp:[::1]:", &hostname, &port, &error)); - EXPECT_FALSE(parse_tcp_socket_spec("tcp:[::1]:-1", &hostname, &port, &error)); - EXPECT_FALSE(parse_tcp_socket_spec("tcp:::1", &hostname, &port, &error)); - EXPECT_FALSE(parse_tcp_socket_spec("tcp:::1:1234", &hostname, &port, &error)); + EXPECT_FALSE(parse_tcp_socket_spec("tcp:[::1]", &hostname, &port, &serial, &error)); + EXPECT_FALSE(parse_tcp_socket_spec("tcp:[::1]:", &hostname, &port, &serial, &error)); + EXPECT_FALSE(parse_tcp_socket_spec("tcp:[::1]:-1", &hostname, &port, &serial, &error)); + EXPECT_FALSE(parse_tcp_socket_spec("tcp:::1", &hostname, &port, &serial, &error)); + EXPECT_FALSE(parse_tcp_socket_spec("tcp:::1:1234", &hostname, &port, &serial, &error)); } diff --git a/adb/transport_local.cpp b/adb/transport_local.cpp index dc87ac74f..b384085c7 100644 --- a/adb/transport_local.cpp +++ b/adb/transport_local.cpp @@ -45,6 +45,7 @@ #include "adb_io.h" #include "adb_unique_fd.h" #include "adb_utils.h" +#include "socket_spec.h" #include "sysdeps/chrono.h" #if ADB_HOST @@ -70,32 +71,17 @@ bool local_connect(int port) { std::tuple tcp_connect(const std::string& address, std::string* response) { - std::string serial; - std::string host; + unique_fd fd; int port = DEFAULT_ADB_LOCAL_TRANSPORT_PORT; - if (!android::base::ParseNetAddress(address, &host, &port, &serial, response)) { - D("failed to parse address: '%s'", address.c_str()); - return std::make_tuple(unique_fd(), port, serial); - } - - std::string error; - unique_fd 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(), error.c_str()); + std::string serial; + if (socket_spec_connect(&fd, "tcp:" + address, &port, &serial, response)) { + close_on_exec(fd); + if (!set_tcp_keepalive(fd, 1)) { + D("warning: failed to configure TCP keepalives (%s)", strerror(errno)); + } return std::make_tuple(std::move(fd), port, serial); } - - D("client: connected %s remote on fd %d", serial.c_str(), fd.get()); - close_on_exec(fd); - disable_tcp_nagle(fd); - - // Send a TCP keepalive ping to the device every second so we can detect disconnects. - if (!set_tcp_keepalive(fd, 1)) { - D("warning: failed to configure TCP keepalives (%s)", strerror(errno)); - } - - return std::make_tuple(std::move(fd), port, serial); + return std::make_tuple(unique_fd(), 0, ""); } void connect_device(const std::string& address, std::string* response) { @@ -251,8 +237,9 @@ static void server_socket_thread(int port) { adb_thread_setname("server socket"); D("transport: server_socket_thread() starting"); while (serverfd == -1) { + std::string spec = android::base::StringPrintf("tcp:%d", port); std::string error; - serverfd.reset(network_inaddr_any_server(port, SOCK_STREAM, &error)); + serverfd.reset(socket_spec_listen(spec, &error)); if (serverfd < 0) { D("server: cannot bind socket yet: %s", error.c_str()); std::this_thread::sleep_for(1s);