am f4bda753: Merge "Add feature list to connection banner."

* commit 'f4bda7538955adef6d66b8f9871c83db67a7cde4':
  Add feature list to connection banner.

adb/adb.cpp

@@ -58,6 +58,8 @@ ADB_MUTEX_DEFINE(D_lock);
 #if !ADB_HOST
 const char* adb_device_banner = "device";
 static android::base::LogdLogger gLogdLogger;
+#else
+const char* adb_device_banner = "host";
 #endif
 
 void AdbLogger(android::base::LogId id, android::base::LogSeverity severity,
@@ -305,45 +307,50 @@ static void send_close(unsigned local, unsigned remote, atransport *t)
     send_packet(p, t);
 }
 
-static size_t fill_connect_data(char *buf, size_t bufsize)
+std::string get_connection_string() {
-{
+    std::vector<std::string> connection_properties;
-#if ADB_HOST
+
-    return snprintf(buf, bufsize, "host::") + 1;
+#if !ADB_HOST
-#else
+    static const char* cnxn_props[] = {
-    static const char *cnxn_props[] = {
         "ro.product.name",
         "ro.product.model",
         "ro.product.device",
     };
-    static const int num_cnxn_props = ARRAY_SIZE(cnxn_props);
-    int i;
-    size_t remaining = bufsize;
-    size_t len;
 
-    len = snprintf(buf, remaining, "%s::", adb_device_banner);
+    for (const auto& prop_name : cnxn_props) {
-    remaining -= len;
-    buf += len;
-    for (i = 0; i < num_cnxn_props; i++) {
         char value[PROPERTY_VALUE_MAX];
-        property_get(cnxn_props[i], value, "");
+        property_get(prop_name, value, "");
-        len = snprintf(buf, remaining, "%s=%s;", cnxn_props[i], value);
+        connection_properties.push_back(
-        remaining -= len;
+            android::base::StringPrintf("%s=%s", prop_name, value));
-        buf += len;
     }
-
-    return bufsize - remaining + 1;
 #endif
+
+    connection_properties.push_back(android::base::StringPrintf(
+        "features=%s", android::base::Join(supported_features(), ',').c_str()));
+
+    return android::base::StringPrintf(
+        "%s::%s", adb_device_banner,
+        android::base::Join(connection_properties, ';').c_str());
 }
 
-void send_connect(atransport *t)
+void send_connect(atransport* t) {
-{
     D("Calling send_connect \n");
-    apacket *cp = get_apacket();
+    apacket* cp = get_apacket();
     cp->msg.command = A_CNXN;
     cp->msg.arg0 = t->get_protocol_version();
     cp->msg.arg1 = t->get_max_payload();
-    cp->msg.data_length = fill_connect_data((char *)cp->data,
+
-                                            MAX_PAYLOAD_V1);
+    std::string connection_str = get_connection_string();
+    // Connect and auth packets are limited to MAX_PAYLOAD_V1 because we don't
+    // yet know how much data the other size is willing to accept.
+    if (connection_str.length() > MAX_PAYLOAD_V1) {
+        LOG(FATAL) << "Connection banner is too long (length = "
+                   << connection_str.length() << ")";
+    }
+
+    memcpy(cp->data, connection_str.c_str(), connection_str.length());
+    cp->msg.data_length = connection_str.length();
+
     send_packet(cp, t);
 }
 
@@ -356,8 +363,8 @@ static void qual_overwrite(char** dst, const std::string& src) {
     *dst = strdup(src.c_str());
 }
 
-void parse_banner(const char* banner, atransport* t) {
+void parse_banner(const std::string& banner, atransport* t) {
-    D("parse_banner: %s\n", banner);
+    D("parse_banner: %s\n", banner.c_str());
 
     // The format is something like:
     // "device::ro.product.name=x;ro.product.model=y;ro.product.device=z;".
@@ -380,6 +387,10 @@ void parse_banner(const char* banner, atransport* t) {
                 qual_overwrite(&t->model, value);
             } else if (key == "ro.product.device") {
                 qual_overwrite(&t->device, value);
+            } else if (key == "features") {
+                for (const auto& feature : android::base::Split(value, ",")) {
+                    t->add_feature(feature);
+                }
             }
         }
     }
@@ -407,6 +418,29 @@ void parse_banner(const char* banner, atransport* t) {
     }
 }
 
+static void handle_new_connection(atransport* t, apacket* p) {
+    if (t->connection_state != kCsOffline) {
+        t->connection_state = kCsOffline;
+        handle_offline(t);
+    }
+
+    t->update_version(p->msg.arg0, p->msg.arg1);
+    std::string banner(reinterpret_cast<const char*>(p->data),
+                       p->msg.data_length);
+    parse_banner(banner, t);
+
+#if ADB_HOST
+    handle_online(t);
+#else
+    if (!auth_required) {
+        handle_online(t);
+        send_connect(t);
+    } else {
+        send_auth_request(t);
+    }
+#endif
+}
+
 void handle_packet(apacket *p, atransport *t)
 {
     asocket *s;
@@ -431,25 +465,8 @@ void handle_packet(apacket *p, atransport *t)
         }
         return;
 
-    case A_CNXN: /* CONNECT(version, maxdata, "system-id-string") */
+    case A_CNXN:  // CONNECT(version, maxdata, "system-id-string")
-        if(t->connection_state != kCsOffline) {
+        handle_new_connection(t, p);
-            t->connection_state = kCsOffline;
-            handle_offline(t);
-        }
-
-        t->update_version(p->msg.arg0, p->msg.arg1);
-        parse_banner(reinterpret_cast<const char*>(p->data), t);
-
-#if ADB_HOST
-        handle_online(t);
-#else
-        if (!auth_required) {
-            handle_online(t);
-            send_connect(t);
-        } else {
-            send_auth_request(t);
-        }
-#endif
         break;
 
     case A_AUTH:

adb/adb.h

@@ -20,10 +20,10 @@
 #include <limits.h>
 #include <sys/types.h>
 
-#include <base/macros.h>
-
 #include <string>
 
+#include <base/macros.h>
+
 #include "adb_trace.h"
 #include "fdevent.h"
 
@@ -191,71 +191,6 @@ enum ConnectionState {
     kCsUnauthorized,
 };
 
-class atransport {
-public:
-    // TODO(danalbert): We expose waaaaaaay too much stuff because this was
-    // historically just a struct, but making the whole thing a more idiomatic
-    // class in one go is a very large change. Given how bad our testing is,
-    // it's better to do this piece by piece.
-
-    atransport() {
-        auth_fde = {};
-        transport_fde = {};
-        protocol_version = A_VERSION;
-        max_payload = MAX_PAYLOAD;
-    }
-
-    virtual ~atransport() {}
-
-    int (*read_from_remote)(apacket* p, atransport* t) = nullptr;
-    int (*write_to_remote)(apacket* p, atransport* t) = nullptr;
-    void (*close)(atransport* t) = nullptr;
-    void (*kick)(atransport* t) = nullptr;
-
-    int fd = -1;
-    int transport_socket = -1;
-    fdevent transport_fde;
-    int ref_count = 0;
-    uint32_t sync_token = 0;
-    ConnectionState connection_state = kCsOffline;
-    bool online = false;
-    TransportType type = kTransportAny;
-
-    // USB handle or socket fd as needed.
-    usb_handle* usb = nullptr;
-    int sfd = -1;
-
-    // Used to identify transports for clients.
-    char* serial = nullptr;
-    char* product = nullptr;
-    char* model = nullptr;
-    char* device = nullptr;
-    char* devpath = nullptr;
-    int adb_port = -1;  // Use for emulators (local transport)
-    bool kicked = false;
-
-    // A list of adisconnect callbacks called when the transport is kicked.
-    adisconnect disconnects = {};
-
-    void* key = nullptr;
-    unsigned char token[TOKEN_SIZE] = {};
-    fdevent auth_fde;
-    size_t failed_auth_attempts = 0;
-
-    const char* connection_state_name() const;
-
-    void update_version(int version, size_t payload);
-    int get_protocol_version() const;
-    size_t get_max_payload() const;
-
-private:
-    int protocol_version;
-    size_t max_payload;
-
-    DISALLOW_COPY_AND_ASSIGN(atransport);
-};
-
-
 /* A listener is an entity which binds to a local port
 ** and, upon receiving a connection on that port, creates
 ** an asocket to connect the new local connection to a
@@ -380,7 +315,8 @@ int adb_commandline(int argc, const char **argv);
 
 ConnectionState connection_state(atransport *t);
 
-extern const char *adb_device_banner;
+extern const char* adb_device_banner;
+
 #if !ADB_HOST
 extern int SHELL_EXIT_NOTIFY_FD;
 #endif // !ADB_HOST
@@ -405,4 +341,6 @@ void handle_offline(atransport *t);
 
 void send_connect(atransport *t);
 
+void parse_banner(const std::string&, atransport* t);
+
 #endif

adb/transport.cpp

@@ -29,6 +29,7 @@
 #include <list>
 
 #include <base/stringprintf.h>
+#include <base/strings.h>
 
 #include "adb.h"
 #include "adb_utils.h"
@@ -535,7 +536,7 @@ static void transport_registration_func(int _fd, unsigned ev, void *data)
 
     t = m.transport;
 
-    if(m.action == 0){
+    if (m.action == 0) {
         D("transport: %s removing and free'ing %d\n", t->serial, t->transport_socket);
 
             /* IMPORTANT: the remove closes one half of the
@@ -845,6 +846,29 @@ size_t atransport::get_max_payload() const {
     return max_payload;
 }
 
+// The list of features supported by the current system. Will be sent to the
+// other side of the connection in the banner.
+static const FeatureSet gSupportedFeatures = {
+    // None yet.
+};
+
+const FeatureSet& supported_features() {
+    return gSupportedFeatures;
+}
+
+bool atransport::has_feature(const std::string& feature) const {
+    return features_.count(feature) > 0;
+}
+
+void atransport::add_feature(const std::string& feature) {
+    features_.insert(feature);
+}
+
+bool atransport::CanUseFeature(const std::string& feature) const {
+    return has_feature(feature) &&
+           supported_features().count(feature) > 0;
+}
+
 #if ADB_HOST
 
 static void append_transport_info(std::string* result, const char* key,
@@ -879,6 +903,9 @@ static void append_transport(const atransport* t, std::string* result,
         append_transport_info(result, "product:", t->product, false);
         append_transport_info(result, "model:", t->model, true);
         append_transport_info(result, "device:", t->device, false);
+        append_transport_info(result, "features:",
+                              android::base::Join(t->features(), ',').c_str(),
+                              false);
     }
     *result += '\n';
 }

adb/transport.h

@@ -20,9 +20,92 @@
 #include <sys/types.h>
 
 #include <string>
+#include <unordered_set>
 
 #include "adb.h"
 
+typedef std::unordered_set<std::string> FeatureSet;
+
+const FeatureSet& supported_features();
+
+class atransport {
+public:
+    // TODO(danalbert): We expose waaaaaaay too much stuff because this was
+    // historically just a struct, but making the whole thing a more idiomatic
+    // class in one go is a very large change. Given how bad our testing is,
+    // it's better to do this piece by piece.
+
+    atransport() {
+        auth_fde = {};
+        transport_fde = {};
+        protocol_version = A_VERSION;
+        max_payload = MAX_PAYLOAD;
+    }
+
+    virtual ~atransport() {}
+
+    int (*read_from_remote)(apacket* p, atransport* t) = nullptr;
+    int (*write_to_remote)(apacket* p, atransport* t) = nullptr;
+    void (*close)(atransport* t) = nullptr;
+    void (*kick)(atransport* t) = nullptr;
+
+    int fd = -1;
+    int transport_socket = -1;
+    fdevent transport_fde;
+    int ref_count = 0;
+    uint32_t sync_token = 0;
+    ConnectionState connection_state = kCsOffline;
+    bool online = false;
+    TransportType type = kTransportAny;
+
+    // USB handle or socket fd as needed.
+    usb_handle* usb = nullptr;
+    int sfd = -1;
+
+    // Used to identify transports for clients.
+    char* serial = nullptr;
+    char* product = nullptr;
+    char* model = nullptr;
+    char* device = nullptr;
+    char* devpath = nullptr;
+    int adb_port = -1;  // Use for emulators (local transport)
+    bool kicked = false;
+
+    // A list of adisconnect callbacks called when the transport is kicked.
+    adisconnect disconnects = {};
+
+    void* key = nullptr;
+    unsigned char token[TOKEN_SIZE] = {};
+    fdevent auth_fde;
+    size_t failed_auth_attempts = 0;
+
+    const char* connection_state_name() const;
+
+    void update_version(int version, size_t payload);
+    int get_protocol_version() const;
+    size_t get_max_payload() const;
+
+    inline const FeatureSet features() const {
+        return features_;
+    }
+
+    bool has_feature(const std::string& feature) const;
+    void add_feature(const std::string& feature);
+
+    // Returns true if both we and the other end of the transport support the
+    // feature.
+    bool CanUseFeature(const std::string& feature) const;
+
+private:
+    // A set of features transmitted in the banner with the initial connection.
+    // This is stored in the banner as 'features=feature0,feature1,etc'.
+    FeatureSet features_;
+    int protocol_version;
+    size_t max_payload;
+
+    DISALLOW_COPY_AND_ASSIGN(atransport);
+};
+
 /*
  * Obtain a transport from the available transports.
  * If state is != kCsAny, only transports in that state are considered.

adb/transport_test.cpp

@@ -59,6 +59,8 @@ public:
         EXPECT_EQ(0, memcmp(&auth_fde, &rhs.auth_fde, sizeof(fdevent)));
         EXPECT_EQ(failed_auth_attempts, rhs.failed_auth_attempts);
 
+        EXPECT_EQ(features(), rhs.features());
+
         return true;
     }
 };
@@ -120,6 +122,73 @@ TEST(transport, kick_transport_already_kicked) {
 // want to make sure I understand how this is working at all before I try fixing
 // that.
 TEST(transport, DISABLED_run_transport_disconnects_zeroed_atransport) {
-  atransport t;
+    atransport t;
-  run_transport_disconnects(&t);
+    run_transport_disconnects(&t);
+}
+
+TEST(transport, add_feature) {
+    atransport t;
+    ASSERT_EQ(0U, t.features().size());
+
+    t.add_feature("foo");
+    ASSERT_EQ(1U, t.features().size());
+    ASSERT_TRUE(t.has_feature("foo"));
+
+    t.add_feature("bar");
+    ASSERT_EQ(2U, t.features().size());
+    ASSERT_TRUE(t.has_feature("foo"));
+    ASSERT_TRUE(t.has_feature("bar"));
+
+    t.add_feature("foo");
+    ASSERT_EQ(2U, t.features().size());
+    ASSERT_TRUE(t.has_feature("foo"));
+    ASSERT_TRUE(t.has_feature("bar"));
+}
+
+TEST(transport, parse_banner_no_features) {
+    atransport t;
+
+    parse_banner("host::", &t);
+
+    ASSERT_EQ(0U, t.features().size());
+    ASSERT_EQ(kCsHost, t.connection_state);
+
+    ASSERT_EQ(nullptr, t.product);
+    ASSERT_EQ(nullptr, t.model);
+    ASSERT_EQ(nullptr, t.device);
+}
+
+TEST(transport, parse_banner_product_features) {
+    atransport t;
+
+    const char banner[] =
+        "host::ro.product.name=foo;ro.product.model=bar;ro.product.device=baz;";
+    parse_banner(banner, &t);
+
+    ASSERT_EQ(kCsHost, t.connection_state);
+
+    ASSERT_EQ(0U, t.features().size());
+
+    ASSERT_EQ(std::string("foo"), t.product);
+    ASSERT_EQ(std::string("bar"), t.model);
+    ASSERT_EQ(std::string("baz"), t.device);
+}
+
+TEST(transport, parse_banner_features) {
+    atransport t;
+
+    const char banner[] =
+        "host::ro.product.name=foo;ro.product.model=bar;ro.product.device=baz;"
+        "features=woodly,doodly";
+    parse_banner(banner, &t);
+
+    ASSERT_EQ(kCsHost, t.connection_state);
+
+    ASSERT_EQ(2U, t.features().size());
+    ASSERT_TRUE(t.has_feature("woodly"));
+    ASSERT_TRUE(t.has_feature("doodly"));
+
+    ASSERT_EQ(std::string("foo"), t.product);
+    ASSERT_EQ(std::string("bar"), t.model);
+    ASSERT_EQ(std::string("baz"), t.device);
 }