Make atransport be a real class.

Using non-POD types in atransport means we'll need to start treating
it as a real class (specifically with regards to new/delete rather
than malloc/free).

I've also cleaned up the home grown linked lists for transport_list
and pending_list to just be std::lists. We might want to refactor that
again to be an std::unordered_map keyed on serial, since that seems to
be a common way to search it.

Change-Id: I7f5e23cdc47944a9278099723ca029585fe52105
This commit is contained in:
Dan Albert 2015-05-18 16:46:31 -07:00
parent 02b29c49d4
commit c7915a3470
4 changed files with 177 additions and 147 deletions

View file

@ -16,6 +16,7 @@ adb_version := $(shell git -C $(LOCAL_PATH) rev-parse --short=12 HEAD 2>/dev/nul
ADB_COMMON_CFLAGS := \
-Wall -Werror \
-Wno-unused-parameter \
-Wno-missing-field-initializers \
-DADB_REVISION='"$(adb_version)"' \
# libadb
@ -45,7 +46,6 @@ LIBADB_TEST_SRCS := \
LIBADB_CFLAGS := \
$(ADB_COMMON_CFLAGS) \
-Wno-missing-field-initializers \
-fvisibility=hidden \
LIBADB_darwin_SRC_FILES := \

103
adb/adb.h
View file

@ -20,6 +20,8 @@
#include <limits.h>
#include <sys/types.h>
#include <base/macros.h>
#include "adb_trace.h"
#include "fdevent.h"
@ -43,7 +45,7 @@
// Increment this when we want to force users to start a new adb server.
#define ADB_SERVER_VERSION 32
struct atransport;
class atransport;
struct usb_handle;
struct amessage {
@ -152,20 +154,19 @@ struct adisconnect
};
/* a transport object models the connection to a remote device or emulator
** there is one transport per connected device/emulator. a "local transport"
** connects through TCP (for the emulator), while a "usb transport" through
** USB (for real devices)
**
** note that kTransportHost doesn't really correspond to a real transport
** object, it's a special value used to indicate that a client wants to
** connect to a service implemented within the ADB server itself.
*/
// A transport object models the connection to a remote device or emulator there
// is one transport per connected device/emulator. A "local transport" connects
// through TCP (for the emulator), while a "usb transport" through USB (for real
// devices).
//
// Note that kTransportHost doesn't really correspond to a real transport
// object, it's a special value used to indicate that a client wants to connect
// to a service implemented within the ADB server itself.
enum TransportType {
kTransportUsb,
kTransportLocal,
kTransportAny,
kTransportHost,
kTransportUsb,
kTransportLocal,
kTransportAny,
kTransportHost,
};
#define TOKEN_SIZE 20
@ -182,47 +183,59 @@ enum ConnectionState {
kCsUnauthorized,
};
struct atransport
{
atransport *next;
atransport *prev;
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.
int (*read_from_remote)(apacket *p, atransport *t);
int (*write_to_remote)(apacket *p, atransport *t);
void (*close)(atransport *t);
void (*kick)(atransport *t);
atransport() {
auth_fde = {};
transport_fde = {};
}
int fd;
int transport_socket;
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;
unsigned sync_token;
int connection_state;
int online;
TransportType type;
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;
int sfd;
// USB handle or socket fd as needed.
usb_handle* usb = nullptr;
int sfd = -1;
/* used to identify transports for clients */
char *serial;
char *product;
char *model;
char *device;
char *devpath;
int adb_port; // Use for emulators (local transport)
// 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 */
int kicked;
adisconnect disconnects;
// A list of adisconnect callbacks called when the transport is kicked.
adisconnect disconnects = {};
void *key;
unsigned char token[TOKEN_SIZE];
void* key = nullptr;
unsigned char token[TOKEN_SIZE] = {};
fdevent auth_fde;
unsigned failed_auth_attempts;
size_t failed_auth_attempts = 0;
const char* connection_state_name() const;
private:
DISALLOW_COPY_AND_ASSIGN(atransport);
};

View file

@ -26,6 +26,8 @@
#include <string.h>
#include <unistd.h>
#include <list>
#include <base/stringprintf.h>
#include "adb.h"
@ -33,15 +35,8 @@
static void transport_unref(atransport *t);
static atransport transport_list = {
.next = &transport_list,
.prev = &transport_list,
};
static atransport pending_list = {
.next = &pending_list,
.prev = &pending_list,
};
static std::list<atransport*> transport_list;
static std::list<atransport*> pending_list;
ADB_MUTEX_DEFINE( transport_lock );
@ -553,8 +548,7 @@ static void transport_registration_func(int _fd, unsigned ev, void *data)
adb_close(t->fd);
adb_mutex_lock(&transport_lock);
t->next->prev = t->prev;
t->prev->next = t->next;
transport_list.remove(t);
adb_mutex_unlock(&transport_lock);
run_transport_disconnects(t);
@ -570,8 +564,7 @@ static void transport_registration_func(int _fd, unsigned ev, void *data)
if (t->devpath)
free(t->devpath);
memset(t,0xee,sizeof(atransport));
free(t);
delete t;
update_transports();
return;
@ -582,7 +575,7 @@ static void transport_registration_func(int _fd, unsigned ev, void *data)
/* initial references are the two threads */
t->ref_count = 2;
if(adb_socketpair(s)) {
if (adb_socketpair(s)) {
fatal_errno("cannot open transport socketpair");
}
@ -608,14 +601,8 @@ static void transport_registration_func(int _fd, unsigned ev, void *data)
}
adb_mutex_lock(&transport_lock);
/* remove from pending list */
t->next->prev = t->prev;
t->prev->next = t->next;
/* put us on the master device list */
t->next = &transport_list;
t->prev = transport_list.prev;
t->next->prev = t;
t->prev->next = t;
pending_list.remove(t);
transport_list.push_front(t);
adb_mutex_unlock(&transport_lock);
t->disconnects.next = t->disconnects.prev = &t->disconnects;
@ -740,7 +727,6 @@ static int qual_match(const char *to_test,
atransport* acquire_one_transport(ConnectionState state, TransportType type,
const char* serial, std::string* error_out) {
atransport *t;
atransport *result = NULL;
int ambiguous = 0;
@ -748,7 +734,7 @@ retry:
if (error_out) *error_out = android::base::StringPrintf("device '%s' not found", serial);
adb_mutex_lock(&transport_lock);
for (t = transport_list.next; t != &transport_list; t = t->next) {
for (auto t : transport_list) {
if (t->connection_state == kCsNoPerm) {
if (error_out) *error_out = "insufficient permissions for device";
continue;
@ -866,7 +852,8 @@ static void append_transport_info(std::string* result, const char* key,
}
}
static void append_transport(atransport* t, std::string* result, bool long_listing) {
static void append_transport(const atransport* t, std::string* result,
bool long_listing) {
const char* serial = t->serial;
if (!serial || !serial[0]) {
serial = "(no serial number)";
@ -890,7 +877,7 @@ static void append_transport(atransport* t, std::string* result, bool long_listi
std::string list_transports(bool long_listing) {
std::string result;
adb_mutex_lock(&transport_lock);
for (atransport* t = transport_list.next; t != &transport_list; t = t->next) {
for (const auto t : transport_list) {
append_transport(t, &result, long_listing);
}
adb_mutex_unlock(&transport_lock);
@ -898,11 +885,10 @@ std::string list_transports(bool long_listing) {
}
/* hack for osx */
void close_usb_devices()
{
void close_usb_devices() {
adb_mutex_lock(&transport_lock);
for (atransport* t = transport_list.next; t != &transport_list; t = t->next) {
if ( !t->kicked ) {
for (auto t : transport_list) {
if (!t->kicked) {
t->kicked = 1;
t->kick(t);
}
@ -911,47 +897,39 @@ void close_usb_devices()
}
#endif // ADB_HOST
int register_socket_transport(int s, const char *serial, int port, int local)
{
atransport *t = reinterpret_cast<atransport*>(calloc(1, sizeof(atransport)));
if (t == nullptr) {
return -1;
}
atransport *n;
char buff[32];
int register_socket_transport(int s, const char *serial, int port, int local) {
atransport* t = new atransport();
if (!serial) {
snprintf(buff, sizeof buff, "T-%p", t);
serial = buff;
char buf[32];
snprintf(buf, sizeof(buf), "T-%p", t);
serial = buf;
}
D("transport: %s init'ing for socket %d, on port %d\n", serial, s, port);
if (init_socket_transport(t, s, port, local) < 0) {
free(t);
delete t;
return -1;
}
adb_mutex_lock(&transport_lock);
for (n = pending_list.next; n != &pending_list; n = n->next) {
if (n->serial && !strcmp(serial, n->serial)) {
for (auto transport : pending_list) {
if (transport->serial && strcmp(serial, transport->serial) == 0) {
adb_mutex_unlock(&transport_lock);
free(t);
delete t;
return -1;
}
}
for (n = transport_list.next; n != &transport_list; n = n->next) {
if (n->serial && !strcmp(serial, n->serial)) {
for (auto transport : transport_list) {
if (transport->serial && strcmp(serial, transport->serial) == 0) {
adb_mutex_unlock(&transport_lock);
free(t);
delete t;
return -1;
}
}
t->next = &pending_list;
t->prev = pending_list.prev;
t->next->prev = t;
t->prev->next = t;
pending_list.push_front(t);
t->serial = strdup(serial);
adb_mutex_unlock(&transport_lock);
@ -960,95 +938,83 @@ int register_socket_transport(int s, const char *serial, int port, int local)
}
#if ADB_HOST
atransport *find_transport(const char *serial)
{
atransport *t;
atransport *find_transport(const char *serial) {
atransport* result = nullptr;
adb_mutex_lock(&transport_lock);
for(t = transport_list.next; t != &transport_list; t = t->next) {
if (t->serial && !strcmp(serial, t->serial)) {
for (auto t : transport_list) {
if (t->serial && strcmp(serial, t->serial) == 0) {
result = t;
break;
}
}
}
adb_mutex_unlock(&transport_lock);
if (t != &transport_list)
return t;
else
return 0;
return result;
}
void unregister_transport(atransport *t)
{
adb_mutex_lock(&transport_lock);
t->next->prev = t->prev;
t->prev->next = t->next;
transport_list.remove(t);
adb_mutex_unlock(&transport_lock);
kick_transport(t);
transport_unref(t);
}
// unregisters all non-emulator TCP transports
void unregister_all_tcp_transports()
{
atransport *t, *next;
// Unregisters all non-emulator TCP transports.
void unregister_all_tcp_transports() {
adb_mutex_lock(&transport_lock);
for (t = transport_list.next; t != &transport_list; t = next) {
next = t->next;
for (auto it = transport_list.begin(); it != transport_list.end(); ) {
atransport* t = *it;
if (t->type == kTransportLocal && t->adb_port == 0) {
t->next->prev = t->prev;
t->prev->next = next;
// we cannot call kick_transport when holding transport_lock
if (!t->kicked)
{
// We cannot call kick_transport when holding transport_lock.
if (!t->kicked) {
t->kicked = 1;
t->kick(t);
}
transport_unref_locked(t);
it = transport_list.erase(it);
} else {
++it;
}
}
}
adb_mutex_unlock(&transport_lock);
}
#endif
void register_usb_transport(usb_handle *usb, const char *serial, const char *devpath, unsigned writeable)
{
atransport *t = reinterpret_cast<atransport*>(calloc(1, sizeof(atransport)));
if (t == nullptr) fatal("cannot allocate USB atransport");
void register_usb_transport(usb_handle* usb, const char* serial,
const char* devpath, unsigned writeable) {
atransport* t = new atransport();
D("transport: %p init'ing for usb_handle %p (sn='%s')\n", t, usb,
serial ? serial : "");
init_usb_transport(t, usb, (writeable ? kCsOffline : kCsNoPerm));
if(serial) {
t->serial = strdup(serial);
}
if(devpath) {
if (devpath) {
t->devpath = strdup(devpath);
}
adb_mutex_lock(&transport_lock);
t->next = &pending_list;
t->prev = pending_list.prev;
t->next->prev = t;
t->prev->next = t;
pending_list.push_front(t);
adb_mutex_unlock(&transport_lock);
register_transport(t);
}
// This should only be used for transports with connection_state == kCsNoPerm.
void unregister_usb_transport(usb_handle* usb) {
void unregister_usb_transport(usb_handle *usb) {
adb_mutex_lock(&transport_lock);
for (atransport* t = transport_list.next; t != &transport_list;
t = t->next) {
if (t->usb == usb && t->connection_state == kCsNoPerm) {
t->next->prev = t->prev;
t->prev->next = t->next;
break;
}
}
transport_list.remove_if([usb](atransport* t) {
return t->usb == usb && t->connection_state == kCsNoPerm;
});
adb_mutex_unlock(&transport_lock);
}

View file

@ -20,34 +20,85 @@
#include "adb.h"
class TestTransport : public atransport {
public:
bool operator==(const atransport& rhs) const {
EXPECT_EQ(read_from_remote, rhs.read_from_remote);
EXPECT_EQ(write_to_remote, rhs.write_to_remote);
EXPECT_EQ(close, rhs.close);
EXPECT_EQ(kick, rhs.kick);
EXPECT_EQ(fd, rhs.fd);
EXPECT_EQ(transport_socket, rhs.transport_socket);
EXPECT_EQ(
0, memcmp(&transport_fde, &rhs.transport_fde, sizeof(fdevent)));
EXPECT_EQ(ref_count, rhs.ref_count);
EXPECT_EQ(sync_token, rhs.sync_token);
EXPECT_EQ(connection_state, rhs.connection_state);
EXPECT_EQ(online, rhs.online);
EXPECT_EQ(type, rhs.type);
EXPECT_EQ(usb, rhs.usb);
EXPECT_EQ(sfd, rhs.sfd);
EXPECT_EQ(serial, rhs.serial);
EXPECT_EQ(product, rhs.product);
EXPECT_EQ(model, rhs.model);
EXPECT_EQ(device, rhs.device);
EXPECT_EQ(devpath, rhs.devpath);
EXPECT_EQ(adb_port, rhs.adb_port);
EXPECT_EQ(kicked, rhs.kicked);
EXPECT_EQ(
0, memcmp(&disconnects, &rhs.disconnects, sizeof(adisconnect)));
EXPECT_EQ(key, rhs.key);
EXPECT_EQ(0, memcmp(token, rhs.token, TOKEN_SIZE));
EXPECT_EQ(0, memcmp(&auth_fde, &rhs.auth_fde, sizeof(fdevent)));
EXPECT_EQ(failed_auth_attempts, rhs.failed_auth_attempts);
return true;
}
};
TEST(transport, kick_transport) {
atransport t = {};
TestTransport t;
// Mutate some member so we can test that the function is run.
t.kick = [](atransport* trans) { trans->fd = 42; };
atransport expected = t;
TestTransport expected;
expected.kick = t.kick;
expected.fd = 42;
expected.kicked = 1;
kick_transport(&t);
ASSERT_EQ(42, t.fd);
ASSERT_EQ(1, t.kicked);
ASSERT_EQ(0, memcmp(&expected, &t, sizeof(atransport)));
ASSERT_EQ(expected, t);
}
TEST(transport, kick_transport_already_kicked) {
// Ensure that the transport is not modified if the transport has already been
// kicked.
atransport t = {};
TestTransport t;
t.kicked = 1;
t.kick = [](atransport*) { FAIL() << "Kick should not have been called"; };
atransport expected = t;
TestTransport expected;
expected.kicked = 1;
expected.kick = t.kick;
kick_transport(&t);
ASSERT_EQ(0, memcmp(&expected, &t, sizeof(atransport)));
ASSERT_EQ(expected, t);
}
// Disabled because the function currently segfaults for a zeroed atransport. I
// 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);
}