Merge changes I465804fd,Ib5a684bb,If5e66570,I8471cc00,I8ba0a70a

* changes:
  adb: convert fdevent over to unique_fd.
  adb: move towards using unique_fd.
  adb: delete FDEVENT_DONTCLOSE.
  adb: remove fdevent_install, fdevent_remove.
  adb: fix uninitialized variable in AsyncServiceRef.
This commit is contained in:
Josh Gao 2018-05-23 00:01:29 +00:00 committed by Gerrit Code Review
commit 7a223584c5
12 changed files with 120 additions and 133 deletions

View file

@ -885,9 +885,8 @@ int launch_server(const std::string& socket_spec) {
}
#else /* !defined(_WIN32) */
// set up a pipe so the child can tell us when it is ready.
// fd[0] will be parent's end, and the child will write on fd[1]
int fd[2];
if (pipe(fd)) {
unique_fd pipe_read, pipe_write;
if (!Pipe(&pipe_read, &pipe_write)) {
fprintf(stderr, "pipe failed in launch_server, errno: %d\n", errno);
return -1;
}
@ -899,11 +898,10 @@ int launch_server(const std::string& socket_spec) {
if (pid == 0) {
// child side of the fork
adb_close(fd[0]);
pipe_read.reset();
char reply_fd[30];
snprintf(reply_fd, sizeof(reply_fd), "%d", fd[1]);
snprintf(reply_fd, sizeof(reply_fd), "%d", pipe_write.get());
// child process
int result = execl(path.c_str(), "adb", "-L", socket_spec.c_str(), "fork-server", "server",
"--reply-fd", reply_fd, NULL);
@ -913,10 +911,10 @@ int launch_server(const std::string& socket_spec) {
// parent side of the fork
char temp[3] = {};
// wait for the "OK\n" message
adb_close(fd[1]);
int ret = adb_read(fd[0], temp, 3);
pipe_write.reset();
int ret = adb_read(pipe_read.get(), temp, 3);
int saved_errno = errno;
adb_close(fd[0]);
pipe_read.reset();
if (ret < 0) {
fprintf(stderr, "could not read ok from ADB Server, errno = %d\n", saved_errno);
return -1;

View file

@ -42,7 +42,7 @@ class alistener {
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(const std::string& _local_name, const std::string& _connect
alistener::~alistener() {
// Closes the corresponding fd.
fdevent_remove(&fde);
fdevent_destroy(fde);
if (transport) {
transport->RemoveDisconnect(&disconnect);
@ -222,11 +222,11 @@ InstallStatus install_listener(const std::string& local_name, const char* connec
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;

View file

@ -16,6 +16,8 @@
#pragma once
#include <unistd.h>
#include <android-base/unique_fd.h>
// Helper to automatically close an FD when it goes out of scope.
@ -24,3 +26,15 @@ struct AdbCloser {
};
using unique_fd = android::base::unique_fd_impl<AdbCloser>;
#if !defined(_WIN32)
inline bool Pipe(unique_fd* read, unique_fd* write) {
int pipefd[2];
if (pipe(pipefd) != 0) {
return false;
}
read->reset(pipefd[0]);
write->reset(pipefd[1]);
return true;
}
#endif

View file

@ -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 @@ class AsyncServiceRef {
}
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_;
fdevent fde_;
bool initialized_ = false;
fdevent* fde_;
};
class ResolvedService : public AsyncServiceRef {
@ -252,14 +251,12 @@ static void DNSSD_API register_mdns_transport(DNSServiceRef sdRef,
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 @@ void init_mdns_transport_discovery_thread(void) {
}
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);
});
}

View file

@ -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 usb_disconnected(void* unused, atransport* t) {
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 @@ static void adbd_auth_listener(int fd, unsigned events, void* data) {
}
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 @@ void adbd_auth_init(void) {
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) {

View file

@ -139,8 +139,6 @@ struct JdwpProcess {
fatal("could not create fdevent for new JDWP process");
}
this->fde->state |= FDE_DONT_CLOSE;
/* start by waiting for the PID */
fdevent_add(this->fde, FDE_READ);
}
@ -148,7 +146,6 @@ struct JdwpProcess {
~JdwpProcess() {
if (this->socket >= 0) {
adb_shutdown(this->socket);
adb_close(this->socket);
this->socket = -1;
}

View file

@ -57,7 +57,7 @@ struct PollNode {
explicit PollNode(fdevent* fde) : fde(fde) {
memset(&pollfd, 0, sizeof(pollfd));
pollfd.fd = fde->fd;
pollfd.fd = fde->fd.get();
#if defined(__linux__)
// Always enable POLLRDHUP, so the host server can take action when some clients disconnect.
@ -111,37 +111,22 @@ static std::string dump_fde(const fdevent* fde) {
if (fde->state & FDE_ERROR) {
state += "E";
}
if (fde->state & FDE_DONT_CLOSE) {
state += "D";
}
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);
return android::base::StringPrintf("(fdevent %d %s)", fde->fd.get(), state.c_str());
}
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->fd.reset(fd);
fde->func = func;
fde->arg = arg;
if (!set_file_block_mode(fd, false)) {
@ -150,30 +135,35 @@ void fdevent_install(fdevent* fde, int fd, fd_func func, void* arg) {
// to handle it.
LOG(ERROR) << "failed to set non-blocking mode for fd " << fd;
}
auto pair = g_poll_node_map.emplace(fde->fd, PollNode(fde));
auto pair = g_poll_node_map.emplace(fde->fd.get(), 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);
g_poll_node_map.erase(fde->fd.get());
if (fde->state & FDE_PENDING) {
g_pending_list.remove(fde);
}
if (!(fde->state & FDE_DONT_CLOSE)) {
adb_close(fde->fd);
fde->fd = -1;
}
fde->fd.reset();
fde->state = 0;
fde->events = 0;
}
delete fde;
}
static void fdevent_update(fdevent* fde, unsigned events) {
auto it = g_poll_node_map.find(fde->fd);
auto it = g_poll_node_map.find(fde->fd.get());
CHECK(it != g_poll_node_map.end());
PollNode& node = it->second;
if (events & FDE_READ) {
@ -272,7 +262,7 @@ static void fdevent_process() {
auto it = g_poll_node_map.find(pollfd.fd);
CHECK(it != g_poll_node_map.end());
fdevent* fde = it->second.fde;
CHECK_EQ(fde->fd, pollfd.fd);
CHECK_EQ(fde->fd.get(), pollfd.fd);
fde->events |= events;
D("%s got events %x", dump_fde(fde).c_str(), events);
fde->state |= FDE_PENDING;
@ -287,7 +277,7 @@ static void fdevent_call_fdfunc(fdevent* fde) {
CHECK(fde->state & FDE_PENDING);
fde->state &= (~FDE_PENDING);
D("fdevent_call_fdfunc %s", dump_fde(fde).c_str());
fde->func(fde->fd, events, fde->arg);
fde->func(fde->fd.get(), events, fde->arg);
}
static void fdevent_run_flush() EXCLUDES(run_queue_mutex) {

View file

@ -22,28 +22,27 @@
#include <functional>
#include "adb_unique_fd.h"
/* events that may be observed */
#define FDE_READ 0x0001
#define FDE_WRITE 0x0002
#define FDE_ERROR 0x0004
/* features that may be set (via the events set/add/del interface) */
#define FDE_DONT_CLOSE 0x0080
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;
unique_fd fd;
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 +56,6 @@ fdevent *fdevent_create(int fd, fd_func func, void *arg);
*/
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);

View file

@ -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 @@ class FdHandler {
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 @@ class FdHandler {
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 @@ class FdHandler {
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 @@ TEST_F(FdeventTest, smoke) {
}
struct InvalidFdArg {
fdevent fde;
fdevent* fde;
unsigned expected_events;
size_t* happened_event_count;
};
@ -145,7 +145,7 @@ struct InvalidFdArg {
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 @@ static void InvalidFdThreadFunc() {
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();
}

View file

@ -58,8 +58,8 @@ struct asocket {
* 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;

View file

@ -121,10 +121,10 @@ static SocketFlushResult local_socket_flush_incoming(asocket* s) {
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 @@ static SocketFlushResult local_socket_flush_incoming(asocket* s) {
return SocketFlushResult::Destroyed;
}
fdevent_del(&s->fde, FDE_WRITE);
fdevent_del(s->fde, FDE_WRITE);
return SocketFlushResult::Completed;
}
@ -173,7 +173,7 @@ static bool local_socket_flush_outgoing(asocket* s) {
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 @@ static bool local_socket_flush_outgoing(asocket* s) {
** 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 int local_socket_enqueue(asocket* s, apacket::payload_type data) {
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 @@ static void local_socket_close(asocket* s) {
*/
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 @@ asocket* create_local_socket(int fd) {
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;
}

View file

@ -427,7 +427,7 @@ void kick_transport(atransport* t) {
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
@ -698,10 +698,9 @@ void init_transport_registration(void) {
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() {