diff --git a/adb/adb.h b/adb/adb.h index d79cd2d4c..920999705 100644 --- a/adb/adb.h +++ b/adb/adb.h @@ -139,7 +139,7 @@ atransport* find_emulator_transport_by_adb_port(int adb_port); atransport* find_emulator_transport_by_console_port(int console_port); #endif -int service_to_fd(std::string_view name, atransport* transport); +unique_fd service_to_fd(std::string_view name, atransport* transport); #if !ADB_HOST unique_fd daemon_service_to_fd(std::string_view name, atransport* transport); #endif diff --git a/adb/adb_listeners.cpp b/adb/adb_listeners.cpp index be457a683..29909a555 100644 --- a/adb/adb_listeners.cpp +++ b/adb/adb_listeners.cpp @@ -75,41 +75,36 @@ static ListenerList& listener_list GUARDED_BY(listener_list_mutex) = *new Listen static void ss_listener_event_func(int _fd, unsigned ev, void *_l) { if (ev & FDE_READ) { - int fd = adb_socket_accept(_fd, nullptr, nullptr); + unique_fd fd(adb_socket_accept(_fd, nullptr, nullptr)); if (fd < 0) return; int rcv_buf_size = CHUNK_SIZE; - adb_setsockopt(fd, SOL_SOCKET, SO_RCVBUF, &rcv_buf_size, sizeof(rcv_buf_size)); + adb_setsockopt(fd.get(), SOL_SOCKET, SO_RCVBUF, &rcv_buf_size, sizeof(rcv_buf_size)); - asocket* s = create_local_socket(fd); + asocket* s = create_local_socket(std::move(fd)); if (s) { connect_to_smartsocket(s); return; } - - adb_close(fd); } } static void listener_event_func(int _fd, unsigned ev, void* _l) { alistener* listener = reinterpret_cast(_l); - asocket *s; if (ev & FDE_READ) { - int fd = adb_socket_accept(_fd, nullptr, nullptr); + unique_fd fd(adb_socket_accept(_fd, nullptr, nullptr)); if (fd < 0) { return; } - s = create_local_socket(fd); + asocket* s = create_local_socket(std::move(fd)); if (s) { s->transport = listener->transport; connect_to_remote(s, listener->connect_to); return; } - - adb_close(fd); } } diff --git a/adb/adb_utils.h b/adb/adb_utils.h index 8253487a8..a85ca8c84 100644 --- a/adb/adb_utils.h +++ b/adb/adb_utils.h @@ -53,8 +53,6 @@ std::string perror_str(const char* msg); bool set_file_block_mode(int fd, bool block); -int adb_close(int fd); - // Given forward/reverse targets, returns true if they look sane. If an error is found, fills // |error| and returns false. // Currently this only checks "tcp:" targets. Additional checking could be added for other targets diff --git a/adb/adb_utils_test.cpp b/adb/adb_utils_test.cpp index bb094258c..8518e1782 100644 --- a/adb/adb_utils_test.cpp +++ b/adb/adb_utils_test.cpp @@ -147,17 +147,16 @@ TEST(adb_utils, mkdirs) { #if !defined(_WIN32) TEST(adb_utils, set_file_block_mode) { - int fd = adb_open("/dev/null", O_RDWR | O_APPEND); - ASSERT_GE(fd, 0); - int flags = fcntl(fd, F_GETFL, 0); - ASSERT_EQ(O_RDWR | O_APPEND, (flags & (O_RDWR | O_APPEND))); - ASSERT_TRUE(set_file_block_mode(fd, false)); - int new_flags = fcntl(fd, F_GETFL, 0); - ASSERT_EQ(flags | O_NONBLOCK, new_flags); - ASSERT_TRUE(set_file_block_mode(fd, true)); - new_flags = fcntl(fd, F_GETFL, 0); - ASSERT_EQ(flags, new_flags); - ASSERT_EQ(0, adb_close(fd)); + unique_fd fd(adb_open("/dev/null", O_RDWR | O_APPEND)); + ASSERT_GE(fd, 0); + int flags = fcntl(fd, F_GETFL, 0); + ASSERT_EQ(O_RDWR | O_APPEND, (flags & (O_RDWR | O_APPEND))); + ASSERT_TRUE(set_file_block_mode(fd, false)); + int new_flags = fcntl(fd, F_GETFL, 0); + ASSERT_EQ(flags | O_NONBLOCK, new_flags); + ASSERT_TRUE(set_file_block_mode(fd, true)); + new_flags = fcntl(fd, F_GETFL, 0); + ASSERT_EQ(flags, new_flags); } #endif diff --git a/adb/client/adb_client.cpp b/adb/client/adb_client.cpp index 9a25d104c..0a09d1ee5 100644 --- a/adb/client/adb_client.cpp +++ b/adb/client/adb_client.cpp @@ -100,13 +100,11 @@ static int switch_socket_transport(int fd, std::string* error) { if (!SendProtocolString(fd, service)) { *error = perror_str("write failure during connection"); - adb_close(fd); return -1; } D("Switch transport in progress"); if (!adb_status(fd, error)) { - adb_close(fd); D("Switch transport failed: %s", error->c_str()); return -1; } @@ -194,7 +192,7 @@ bool adb_kill_server() { int adb_connect(const std::string& service, std::string* error) { // first query the adb server's version - int fd = _adb_connect("host:version", error); + unique_fd fd(_adb_connect("host:version", error)); D("adb_connect: service %s", service.c_str()); if (fd == -2 && !is_local_socket_spec(__adb_server_socket_spec)) { @@ -224,12 +222,10 @@ int adb_connect(const std::string& service, std::string* error) { if (fd >= 0) { std::string version_string; if (!ReadProtocolString(fd, &version_string, error)) { - adb_close(fd); return -1; } ReadOrderlyShutdown(fd); - adb_close(fd); if (sscanf(&version_string[0], "%04x", &version) != 1) { *error = android::base::StringPrintf("cannot parse version string: %s", @@ -258,52 +254,48 @@ int adb_connect(const std::string& service, std::string* error) { return 0; } - fd = _adb_connect(service, error); + fd.reset(_adb_connect(service, error)); if (fd == -1) { D("_adb_connect error: %s", error->c_str()); } else if(fd == -2) { fprintf(stderr, "* daemon still not running\n"); } - D("adb_connect: return fd %d", fd); + D("adb_connect: return fd %d", fd.get()); - return fd; + return fd.release(); } bool adb_command(const std::string& service) { std::string error; - int fd = adb_connect(service, &error); + unique_fd fd(adb_connect(service, &error)); if (fd < 0) { fprintf(stderr, "error: %s\n", error.c_str()); return false; } - if (!adb_status(fd, &error)) { + if (!adb_status(fd.get(), &error)) { fprintf(stderr, "error: %s\n", error.c_str()); - adb_close(fd); return false; } - ReadOrderlyShutdown(fd); - adb_close(fd); + ReadOrderlyShutdown(fd.get()); return true; } bool adb_query(const std::string& service, std::string* result, std::string* error) { D("adb_query: %s", service.c_str()); - int fd = adb_connect(service, error); + unique_fd fd(adb_connect(service, error)); if (fd < 0) { return false; } result->clear(); - if (!ReadProtocolString(fd, result, error)) { - adb_close(fd); + if (!ReadProtocolString(fd.get(), result, error)) { return false; } - ReadOrderlyShutdown(fd); - adb_close(fd); + ReadOrderlyShutdown(fd.get()); return true; } diff --git a/adb/client/commandline.cpp b/adb/client/commandline.cpp index e963e3d20..f70b48009 100644 --- a/adb/client/commandline.cpp +++ b/adb/client/commandline.cpp @@ -794,7 +794,7 @@ static int adb_abb(int argc, const char** argv) { static int adb_sideload_legacy(const char* filename, int in_fd, int size) { std::string error; - int out_fd = adb_connect(android::base::StringPrintf("sideload:%d", size), &error); + unique_fd out_fd(adb_connect(android::base::StringPrintf("sideload:%d", size), &error)); if (out_fd < 0) { fprintf(stderr, "adb: pre-KitKat sideload connection failed: %s\n", error.c_str()); return -1; @@ -809,14 +809,12 @@ static int adb_sideload_legacy(const char* filename, int in_fd, int size) { unsigned xfer = (size > CHUNK_SIZE) ? CHUNK_SIZE : size; if (!ReadFdExactly(in_fd, buf, xfer)) { fprintf(stderr, "adb: failed to read data from %s: %s\n", filename, strerror(errno)); - adb_close(out_fd); return -1; } if (!WriteFdExactly(out_fd, buf, xfer)) { std::string error; adb_status(out_fd, &error); fprintf(stderr, "adb: failed to write data: %s\n", error.c_str()); - adb_close(out_fd); return -1; } size -= xfer; @@ -827,11 +825,9 @@ static int adb_sideload_legacy(const char* filename, int in_fd, int size) { if (!adb_status(out_fd, &error)) { fprintf(stderr, "adb: error response: %s\n", error.c_str()); - adb_close(out_fd); return -1; } - adb_close(out_fd); return 0; } @@ -1091,7 +1087,7 @@ static bool adb_root(const char* command) { int send_shell_command(const std::string& command, bool disable_shell_protocol, StandardStreamsCallbackInterface* callback) { - int fd; + unique_fd fd; bool use_shell_protocol = false; while (true) { @@ -1114,7 +1110,7 @@ int send_shell_command(const std::string& command, bool disable_shell_protocol, std::string error; std::string service_string = ShellServiceString(use_shell_protocol, "", command); - fd = adb_connect(service_string, &error); + fd.reset(adb_connect(service_string, &error)); if (fd >= 0) { break; } @@ -1126,13 +1122,7 @@ int send_shell_command(const std::string& command, bool disable_shell_protocol, } } - int exit_code = read_and_dump(fd, use_shell_protocol, callback); - - if (adb_close(fd) < 0) { - PLOG(ERROR) << "failure closing FD " << fd; - } - - return exit_code; + return read_and_dump(fd.get(), use_shell_protocol, callback); } static int logcat(int argc, const char** argv) { @@ -1196,7 +1186,7 @@ static int backup(int argc, const char** argv) { if (argc < 2) error_exit("backup either needs a list of packages or -all/-shared"); adb_unlink(filename); - int outFd = adb_creat(filename, 0640); + unique_fd outFd(adb_creat(filename, 0640)); if (outFd < 0) { fprintf(stderr, "adb: backup unable to create file '%s': %s\n", filename, strerror(errno)); return EXIT_FAILURE; @@ -1211,20 +1201,16 @@ static int backup(int argc, const char** argv) { D("backup. filename=%s cmd=%s", filename, cmd.c_str()); std::string error; - int fd = adb_connect(cmd, &error); + unique_fd fd(adb_connect(cmd, &error)); if (fd < 0) { fprintf(stderr, "adb: unable to connect for backup: %s\n", error.c_str()); - adb_close(outFd); return EXIT_FAILURE; } fprintf(stdout, "Now unlock your device and confirm the backup operation...\n"); fflush(stdout); - copy_to_file(fd, outFd); - - adb_close(fd); - adb_close(outFd); + copy_to_file(fd.get(), outFd.get()); return EXIT_SUCCESS; } @@ -1232,33 +1218,29 @@ static int restore(int argc, const char** argv) { if (argc != 2) error_exit("restore requires an argument"); const char* filename = argv[1]; - int tarFd = adb_open(filename, O_RDONLY); + unique_fd tarFd(adb_open(filename, O_RDONLY)); if (tarFd < 0) { fprintf(stderr, "adb: unable to open file %s: %s\n", filename, strerror(errno)); return -1; } std::string error; - int fd = adb_connect("restore:", &error); + unique_fd fd(adb_connect("restore:", &error)); if (fd < 0) { fprintf(stderr, "adb: unable to connect for restore: %s\n", error.c_str()); - adb_close(tarFd); return -1; } fprintf(stdout, "Now unlock your device and confirm the restore operation.\n"); fflush(stdout); - copy_to_file(tarFd, fd); + copy_to_file(tarFd.get(), fd.get()); // Provide an in-band EOD marker in case the archive file is malformed - write_zeros(512*2, fd); + write_zeros(512 * 2, fd); // Wait until the other side finishes, or it'll get sent SIGHUP. - copy_to_file(fd, STDOUT_FILENO); - - adb_close(fd); - adb_close(tarFd); + copy_to_file(fd.get(), STDOUT_FILENO); return 0; } @@ -1298,19 +1280,18 @@ static void parse_push_pull_args(const char** arg, int narg, std::vector= 0; } bool ReceivedError(const char* from, const char* to) { - adb_pollfd pfd = {.fd = fd, .events = POLLIN}; + adb_pollfd pfd = {.fd = fd.get(), .events = POLLIN}; int rc = adb_poll(&pfd, 1, 0); if (rc < 0) { Error("failed to poll: %s", strerror(errno)); @@ -324,7 +322,7 @@ class SyncConnection { memset(st, 0, sizeof(*st)); if (have_stat_v2_) { - if (!ReadFdExactly(fd, &msg.stat_v2, sizeof(msg.stat_v2))) { + if (!ReadFdExactly(fd.get(), &msg.stat_v2, sizeof(msg.stat_v2))) { PLOG(FATAL) << "protocol fault: failed to read stat response"; } @@ -350,7 +348,7 @@ class SyncConnection { st->st_ctime = msg.stat_v2.ctime; return true; } else { - if (!ReadFdExactly(fd, &msg.stat_v1, sizeof(msg.stat_v1))) { + if (!ReadFdExactly(fd.get(), &msg.stat_v1, sizeof(msg.stat_v1))) { PLOG(FATAL) << "protocol fault: failed to read stat response"; } @@ -437,7 +435,7 @@ class SyncConnection { uint64_t total_size = st.st_size; uint64_t bytes_copied = 0; - int lfd = adb_open(lpath, O_RDONLY); + unique_fd lfd(adb_open(lpath, O_RDONLY)); if (lfd < 0) { Error("opening '%s' locally failed: %s", lpath, strerror(errno)); return false; @@ -449,7 +447,6 @@ class SyncConnection { int bytes_read = adb_read(lfd, sbuf.data, max - sizeof(SyncRequest)); if (bytes_read == -1) { Error("reading '%s' locally failed: %s", lpath, strerror(errno)); - adb_close(lfd); return false; } else if (bytes_read == 0) { break; @@ -469,8 +466,6 @@ class SyncConnection { ReportProgress(rpath, bytes_copied, total_size); } - adb_close(lfd); - syncmsg msg; msg.data.id = ID_DONE; msg.data.size = mtime; @@ -576,7 +571,7 @@ class SyncConnection { } // TODO: add a char[max] buffer here, to replace syncsendbuf... - int fd; + unique_fd fd; size_t max; private: @@ -740,7 +735,7 @@ static bool sync_recv(SyncConnection& sc, const char* rpath, const char* lpath, if (!sc.SendRequest(ID_RECV, rpath)) return false; adb_unlink(lpath); - int lfd = adb_creat(lpath, 0644); + unique_fd lfd(adb_creat(lpath, 0644)); if (lfd < 0) { sc.Error("cannot create '%s': %s", lpath, strerror(errno)); return false; @@ -750,7 +745,6 @@ static bool sync_recv(SyncConnection& sc, const char* rpath, const char* lpath, while (true) { syncmsg msg; if (!ReadFdExactly(sc.fd, &msg.data, sizeof(msg.data))) { - adb_close(lfd); adb_unlink(lpath); return false; } @@ -758,7 +752,6 @@ static bool sync_recv(SyncConnection& sc, const char* rpath, const char* lpath, if (msg.data.id == ID_DONE) break; if (msg.data.id != ID_DATA) { - adb_close(lfd); adb_unlink(lpath); sc.ReportCopyFailure(rpath, lpath, msg); return false; @@ -766,21 +759,18 @@ static bool sync_recv(SyncConnection& sc, const char* rpath, const char* lpath, if (msg.data.size > sc.max) { sc.Error("msg.data.size too large: %u (max %zu)", msg.data.size, sc.max); - adb_close(lfd); adb_unlink(lpath); return false; } char buffer[SYNC_DATA_MAX]; if (!ReadFdExactly(sc.fd, buffer, msg.data.size)) { - adb_close(lfd); adb_unlink(lpath); return false; } if (!WriteFdExactly(lfd, buffer, msg.data.size)) { sc.Error("cannot write '%s': %s", lpath, strerror(errno)); - adb_close(lfd); adb_unlink(lpath); return false; } @@ -792,7 +782,6 @@ static bool sync_recv(SyncConnection& sc, const char* rpath, const char* lpath, } sc.RecordFilesTransferred(1); - adb_close(lfd); return true; } diff --git a/adb/daemon/file_sync_service.cpp b/adb/daemon/file_sync_service.cpp index 62c18c57d..56b5cd8d9 100644 --- a/adb/daemon/file_sync_service.cpp +++ b/adb/daemon/file_sync_service.cpp @@ -232,7 +232,7 @@ static bool handle_send_file(int s, const char* path, uid_t uid, gid_t gid, uint __android_log_security_bswrite(SEC_TAG_ADB_SEND_FILE, path); - int fd = adb_open_mode(path, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, mode); + unique_fd fd(adb_open_mode(path, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, mode)); if (posix_fadvise(fd, 0, 0, POSIX_FADV_SEQUENTIAL | POSIX_FADV_NOREUSE | POSIX_FADV_WILLNEED) < 0) { @@ -244,16 +244,16 @@ static bool handle_send_file(int s, const char* path, uid_t uid, gid_t gid, uint SendSyncFailErrno(s, "secure_mkdirs failed"); goto fail; } - fd = adb_open_mode(path, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, mode); + fd.reset(adb_open_mode(path, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, mode)); } if (fd < 0 && errno == EEXIST) { - fd = adb_open_mode(path, O_WRONLY | O_CLOEXEC, mode); + fd.reset(adb_open_mode(path, O_WRONLY | O_CLOEXEC, mode)); } if (fd < 0) { SendSyncFailErrno(s, "couldn't create file"); goto fail; } else { - if (fchown(fd, uid, gid) == -1) { + if (fchown(fd.get(), uid, gid) == -1) { SendSyncFailErrno(s, "fchown failed"); goto fail; } @@ -266,7 +266,7 @@ static bool handle_send_file(int s, const char* path, uid_t uid, gid_t gid, uint // fchown clears the setuid bit - restore it if present. // Ignore the result of calling fchmod. It's not supported // by all filesystems, so we don't check for success. b/12441485 - fchmod(fd, mode); + fchmod(fd.get(), mode); } while (true) { @@ -288,14 +288,12 @@ static bool handle_send_file(int s, const char* path, uid_t uid, gid_t gid, uint if (!ReadFdExactly(s, &buffer[0], msg.data.size)) goto abort; - if (!WriteFdExactly(fd, &buffer[0], msg.data.size)) { + if (!WriteFdExactly(fd.get(), &buffer[0], msg.data.size)) { SendSyncFailErrno(s, "write failed"); goto fail; } } - adb_close(fd); - if (!update_capabilities(path, capabilities)) { SendSyncFailErrno(s, "update_capabilities failed"); goto fail; @@ -340,7 +338,6 @@ fail: } abort: - if (fd >= 0) adb_close(fd); if (do_unlink) adb_unlink(path); return false; } @@ -445,35 +442,31 @@ static bool do_send(int s, const std::string& spec, std::vector& buffer) { static bool do_recv(int s, const char* path, std::vector& buffer) { __android_log_security_bswrite(SEC_TAG_ADB_RECV_FILE, path); - int fd = adb_open(path, O_RDONLY | O_CLOEXEC); + unique_fd fd(adb_open(path, O_RDONLY | O_CLOEXEC)); if (fd < 0) { SendSyncFailErrno(s, "open failed"); return false; } - if (posix_fadvise(fd, 0, 0, POSIX_FADV_SEQUENTIAL | POSIX_FADV_NOREUSE) < 0) { + if (posix_fadvise(fd.get(), 0, 0, POSIX_FADV_SEQUENTIAL | POSIX_FADV_NOREUSE) < 0) { D("[ Failed to fadvise: %d ]", errno); } syncmsg msg; msg.data.id = ID_DATA; while (true) { - int r = adb_read(fd, &buffer[0], buffer.size() - sizeof(msg.data)); + int r = adb_read(fd.get(), &buffer[0], buffer.size() - sizeof(msg.data)); if (r <= 0) { if (r == 0) break; SendSyncFailErrno(s, "read failed"); - adb_close(fd); return false; } msg.data.size = r; if (!WriteFdExactly(s, &msg.data, sizeof(msg.data)) || !WriteFdExactly(s, &buffer[0], r)) { - adb_close(fd); return false; } } - adb_close(fd); - msg.data.id = ID_DONE; msg.data.size = 0; return WriteFdExactly(s, &msg.data, sizeof(msg.data)); diff --git a/adb/daemon/jdwp_service.cpp b/adb/daemon/jdwp_service.cpp index f02cc1350..032ee42a2 100644 --- a/adb/daemon/jdwp_service.cpp +++ b/adb/daemon/jdwp_service.cpp @@ -303,7 +303,6 @@ static void jdwp_control_event(int s, unsigned events, void* user); static int jdwp_control_init(JdwpControl* control, const char* sockname, int socknamelen) { sockaddr_un addr; socklen_t addrlen; - int s; int maxpath = sizeof(addr.sun_path); int pathlen = socknamelen; @@ -316,7 +315,7 @@ static int jdwp_control_init(JdwpControl* control, const char* sockname, int soc addr.sun_family = AF_UNIX; memcpy(addr.sun_path, sockname, socknamelen); - s = socket(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0); + unique_fd s(socket(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0)); if (s < 0) { D("could not create vm debug control socket. %d: %s", errno, strerror(errno)); return -1; @@ -326,22 +325,18 @@ static int jdwp_control_init(JdwpControl* control, const char* sockname, int soc if (bind(s, reinterpret_cast(&addr), addrlen) < 0) { D("could not bind vm debug control socket: %d: %s", errno, strerror(errno)); - adb_close(s); return -1; } if (listen(s, 4) < 0) { D("listen failed in jdwp control socket: %d: %s", errno, strerror(errno)); - adb_close(s); return -1; } - control->listen_socket = s; - - control->fde = fdevent_create(s, jdwp_control_event, control); + control->listen_socket = s.release(); + control->fde = fdevent_create(control->listen_socket, jdwp_control_event, control); if (control->fde == nullptr) { D("could not create fdevent for jdwp control socket"); - adb_close(s); return -1; } diff --git a/adb/fdevent_test.h b/adb/fdevent_test.h index 8d853c369..24bce59a7 100644 --- a/adb/fdevent_test.h +++ b/adb/fdevent_test.h @@ -21,6 +21,7 @@ #include #include "adb_io.h" +#include "adb_unique_fd.h" #include "socket.h" #include "sysdeps.h" #include "sysdeps/chrono.h" @@ -45,7 +46,7 @@ static void WaitForFdeventLoop() { class FdeventTest : public ::testing::Test { protected: - int dummy = -1; + unique_fd dummy; static void SetUpTestCase() { #if !defined(_WIN32) @@ -65,12 +66,12 @@ class FdeventTest : public ::testing::Test { FAIL() << "failed to create socketpair: " << strerror(errno); } - asocket* dummy_socket = create_local_socket(dummy_fds[1]); + asocket* dummy_socket = create_local_socket(unique_fd(dummy_fds[1])); if (!dummy_socket) { FAIL() << "failed to create local socket: " << strerror(errno); } dummy_socket->ready(dummy_socket); - dummy = dummy_fds[0]; + dummy.reset(dummy_fds[0]); thread_ = std::thread([]() { fdevent_loop(); }); WaitForFdeventLoop(); @@ -85,7 +86,7 @@ class FdeventTest : public ::testing::Test { fdevent_terminate_loop(); ASSERT_TRUE(WriteFdExactly(dummy, "", 1)); thread_.join(); - ASSERT_EQ(0, adb_close(dummy)); + dummy.reset(); } std::thread thread_; diff --git a/adb/services.cpp b/adb/services.cpp index 73fed0932..0061f0ead 100644 --- a/adb/services.cpp +++ b/adb/services.cpp @@ -71,7 +71,7 @@ unique_fd create_service_thread(const char* service_name, std::function= 0) { - close_on_exec(ret); + close_on_exec(ret.get()); } - return ret.release(); + return ret; } #if ADB_HOST @@ -99,9 +99,7 @@ struct state_info { ConnectionState state; }; -static void wait_for_state(int fd, void* data) { - std::unique_ptr sinfo(reinterpret_cast(data)); - +static void wait_for_state(int fd, state_info* sinfo) { D("wait_for_state %d", sinfo->state); while (true) { @@ -197,7 +195,7 @@ asocket* host_service_to_socket(const char* name, const char* serial, TransportI } else if (android::base::StartsWith(name, "wait-for-")) { name += strlen("wait-for-"); - std::unique_ptr sinfo = std::make_unique(); + std::shared_ptr sinfo = std::make_shared(); if (sinfo == nullptr) { fprintf(stderr, "couldn't allocate state_info: %s", strerror(errno)); return nullptr; @@ -233,19 +231,15 @@ asocket* host_service_to_socket(const char* name, const char* serial, TransportI return nullptr; } - int fd = create_service_thread( - "wait", std::bind(wait_for_state, std::placeholders::_1, sinfo.get())) - .release(); - if (fd != -1) { - sinfo.release(); - } - return create_local_socket(fd); + unique_fd fd = create_service_thread("wait", [sinfo](int fd) { + wait_for_state(fd, sinfo.get()); + }); + return create_local_socket(std::move(fd)); } else if (!strncmp(name, "connect:", 8)) { std::string host(name + strlen("connect:")); - int fd = create_service_thread("connect", - std::bind(connect_service, std::placeholders::_1, host)) - .release(); - return create_local_socket(fd); + unique_fd fd = create_service_thread( + "connect", std::bind(connect_service, std::placeholders::_1, host)); + return create_local_socket(std::move(fd)); } return nullptr; } diff --git a/adb/socket.h b/adb/socket.h index e7df9916b..b8c559a90 100644 --- a/adb/socket.h +++ b/adb/socket.h @@ -23,6 +23,7 @@ #include #include +#include "adb_unique_fd.h" #include "fdevent.h" #include "types.h" @@ -102,7 +103,7 @@ void install_local_socket(asocket *s); void remove_socket(asocket *s); void close_all_sockets(atransport *t); -asocket *create_local_socket(int fd); +asocket* create_local_socket(unique_fd fd); asocket* create_local_service_socket(std::string_view destination, atransport* transport); asocket *create_remote_socket(unsigned id, atransport *t); diff --git a/adb/socket_test.cpp b/adb/socket_test.cpp index 80f9430b7..7908f82e9 100644 --- a/adb/socket_test.cpp +++ b/adb/socket_test.cpp @@ -58,7 +58,7 @@ TEST_F(LocalSocketTest, smoke) { intermediates.resize(INTERMEDIATE_COUNT); ASSERT_EQ(0, adb_socketpair(first)) << strerror(errno); ASSERT_EQ(0, adb_socketpair(last)) << strerror(errno); - asocket* prev_tail = create_local_socket(first[1]); + asocket* prev_tail = create_local_socket(unique_fd(first[1])); ASSERT_NE(nullptr, prev_tail); auto connect = [](asocket* tail, asocket* head) { @@ -70,17 +70,17 @@ TEST_F(LocalSocketTest, smoke) { for (auto& intermediate : intermediates) { ASSERT_EQ(0, adb_socketpair(intermediate.data())) << strerror(errno); - asocket* head = create_local_socket(intermediate[0]); + asocket* head = create_local_socket(unique_fd(intermediate[0])); ASSERT_NE(nullptr, head); - asocket* tail = create_local_socket(intermediate[1]); + asocket* tail = create_local_socket(unique_fd(intermediate[1])); ASSERT_NE(nullptr, tail); connect(prev_tail, head); prev_tail = tail; } - asocket* end = create_local_socket(last[0]); + asocket* end = create_local_socket(unique_fd(last[0])); ASSERT_NE(nullptr, end); connect(prev_tail, end); @@ -104,14 +104,14 @@ TEST_F(LocalSocketTest, smoke) { } struct CloseWithPacketArg { - int socket_fd; + unique_fd socket_fd; size_t bytes_written; - int cause_close_fd; + unique_fd cause_close_fd; }; static void CreateCloser(CloseWithPacketArg* arg) { fdevent_run_on_main_thread([arg]() { - asocket* s = create_local_socket(arg->socket_fd); + asocket* s = create_local_socket(std::move(arg->socket_fd)); ASSERT_TRUE(s != nullptr); arg->bytes_written = 0; @@ -135,7 +135,7 @@ static void CreateCloser(CloseWithPacketArg* arg) { } ASSERT_TRUE(socket_filled); - asocket* cause_close_s = create_local_socket(arg->cause_close_fd); + asocket* cause_close_s = create_local_socket(std::move(arg->cause_close_fd)); ASSERT_TRUE(cause_close_s != nullptr); cause_close_s->peer = s; s->peer = cause_close_s; @@ -154,8 +154,8 @@ TEST_F(LocalSocketTest, close_socket_with_packet) { int cause_close_fd[2]; ASSERT_EQ(0, adb_socketpair(cause_close_fd)); CloseWithPacketArg arg; - arg.socket_fd = socket_fd[1]; - arg.cause_close_fd = cause_close_fd[1]; + arg.socket_fd.reset(socket_fd[1]); + arg.cause_close_fd.reset(cause_close_fd[1]); PrepareThread(); CreateCloser(&arg); @@ -178,8 +178,8 @@ TEST_F(LocalSocketTest, read_from_closing_socket) { int cause_close_fd[2]; ASSERT_EQ(0, adb_socketpair(cause_close_fd)); CloseWithPacketArg arg; - arg.socket_fd = socket_fd[1]; - arg.cause_close_fd = cause_close_fd[1]; + arg.socket_fd.reset(socket_fd[1]); + arg.cause_close_fd.reset(cause_close_fd[1]); PrepareThread(); CreateCloser(&arg); @@ -211,8 +211,8 @@ TEST_F(LocalSocketTest, write_error_when_having_packets) { int cause_close_fd[2]; ASSERT_EQ(0, adb_socketpair(cause_close_fd)); CloseWithPacketArg arg; - arg.socket_fd = socket_fd[1]; - arg.cause_close_fd = cause_close_fd[1]; + arg.socket_fd.reset(socket_fd[1]); + arg.cause_close_fd.reset(cause_close_fd[1]); PrepareThread(); CreateCloser(&arg); @@ -233,8 +233,8 @@ TEST_F(LocalSocketTest, flush_after_shutdown) { ASSERT_EQ(0, adb_socketpair(head_fd)); ASSERT_EQ(0, adb_socketpair(tail_fd)); - asocket* head = create_local_socket(head_fd[1]); - asocket* tail = create_local_socket(tail_fd[1]); + asocket* head = create_local_socket(unique_fd(head_fd[1])); + asocket* tail = create_local_socket(unique_fd(tail_fd[1])); head->peer = tail; head->ready(head); @@ -287,7 +287,7 @@ TEST_F(LocalSocketTest, close_socket_in_CLOSE_WAIT_state) { PrepareThread(); fdevent_run_on_main_thread([accept_fd]() { - asocket* s = create_local_socket(accept_fd); + asocket* s = create_local_socket(unique_fd(accept_fd)); ASSERT_TRUE(s != nullptr); }); diff --git a/adb/sockets.cpp b/adb/sockets.cpp index 47ae883f4..28da975f5 100644 --- a/adb/sockets.cpp +++ b/adb/sockets.cpp @@ -333,7 +333,8 @@ static void local_socket_event_func(int fd, unsigned ev, void* _s) { } } -asocket* create_local_socket(int fd) { +asocket* create_local_socket(unique_fd ufd) { + int fd = ufd.release(); asocket* s = new asocket(); s->fd = fd; s->enqueue = local_socket_enqueue; @@ -353,13 +354,13 @@ asocket* create_local_service_socket(std::string_view name, atransport* transpor return s; } #endif - int fd = service_to_fd(name, transport); + unique_fd fd = service_to_fd(name, transport); if (fd < 0) { return nullptr; } - asocket* s = create_local_socket(fd); - LOG(VERBOSE) << "LS(" << s->id << "): bound to '" << name << "' via " << fd; + asocket* s = create_local_socket(std::move(fd)); + LOG(VERBOSE) << "LS(" << s->id << "): bound to '" << name << "' via " << fd.get(); #if !ADB_HOST if ((name.starts_with("root:") && getuid() != 0 && __android_log_is_debuggable()) ||