adb: remove fdevent_install, fdevent_remove.
Remove fdevent_install and fdevent_remove in favor of using
fdevent_create and fdevent_destroy, so that we can put RAII types (i.e.
unique_fd) into fdevent without worrying about -Wexit-time-destructors
or structs that are freed instead of deleted.
Bug: http://b/79786774
Test: python test_device.py
Change-Id: I8471cc00574ed492fe1b196944976cdaae8b7cff
diff --git a/adb/adb_listeners.cpp b/adb/adb_listeners.cpp
index fecf452..ea5a44e 100644
--- a/adb/adb_listeners.cpp
+++ b/adb/adb_listeners.cpp
@@ -42,7 +42,7 @@
alistener(const std::string& _local_name, const std::string& _connect_to);
~alistener();
- fdevent fde;
+ fdevent* fde = nullptr;
int fd = -1;
std::string local_name;
@@ -60,7 +60,7 @@
alistener::~alistener() {
// Closes the corresponding fd.
- fdevent_remove(&fde);
+ fdevent_destroy(fde);
if (transport) {
transport->RemoveDisconnect(&disconnect);
@@ -222,11 +222,11 @@
close_on_exec(listener->fd);
if (listener->connect_to == "*smartsocket*") {
- fdevent_install(&listener->fde, listener->fd, ss_listener_event_func, listener.get());
+ listener->fde = fdevent_create(listener->fd, ss_listener_event_func, listener.get());
} else {
- fdevent_install(&listener->fde, listener->fd, listener_event_func, listener.get());
+ listener->fde = fdevent_create(listener->fd, listener_event_func, listener.get());
}
- fdevent_set(&listener->fde, FDE_READ);
+ fdevent_set(listener->fde, FDE_READ);
listener->transport = transport;
diff --git a/adb/client/transport_mdns.cpp b/adb/client/transport_mdns.cpp
index 01f140a..283fac5 100644
--- a/adb/client/transport_mdns.cpp
+++ b/adb/client/transport_mdns.cpp
@@ -35,7 +35,7 @@
#include "sysdeps.h"
static DNSServiceRef service_ref;
-static fdevent service_ref_fde;
+static fdevent* service_ref_fde;
// Use adb_DNSServiceRefSockFD() instead of calling DNSServiceRefSockFD()
// directly so that the socket is put through the appropriate compatibility
@@ -68,27 +68,26 @@
}
virtual ~AsyncServiceRef() {
- if (! initialized_) {
+ if (!initialized_) {
return;
}
DNSServiceRefDeallocate(sdRef_);
- fdevent_remove(&fde_);
+ fdevent_destroy(fde_);
}
protected:
DNSServiceRef sdRef_;
void Initialize() {
- fdevent_install(&fde_, adb_DNSServiceRefSockFD(sdRef_),
- pump_service_ref, &sdRef_);
- fdevent_set(&fde_, FDE_READ);
+ fde_ = fdevent_create(adb_DNSServiceRefSockFD(sdRef_), pump_service_ref, &sdRef_);
+ fdevent_set(fde_, FDE_READ);
initialized_ = true;
}
private:
bool initialized_ = false;
- fdevent fde_;
+ fdevent* fde_;
};
class ResolvedService : public AsyncServiceRef {
@@ -252,14 +251,12 @@
if (errorCode != kDNSServiceErr_NoError) {
D("Got error %d during mDNS browse.", errorCode);
DNSServiceRefDeallocate(sdRef);
- fdevent_remove(&service_ref_fde);
+ fdevent_destroy(service_ref_fde);
return;
}
- auto discovered = new DiscoveredService(interfaceIndex, serviceName,
- regtype, domain);
-
- if (! discovered->Initialized()) {
+ auto discovered = new DiscoveredService(interfaceIndex, serviceName, regtype, domain);
+ if (!discovered->Initialized()) {
delete discovered;
}
}
@@ -274,9 +271,9 @@
}
fdevent_run_on_main_thread([]() {
- fdevent_install(&service_ref_fde, adb_DNSServiceRefSockFD(service_ref), pump_service_ref,
- &service_ref);
- fdevent_set(&service_ref_fde, FDE_READ);
+ service_ref_fde =
+ fdevent_create(adb_DNSServiceRefSockFD(service_ref), pump_service_ref, &service_ref);
+ fdevent_set(service_ref_fde, FDE_READ);
});
}
diff --git a/adb/daemon/auth.cpp b/adb/daemon/auth.cpp
index 3fd2b31..f0c3629 100644
--- a/adb/daemon/auth.cpp
+++ b/adb/daemon/auth.cpp
@@ -35,8 +35,8 @@
#include <openssl/rsa.h>
#include <openssl/sha.h>
-static fdevent listener_fde;
-static fdevent framework_fde;
+static fdevent* listener_fde = nullptr;
+static fdevent* framework_fde = nullptr;
static int framework_fd = -1;
static void usb_disconnected(void* unused, atransport* t);
@@ -106,8 +106,10 @@
static void framework_disconnected() {
LOG(INFO) << "Framework disconnect";
- fdevent_remove(&framework_fde);
- framework_fd = -1;
+ if (framework_fde) {
+ fdevent_destroy(framework_fde);
+ framework_fd = -1;
+ }
}
static void adbd_auth_event(int fd, unsigned events, void*) {
@@ -168,8 +170,8 @@
}
framework_fd = s;
- fdevent_install(&framework_fde, framework_fd, adbd_auth_event, nullptr);
- fdevent_add(&framework_fde, FDE_READ);
+ framework_fde = fdevent_create(framework_fd, adbd_auth_event, nullptr);
+ fdevent_add(framework_fde, FDE_READ);
if (needs_retry) {
needs_retry = false;
@@ -198,8 +200,8 @@
return;
}
- fdevent_install(&listener_fde, fd, adbd_auth_listener, NULL);
- fdevent_add(&listener_fde, FDE_READ);
+ listener_fde = fdevent_create(fd, adbd_auth_listener, NULL);
+ fdevent_add(listener_fde, FDE_READ);
}
void send_auth_request(atransport* t) {
diff --git a/adb/fdevent.cpp b/adb/fdevent.cpp
index cf441cf..b3ff457 100644
--- a/adb/fdevent.cpp
+++ b/adb/fdevent.cpp
@@ -117,29 +117,17 @@
return android::base::StringPrintf("(fdevent %d %s)", fde->fd, state.c_str());
}
-fdevent* fdevent_create(int fd, fd_func func, void* arg) {
- check_main_thread();
- fdevent *fde = (fdevent*) malloc(sizeof(fdevent));
- if(fde == 0) return 0;
- fdevent_install(fde, fd, func, arg);
- fde->state |= FDE_CREATED;
- return fde;
-}
-
-void fdevent_destroy(fdevent* fde) {
- check_main_thread();
- if(fde == 0) return;
- if(!(fde->state & FDE_CREATED)) {
- LOG(FATAL) << "destroying fde not created by fdevent_create(): " << dump_fde(fde);
- }
- fdevent_remove(fde);
- free(fde);
-}
-
void fdevent_install(fdevent* fde, int fd, fd_func func, void* arg) {
check_main_thread();
CHECK_GE(fd, 0);
memset(fde, 0, sizeof(fdevent));
+}
+
+fdevent* fdevent_create(int fd, fd_func func, void* arg) {
+ check_main_thread();
+ CHECK_GE(fd, 0);
+
+ fdevent* fde = new fdevent();
fde->state = FDE_ACTIVE;
fde->fd = fd;
fde->func = func;
@@ -152,12 +140,18 @@
}
auto pair = g_poll_node_map.emplace(fde->fd, PollNode(fde));
CHECK(pair.second) << "install existing fd " << fd;
- D("fdevent_install %s", dump_fde(fde).c_str());
+
+ fde->state |= FDE_CREATED;
+ return fde;
}
-void fdevent_remove(fdevent* fde) {
+void fdevent_destroy(fdevent* fde) {
check_main_thread();
- D("fdevent_remove %s", dump_fde(fde).c_str());
+ if (fde == 0) return;
+ if (!(fde->state & FDE_CREATED)) {
+ LOG(FATAL) << "destroying fde not created by fdevent_create(): " << dump_fde(fde);
+ }
+
if (fde->state & FDE_ACTIVE) {
g_poll_node_map.erase(fde->fd);
if (fde->state & FDE_PENDING) {
@@ -170,6 +164,8 @@
fde->state = 0;
fde->events = 0;
}
+
+ delete fde;
}
static void fdevent_update(fdevent* fde, unsigned events) {
diff --git a/adb/fdevent.h b/adb/fdevent.h
index 896400a..2d8efd8 100644
--- a/adb/fdevent.h
+++ b/adb/fdevent.h
@@ -33,17 +33,17 @@
typedef void (*fd_func)(int fd, unsigned events, void *userdata);
struct fdevent {
- fdevent *next;
- fdevent *prev;
+ fdevent* next = nullptr;
+ fdevent* prev = nullptr;
- int fd;
- int force_eof;
+ int fd = -1;
+ int force_eof = 0;
- uint16_t state;
- uint16_t events;
+ uint16_t state = 0;
+ uint16_t events = 0;
- fd_func func;
- void *arg;
+ fd_func func = nullptr;
+ void* arg = nullptr;
};
/* Allocate and initialize a new fdevent object
@@ -57,15 +57,6 @@
*/
void fdevent_destroy(fdevent *fde);
-/* Initialize an fdevent object that was externally allocated
-*/
-void fdevent_install(fdevent *fde, int fd, fd_func func, void *arg);
-
-/* Uninitialize an fdevent object that was initialized by
-** fdevent_install()
-*/
-void fdevent_remove(fdevent *item);
-
/* Change which events should cause notifications
*/
void fdevent_set(fdevent *fde, unsigned events);
diff --git a/adb/fdevent_test.cpp b/adb/fdevent_test.cpp
index 2f0ff18..0cb2439 100644
--- a/adb/fdevent_test.cpp
+++ b/adb/fdevent_test.cpp
@@ -31,14 +31,14 @@
class FdHandler {
public:
FdHandler(int read_fd, int write_fd) : read_fd_(read_fd), write_fd_(write_fd) {
- fdevent_install(&read_fde_, read_fd_, FdEventCallback, this);
- fdevent_add(&read_fde_, FDE_READ);
- fdevent_install(&write_fde_, write_fd_, FdEventCallback, this);
+ read_fde_ = fdevent_create(read_fd_, FdEventCallback, this);
+ fdevent_add(read_fde_, FDE_READ);
+ write_fde_ = fdevent_create(write_fd_, FdEventCallback, this);
}
~FdHandler() {
- fdevent_remove(&read_fde_);
- fdevent_remove(&write_fde_);
+ fdevent_destroy(read_fde_);
+ fdevent_destroy(write_fde_);
}
private:
@@ -50,7 +50,7 @@
char c;
ASSERT_EQ(1, adb_read(fd, &c, 1));
handler->queue_.push(c);
- fdevent_add(&handler->write_fde_, FDE_WRITE);
+ fdevent_add(handler->write_fde_, FDE_WRITE);
}
if (events & FDE_WRITE) {
ASSERT_EQ(fd, handler->write_fd_);
@@ -59,7 +59,7 @@
handler->queue_.pop();
ASSERT_EQ(1, adb_write(fd, &c, 1));
if (handler->queue_.empty()) {
- fdevent_del(&handler->write_fde_, FDE_WRITE);
+ fdevent_del(handler->write_fde_, FDE_WRITE);
}
}
}
@@ -67,8 +67,8 @@
private:
const int read_fd_;
const int write_fd_;
- fdevent read_fde_;
- fdevent write_fde_;
+ fdevent* read_fde_;
+ fdevent* write_fde_;
std::queue<char> queue_;
};
@@ -137,7 +137,7 @@
}
struct InvalidFdArg {
- fdevent fde;
+ fdevent* fde;
unsigned expected_events;
size_t* happened_event_count;
};
@@ -145,7 +145,7 @@
static void InvalidFdEventCallback(int, unsigned events, void* userdata) {
InvalidFdArg* arg = reinterpret_cast<InvalidFdArg*>(userdata);
ASSERT_EQ(arg->expected_events, events);
- fdevent_remove(&arg->fde);
+ fdevent_destroy(arg->fde);
if (++*(arg->happened_event_count) == 2) {
fdevent_terminate_loop();
}
@@ -157,15 +157,15 @@
InvalidFdArg read_arg;
read_arg.expected_events = FDE_READ | FDE_ERROR;
read_arg.happened_event_count = &happened_event_count;
- fdevent_install(&read_arg.fde, INVALID_READ_FD, InvalidFdEventCallback, &read_arg);
- fdevent_add(&read_arg.fde, FDE_READ);
+ read_arg.fde = fdevent_create(INVALID_READ_FD, InvalidFdEventCallback, &read_arg);
+ fdevent_add(read_arg.fde, FDE_READ);
const int INVALID_WRITE_FD = std::numeric_limits<int>::max();
InvalidFdArg write_arg;
write_arg.expected_events = FDE_READ | FDE_ERROR;
write_arg.happened_event_count = &happened_event_count;
- fdevent_install(&write_arg.fde, INVALID_WRITE_FD, InvalidFdEventCallback, &write_arg);
- fdevent_add(&write_arg.fde, FDE_WRITE);
+ write_arg.fde = fdevent_create(INVALID_WRITE_FD, InvalidFdEventCallback, &write_arg);
+ fdevent_add(write_arg.fde, FDE_WRITE);
fdevent_loop();
}
diff --git a/adb/socket.h b/adb/socket.h
index e0fd87f..27e5b05 100644
--- a/adb/socket.h
+++ b/adb/socket.h
@@ -58,8 +58,8 @@
* us to our fd event system. For remote asockets
* these fields are not used.
*/
- fdevent fde = {};
- int fd = 0;
+ fdevent* fde = nullptr;
+ int fd = -1;
// queue of data waiting to be written
std::deque<Range> packet_queue;
diff --git a/adb/sockets.cpp b/adb/sockets.cpp
index 7bc0165..4f4fbfb 100644
--- a/adb/sockets.cpp
+++ b/adb/sockets.cpp
@@ -121,10 +121,10 @@
s->packet_queue.pop_front();
} else if (rc > 0) {
r.drop_front(rc);
- fdevent_add(&s->fde, FDE_WRITE);
+ fdevent_add(s->fde, FDE_WRITE);
return SocketFlushResult::TryAgain;
} else if (rc == -1 && errno == EAGAIN) {
- fdevent_add(&s->fde, FDE_WRITE);
+ fdevent_add(s->fde, FDE_WRITE);
return SocketFlushResult::TryAgain;
} else {
// We failed to write, but it's possible that we can still read from the socket.
@@ -140,7 +140,7 @@
return SocketFlushResult::Destroyed;
}
- fdevent_del(&s->fde, FDE_WRITE);
+ fdevent_del(s->fde, FDE_WRITE);
return SocketFlushResult::Completed;
}
@@ -173,7 +173,7 @@
break;
}
D("LS(%d): fd=%d post avail loop. r=%d is_eof=%d forced_eof=%d", s->id, s->fd, r, is_eof,
- s->fde.force_eof);
+ s->fde->force_eof);
if (avail != max_payload && s->peer) {
data.resize(max_payload - avail);
@@ -200,13 +200,13 @@
** we disable notification of READs. They'll
** be enabled again when we get a call to ready()
*/
- fdevent_del(&s->fde, FDE_READ);
+ fdevent_del(s->fde, FDE_READ);
}
}
// Don't allow a forced eof if data is still there.
- if ((s->fde.force_eof && !r) || is_eof) {
- D(" closing because is_eof=%d r=%d s->fde.force_eof=%d", is_eof, r, s->fde.force_eof);
+ if ((s->fde->force_eof && !r) || is_eof) {
+ D(" closing because is_eof=%d r=%d s->fde.force_eof=%d", is_eof, r, s->fde->force_eof);
s->close(s);
return false;
}
@@ -236,19 +236,19 @@
static void local_socket_ready(asocket* s) {
/* far side is ready for data, pay attention to
readable events */
- fdevent_add(&s->fde, FDE_READ);
+ fdevent_add(s->fde, FDE_READ);
}
// be sure to hold the socket list lock when calling this
static void local_socket_destroy(asocket* s) {
int exit_on_close = s->exit_on_close;
- D("LS(%d): destroying fde.fd=%d", s->id, s->fde.fd);
+ D("LS(%d): destroying fde.fd=%d", s->id, s->fd);
/* IMPORTANT: the remove closes the fd
** that belongs to this socket
*/
- fdevent_remove(&s->fde);
+ fdevent_destroy(s->fde);
remove_socket(s);
delete s;
@@ -290,11 +290,11 @@
*/
D("LS(%d): closing", s->id);
s->closing = 1;
- fdevent_del(&s->fde, FDE_READ);
+ fdevent_del(s->fde, FDE_READ);
remove_socket(s);
D("LS(%d): put on socket_closing_list fd=%d", s->id, s->fd);
local_socket_closing_list.push_back(s);
- CHECK_EQ(FDE_WRITE, s->fde.state & FDE_WRITE);
+ CHECK_EQ(FDE_WRITE, s->fde->state & FDE_WRITE);
}
static void local_socket_event_func(int fd, unsigned ev, void* _s) {
@@ -343,7 +343,7 @@
s->close = local_socket_close;
install_local_socket(s);
- fdevent_install(&s->fde, fd, local_socket_event_func, s);
+ s->fde = fdevent_create(fd, local_socket_event_func, s);
D("LS(%d): created (fd=%d)", s->id, s->fd);
return s;
}
diff --git a/adb/transport.cpp b/adb/transport.cpp
index fa7cc8c..4c9c90c 100644
--- a/adb/transport.cpp
+++ b/adb/transport.cpp
@@ -300,7 +300,7 @@
static int transport_registration_send = -1;
static int transport_registration_recv = -1;
-static fdevent transport_registration_fde;
+static fdevent* transport_registration_fde;
#if ADB_HOST
@@ -565,10 +565,9 @@
transport_registration_send = s[0];
transport_registration_recv = s[1];
- fdevent_install(&transport_registration_fde, transport_registration_recv,
- transport_registration_func, 0);
-
- fdevent_set(&transport_registration_fde, FDE_READ);
+ transport_registration_fde =
+ fdevent_create(transport_registration_recv, transport_registration_func, 0);
+ fdevent_set(transport_registration_fde, FDE_READ);
}
void kick_all_transports() {