From 34cd60f0751c22200988d3a462bdf00e2ed36edd Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Wed, 22 Apr 2020 17:48:43 -0700 Subject: [PATCH 1/4] adb: mark kMaxProcessNameLength as constexpr. Otherwise, the build fails with -O0. Test: mma with -O0 in adb_defaults Change-Id: Id10e320afc183eda5b46555b3b50dd8ffd84a700 --- adb/libs/adbconnection/include/adbconnection/process_info.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/adb/libs/adbconnection/include/adbconnection/process_info.h b/adb/libs/adbconnection/include/adbconnection/process_info.h index 86d325914..d22669978 100644 --- a/adb/libs/adbconnection/include/adbconnection/process_info.h +++ b/adb/libs/adbconnection/include/adbconnection/process_info.h @@ -21,7 +21,7 @@ #include struct ProcessInfo { - const static size_t kMaxArchNameLength = 16; + static constexpr size_t kMaxArchNameLength = 16; uint64_t pid; bool debuggable; From ad18395b654edf03f4f0b2f9604741e67cecdee1 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Wed, 22 Apr 2020 18:40:00 -0700 Subject: [PATCH 2/4] adbd: respect ADB_TRACE on host adbd. Test: ADB_TRACE=1 adbd Change-Id: I7cebbf7596add865fab95f5d6c746c0b8a16997a --- adb/adb_trace.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/adb/adb_trace.cpp b/adb/adb_trace.cpp index c579dde92..210241ce7 100644 --- a/adb/adb_trace.cpp +++ b/adb/adb_trace.cpp @@ -90,7 +90,7 @@ void start_device_log(void) { int adb_trace_mask; std::string get_trace_setting() { -#if ADB_HOST +#if ADB_HOST || !defined(__ANDROID__) const char* setting = getenv("ADB_TRACE"); if (setting == nullptr) { setting = ""; From a786f0a8572e1a156bbd910668d611a8ddccb732 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Wed, 22 Apr 2020 19:28:26 -0700 Subject: [PATCH 3/4] adb: don't run all of the tests again over TCP in coverage. They take forever, and we're only really interested in the transport related code, so test that more directly. Test: ./coverage/gen_coverage.sh Change-Id: I47d3ad50db0f1020fe4b3da5cdfe455190d022b5 --- adb/coverage/gen_coverage.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/adb/coverage/gen_coverage.sh b/adb/coverage/gen_coverage.sh index cced62a29..ace798f3b 100755 --- a/adb/coverage/gen_coverage.sh +++ b/adb/coverage/gen_coverage.sh @@ -85,8 +85,9 @@ sleep 5 adb connect $REMOTE adb -s $REMOTE wait-for-device -# Run test_device.py again. -ANDROID_SERIAL=$REMOTE "$OUTPUT_DIR"/../test_device.py +# Instead of running test_device.py again, which takes forever, do some I/O back and forth instead. +dd if=/dev/zero bs=1024 count=10240 | adb -s $REMOTE raw sink:10485760 +adb -s $REMOTE raw source:10485760 | dd of=/dev/null bs=1024 count=10240 # Dump traces again. adb disconnect $REMOTE From 9f3064f26fd971eb7e81b78d48aae6aeee7ca143 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Wed, 22 Apr 2020 20:57:26 -0700 Subject: [PATCH 4/4] adbd: avoid compiling more code in the daemon. Strip out more code that has no meaning on device, to "improve" coverage. Test: test_device.py over TCP Change-Id: Id8d9fa6cc6c6c30773f67303bcc89e6d60824700 --- adb/adb.cpp | 2 ++ adb/adb_listeners.cpp | 12 ++++++++++-- adb/adb_listeners.h | 9 ++++----- adb/client/main.cpp | 2 +- adb/daemon/main.cpp | 6 ------ adb/socket.h | 3 +++ adb/sockets.cpp | 19 ++----------------- adb/transport.cpp | 20 ++++++++++---------- adb/transport.h | 16 +++++++++++++++- adb/transport_test.cpp | 2 ++ 10 files changed, 49 insertions(+), 42 deletions(-) diff --git a/adb/adb.cpp b/adb/adb.cpp index dcec0ba11..08986b77e 100644 --- a/adb/adb.cpp +++ b/adb/adb.cpp @@ -109,7 +109,9 @@ void handle_online(atransport *t) { D("adb: online"); t->online = 1; +#if ADB_HOST t->SetConnectionEstablished(true); +#endif } void handle_offline(atransport *t) diff --git a/adb/adb_listeners.cpp b/adb/adb_listeners.cpp index 43a925282..124e2d8dc 100644 --- a/adb/adb_listeners.cpp +++ b/adb/adb_listeners.cpp @@ -73,6 +73,7 @@ static auto& listener_list_mutex = *new std::mutex(); typedef std::list> ListenerList; static ListenerList& listener_list GUARDED_BY(listener_list_mutex) = *new ListenerList(); +#if ADB_HOST static void ss_listener_event_func(int _fd, unsigned ev, void *_l) { if (ev & FDE_READ) { unique_fd fd(adb_socket_accept(_fd, nullptr, nullptr)); @@ -88,6 +89,7 @@ static void ss_listener_event_func(int _fd, unsigned ev, void *_l) { } } } +#endif static void listener_event_func(int _fd, unsigned ev, void* _l) { @@ -164,7 +166,7 @@ void remove_all_listeners() EXCLUDES(listener_list_mutex) { } } -void enable_daemon_sockets() EXCLUDES(listener_list_mutex) { +void enable_server_sockets() EXCLUDES(listener_list_mutex) { std::lock_guard lock(listener_list_mutex); for (auto& l : listener_list) { if (l->connect_to == "*smartsocket*") { @@ -173,6 +175,7 @@ void enable_daemon_sockets() EXCLUDES(listener_list_mutex) { } } +#if ADB_HOST void close_smartsockets() EXCLUDES(listener_list_mutex) { std::lock_guard lock(listener_list_mutex); auto pred = [](const std::unique_ptr& listener) { @@ -180,6 +183,7 @@ void close_smartsockets() EXCLUDES(listener_list_mutex) { }; listener_list.remove_if(pred); } +#endif InstallStatus install_listener(const std::string& local_name, const char* connect_to, atransport* transport, int flags, int* resolved_tcp_port, @@ -188,7 +192,7 @@ InstallStatus install_listener(const std::string& local_name, const char* connec for (auto& l : listener_list) { if (local_name == l->local_name) { // Can't repurpose a smartsocket. - if(l->connect_to[0] == '*') { + if (l->connect_to[0] == '*') { *error = "cannot repurpose smartsocket"; return INSTALL_STATUS_INTERNAL_ERROR; } @@ -227,7 +231,11 @@ InstallStatus install_listener(const std::string& local_name, const char* connec close_on_exec(listener->fd); if (listener->connect_to == "*smartsocket*") { +#if ADB_HOST listener->fde = fdevent_create(listener->fd, ss_listener_event_func, listener.get()); +#else + LOG(FATAL) << "attempted to connect to *smartsocket* in daemon"; +#endif } else { listener->fde = fdevent_create(listener->fd, listener_event_func, listener.get()); } diff --git a/adb/adb_listeners.h b/adb/adb_listeners.h index 354dcc5c8..0aa774a7d 100644 --- a/adb/adb_listeners.h +++ b/adb/adb_listeners.h @@ -14,8 +14,7 @@ * limitations under the License. */ -#ifndef __ADB_LISTENERS_H -#define __ADB_LISTENERS_H +#pragma once #include "adb.h" @@ -44,7 +43,7 @@ std::string format_listeners(); InstallStatus remove_listener(const char* local_name, atransport* transport); void remove_all_listeners(void); -void enable_daemon_sockets(); +#if ADB_HOST +void enable_server_sockets(); void close_smartsockets(); - -#endif /* __ADB_LISTENERS_H */ +#endif diff --git a/adb/client/main.cpp b/adb/client/main.cpp index 05e210f60..a19bd6d31 100644 --- a/adb/client/main.cpp +++ b/adb/client/main.cpp @@ -206,7 +206,7 @@ int adb_server_main(int is_daemon, const std::string& socket_spec, int ack_reply // We don't accept() client connections until this point: this way, clients // can't see wonky state early in startup even if they're connecting directly // to the server instead of going through the adb program. - fdevent_run_on_main_thread([] { enable_daemon_sockets(); }); + fdevent_run_on_main_thread([] { enable_server_sockets(); }); }); notify_thread.detach(); diff --git a/adb/daemon/main.cpp b/adb/daemon/main.cpp index 55b77835d..db8f07ba4 100644 --- a/adb/daemon/main.cpp +++ b/adb/daemon/main.cpp @@ -173,12 +173,6 @@ static void drop_privileges(int server_port) { LOG(FATAL) << "Could not set SELinux context"; } } - std::string error; - std::string local_name = - android::base::StringPrintf("tcp:%d", server_port); - if (install_listener(local_name, "*smartsocket*", nullptr, 0, nullptr, &error)) { - LOG(FATAL) << "Could not install *smartsocket* listener: " << error; - } } } #endif diff --git a/adb/socket.h b/adb/socket.h index 4276851d2..062320489 100644 --- a/adb/socket.h +++ b/adb/socket.h @@ -108,7 +108,10 @@ asocket* create_local_service_socket(std::string_view destination, atransport* t asocket *create_remote_socket(unsigned id, atransport *t); void connect_to_remote(asocket* s, std::string_view destination); + +#if ADB_HOST void connect_to_smartsocket(asocket *s); +#endif // Internal functions that are only made available here for testing purposes. namespace internal { diff --git a/adb/sockets.cpp b/adb/sockets.cpp index 423af67f1..13a473776 100644 --- a/adb/sockets.cpp +++ b/adb/sockets.cpp @@ -520,6 +520,7 @@ void connect_to_remote(asocket* s, std::string_view destination) { send_packet(p, s->transport); } +#if ADB_HOST /* this is used by magic sockets to rig local sockets to send the go-ahead message when they connect */ static void local_socket_ready_notify(asocket* s) { @@ -584,8 +585,6 @@ static unsigned unhex(const char* s, int len) { return n; } -#if ADB_HOST - namespace internal { // Parses a host service string of the following format: @@ -714,15 +713,11 @@ bool parse_host_service(std::string_view* out_serial, std::string_view* out_comm } // namespace internal -#endif // ADB_HOST - static int smart_socket_enqueue(asocket* s, apacket::payload_type data) { -#if ADB_HOST std::string_view service; std::string_view serial; TransportId transport_id = 0; TransportType type = kTransportAny; -#endif D("SS(%d): enqueue %zu", s->id, data.size()); @@ -755,7 +750,6 @@ static int smart_socket_enqueue(asocket* s, apacket::payload_type data) { D("SS(%d): '%s'", s->id, (char*)(s->smart_socket_data.data() + 4)); -#if ADB_HOST service = std::string_view(s->smart_socket_data).substr(4); // TODO: These should be handled in handle_host_request. @@ -841,16 +835,6 @@ static int smart_socket_enqueue(asocket* s, apacket::payload_type data) { s2->ready(s2); return 0; } -#else /* !ADB_HOST */ - if (s->transport == nullptr) { - std::string error_msg = "unknown failure"; - s->transport = acquire_one_transport(kTransportAny, nullptr, 0, nullptr, &error_msg); - if (s->transport == nullptr) { - SendFail(s->peer->fd, error_msg); - goto fail; - } - } -#endif if (!s->transport) { SendFail(s->peer->fd, "device offline (no transport)"); @@ -922,6 +906,7 @@ void connect_to_smartsocket(asocket* s) { ss->peer = s; s->ready(s); } +#endif size_t asocket::get_max_payload() const { size_t max_payload = MAX_PAYLOAD; diff --git a/adb/transport.cpp b/adb/transport.cpp index 25ed36654..16670110e 100644 --- a/adb/transport.cpp +++ b/adb/transport.cpp @@ -928,6 +928,7 @@ static void transport_destroy(atransport* t) { remove_transport(t); } +#if ADB_HOST static int qual_match(const std::string& to_test, const char* prefix, const std::string& qual, bool sanitize_qual) { if (to_test.empty()) /* Return true if both the qual and to_test are empty strings. */ @@ -1083,10 +1084,13 @@ void ConnectionWaitable::SetConnectionEstablished(bool success) { } cv_.notify_one(); } +#endif atransport::~atransport() { +#if ADB_HOST // If the connection callback had not been run before, run it now. SetConnectionEstablished(false); +#endif } int atransport::Write(apacket* p) { @@ -1240,6 +1244,7 @@ void atransport::RunDisconnects() { disconnects_.clear(); } +#if ADB_HOST bool atransport::MatchesTarget(const std::string& target) const { if (!serial.empty()) { if (target == serial) { @@ -1283,8 +1288,6 @@ ReconnectResult atransport::Reconnect() { return reconnect_(this); } -#if ADB_HOST - // We use newline as our delimiter, make sure to never output it. static std::string sanitize(std::string str, bool alphanumeric) { auto pred = alphanumeric ? [](const char c) { return !isalnum(c); } @@ -1366,7 +1369,7 @@ void close_usb_devices(std::function predicate, bool re void close_usb_devices(bool reset) { close_usb_devices([](const atransport*) { return true; }, reset); } -#endif // ADB_HOST +#endif bool register_socket_transport(unique_fd s, std::string serial, int port, int local, atransport::ReconnectCallback reconnect, bool use_tls, int* error) { @@ -1406,7 +1409,9 @@ bool register_socket_transport(unique_fd s, std::string serial, int port, int lo lock.unlock(); +#if ADB_HOST auto waitable = t->connection_waitable(); +#endif register_transport(t); if (local == 1) { @@ -1414,6 +1419,7 @@ bool register_socket_transport(unique_fd s, std::string serial, int port, int lo return true; } +#if ADB_HOST if (!waitable->WaitForConnection(std::chrono::seconds(10))) { if (error) *error = ETIMEDOUT; return false; @@ -1423,6 +1429,7 @@ bool register_socket_transport(unique_fd s, std::string serial, int port, int lo if (error) *error = EPERM; return false; } +#endif return true; } @@ -1453,14 +1460,9 @@ void kick_all_tcp_devices() { t->Kick(); } } -#if ADB_HOST reconnect_handler.CheckForKicked(); -#endif } -#endif - -#if ADB_HOST void register_usb_transport(usb_handle* usb, const char* serial, const char* devpath, unsigned writeable) { atransport* t = new atransport(writeable ? kCsOffline : kCsNoPerm); @@ -1482,9 +1484,7 @@ void register_usb_transport(usb_handle* usb, const char* serial, const char* dev register_transport(t); } -#endif -#if ADB_HOST // This should only be used for transports with connection_state == kCsNoPerm. void unregister_usb_transport(usb_handle* usb) { std::lock_guard lock(transport_lock); diff --git a/adb/transport.h b/adb/transport.h index e93c31cbc..2ac21cf63 100644 --- a/adb/transport.h +++ b/adb/transport.h @@ -262,9 +262,12 @@ class atransport : public enable_weak_from_this { : id(NextTransportId()), kicked_(false), connection_state_(state), - connection_waitable_(std::make_shared()), connection_(nullptr), reconnect_(std::move(reconnect)) { +#if ADB_HOST + connection_waitable_ = std::make_shared(); +#endif + // Initialize protocol to min version for compatibility with older versions. // Version will be updated post-connect. protocol_version = A_VERSION_MIN; @@ -350,6 +353,7 @@ class atransport : public enable_weak_from_this { void RemoveDisconnect(adisconnect* disconnect); void RunDisconnects(); +#if ADB_HOST // Returns true if |target| matches this transport. A matching |target| can be any of: // * // * @@ -374,6 +378,7 @@ class atransport : public enable_weak_from_this { // Attempts to reconnect with the underlying Connection. ReconnectResult Reconnect(); +#endif private: std::atomic kicked_; @@ -392,9 +397,11 @@ class atransport : public enable_weak_from_this { std::deque> keys_; #endif +#if ADB_HOST // A sharable object that can be used to wait for the atransport's // connection to be established. std::shared_ptr connection_waitable_; +#endif // The underlying connection object. std::shared_ptr connection_ GUARDED_BY(mutex_); @@ -434,10 +441,17 @@ void init_reconnect_handler(void); void init_transport_registration(void); void init_mdns_transport_discovery(void); std::string list_transports(bool long_listing); + +#if ADB_HOST atransport* find_transport(const char* serial); + void kick_all_tcp_devices(); +#endif + void kick_all_transports(); + void kick_all_tcp_tls_transports(); + #if !ADB_HOST void kick_all_transports_by_auth_key(std::string_view auth_key); #endif diff --git a/adb/transport_test.cpp b/adb/transport_test.cpp index a9ada4abf..8579ff4c2 100644 --- a/adb/transport_test.cpp +++ b/adb/transport_test.cpp @@ -127,6 +127,7 @@ TEST_F(TransportTest, parse_banner_features) { ASSERT_EQ(std::string("baz"), t.device); } +#if ADB_HOST TEST_F(TransportTest, test_matches_target) { std::string serial = "foo"; std::string devpath = "/path/to/bar"; @@ -183,3 +184,4 @@ TEST_F(TransportTest, test_matches_target_local) { EXPECT_FALSE(t.MatchesTarget("abc:100.100.100.100")); } } +#endif