Add the ScopedFd that we've never quite gotten around to.
This is actually for the new change I'm working on, but let's retrofit
it first to separate any bugs in these changes from those in the new
change...
Test: treehugger
Change-Id: I890aeb61f9792810a77ad0da3f9674c9cc5db7bb
diff --git a/libc/bionic/bionic_netlink.cpp b/libc/bionic/bionic_netlink.cpp
index f2449dc..5d5a026 100644
--- a/libc/bionic/bionic_netlink.cpp
+++ b/libc/bionic/bionic_netlink.cpp
@@ -39,8 +39,6 @@
#include "private/ErrnoRestorer.h"
NetlinkConnection::NetlinkConnection() {
- fd_ = -1;
-
// The kernel keeps packets under 8KiB (NLMSG_GOODSIZE),
// but that's a bit too large to go on the stack.
size_ = 8192;
@@ -48,8 +46,6 @@
}
NetlinkConnection::~NetlinkConnection() {
- ErrnoRestorer errno_restorer;
- if (fd_ != -1) close(fd_);
delete[] data_;
}
@@ -59,9 +55,9 @@
if (data_ == nullptr) return false;
// Did we open a netlink socket yet?
- if (fd_ == -1) {
- fd_ = socket(PF_NETLINK, SOCK_RAW | SOCK_CLOEXEC, NETLINK_ROUTE);
- if (fd_ == -1) return false;
+ if (fd_.get() == -1) {
+ fd_.reset(socket(PF_NETLINK, SOCK_RAW | SOCK_CLOEXEC, NETLINK_ROUTE));
+ if (fd_.get() == -1) return false;
}
// Construct and send the message.
@@ -74,13 +70,13 @@
request.hdr.nlmsg_type = type;
request.hdr.nlmsg_len = sizeof(request);
request.msg.rtgen_family = AF_UNSPEC; // All families.
- return (TEMP_FAILURE_RETRY(send(fd_, &request, sizeof(request), 0)) == sizeof(request));
+ return (TEMP_FAILURE_RETRY(send(fd_.get(), &request, sizeof(request), 0)) == sizeof(request));
}
bool NetlinkConnection::ReadResponses(void callback(void*, nlmsghdr*), void* context) {
// Read through all the responses, handing interesting ones to the callback.
ssize_t bytes_read;
- while ((bytes_read = TEMP_FAILURE_RETRY(recv(fd_, data_, size_, 0))) > 0) {
+ while ((bytes_read = TEMP_FAILURE_RETRY(recv(fd_.get(), data_, size_, 0))) > 0) {
nlmsghdr* hdr = reinterpret_cast<nlmsghdr*>(data_);
for (; NLMSG_OK(hdr, static_cast<size_t>(bytes_read)); hdr = NLMSG_NEXT(hdr, bytes_read)) {
if (hdr->nlmsg_type == NLMSG_DONE) return true;
diff --git a/libc/bionic/bionic_netlink.h b/libc/bionic/bionic_netlink.h
index a21200e..fc1bd0f 100644
--- a/libc/bionic/bionic_netlink.h
+++ b/libc/bionic/bionic_netlink.h
@@ -33,6 +33,8 @@
#include <linux/netlink.h>
#include <linux/rtnetlink.h>
+#include "private/ScopedFd.h"
+
struct nlmsghdr;
class NetlinkConnection {
@@ -44,7 +46,7 @@
bool ReadResponses(void callback(void*, nlmsghdr*), void* context);
private:
- int fd_;
+ ScopedFd fd_;
char* data_;
size_t size_;
};
diff --git a/libc/bionic/fchmodat.cpp b/libc/bionic/fchmodat.cpp
index c28e15a..632d2ac 100644
--- a/libc/bionic/fchmodat.cpp
+++ b/libc/bionic/fchmodat.cpp
@@ -32,7 +32,7 @@
#include <errno.h>
#include <unistd.h>
-#include "private/ErrnoRestorer.h"
+#include "private/ScopedFd.h"
extern "C" int __fchmodat(int, const char*, mode_t);
@@ -47,20 +47,15 @@
// at https://sourceware.org/bugzilla/show_bug.cgi?id=14578
// comment #10
- int fd = openat(dirfd, pathname, O_PATH | O_NOFOLLOW | O_CLOEXEC);
- if (fd == -1) {
- return -1; // returns errno from openat
- }
+ ScopedFd fd(openat(dirfd, pathname, O_PATH | O_NOFOLLOW | O_CLOEXEC));
+ if (fd.get() == -1) return -1;
// POSIX requires that ENOTSUP be returned when the system
// doesn't support setting the mode of a symbolic link.
// This is true for all Linux kernels.
// We rely on the O_PATH compatibility layer added in the
// fchmod() function to get errno correct.
- int result = fchmod(fd, mode);
- ErrnoRestorer errno_restorer; // don't let close() clobber errno
- close(fd);
- return result;
+ return fchmod(fd.get(), mode);
}
return __fchmodat(dirfd, pathname, mode);
diff --git a/libc/bionic/getentropy.cpp b/libc/bionic/getentropy.cpp
index 2c6e417..9c93e71 100644
--- a/libc/bionic/getentropy.cpp
+++ b/libc/bionic/getentropy.cpp
@@ -31,22 +31,20 @@
#include <sys/random.h>
#include <unistd.h>
+#include "private/ScopedFd.h"
+
static int getentropy_urandom(void* buffer, size_t buffer_size, int saved_errno) {
- int fd = TEMP_FAILURE_RETRY(open("/dev/urandom", O_RDONLY | O_NOFOLLOW | O_CLOEXEC, 0));
- if (fd == -1) return -1;
+ ScopedFd fd(TEMP_FAILURE_RETRY(open("/dev/urandom", O_RDONLY | O_NOFOLLOW | O_CLOEXEC, 0)));
+ if (fd.get() == -1) return -1;
size_t collected = 0;
while (collected < buffer_size) {
- ssize_t count = TEMP_FAILURE_RETRY(read(fd, static_cast<char*>(buffer) + collected,
+ ssize_t count = TEMP_FAILURE_RETRY(read(fd.get(), static_cast<char*>(buffer) + collected,
buffer_size - collected));
- if (count == -1) {
- close(fd);
- return -1;
- }
+ if (count == -1) return -1;
collected += count;
}
- close(fd);
errno = saved_errno;
return 0;
}
diff --git a/libc/bionic/grp_pwd_file.cpp b/libc/bionic/grp_pwd_file.cpp
index e13604e..81cf893 100644
--- a/libc/bionic/grp_pwd_file.cpp
+++ b/libc/bionic/grp_pwd_file.cpp
@@ -37,6 +37,7 @@
#include <async_safe/log.h>
#include "private/ErrnoRestorer.h"
+#include "private/ScopedFd.h"
// This file mmap's /*/etc/passwd and /*/etc/group in order to return their contents without any
// allocations. Note that these files and the strings contained within them are explicitly not
@@ -230,19 +231,16 @@
}
bool MmapFile::DoMmap() {
- int fd = open(filename_, O_CLOEXEC | O_NOFOLLOW | O_RDONLY);
+ ScopedFd fd(open(filename_, O_CLOEXEC | O_NOFOLLOW | O_RDONLY));
struct stat fd_stat;
- if (fstat(fd, &fd_stat) == -1) {
- close(fd);
+ if (fstat(fd.get(), &fd_stat) == -1) {
return false;
}
auto mmap_size = fd_stat.st_size;
- void* map_result = mmap(nullptr, mmap_size, PROT_READ, MAP_SHARED, fd, 0);
- close(fd);
-
+ void* map_result = mmap(nullptr, mmap_size, PROT_READ, MAP_SHARED, fd.get(), 0);
if (map_result == MAP_FAILED) {
return false;
}
diff --git a/libc/bionic/net_if.cpp b/libc/bionic/net_if.cpp
index db9c9ea2..ad53364 100644
--- a/libc/bionic/net_if.cpp
+++ b/libc/bionic/net_if.cpp
@@ -40,37 +40,27 @@
#include <sys/socket.h>
#include <unistd.h>
-#include "private/ErrnoRestorer.h"
+#include "private/ScopedFd.h"
#include "bionic_netlink.h"
char* if_indextoname(unsigned ifindex, char* ifname) {
- int s = socket(AF_INET, SOCK_DGRAM|SOCK_CLOEXEC, 0);
- if (s == -1) return nullptr;
+ ScopedFd s(socket(AF_INET, SOCK_DGRAM|SOCK_CLOEXEC, 0));
+ if (s.get() == -1) return nullptr;
- struct ifreq ifr;
- memset(&ifr, 0, sizeof(ifr));
- ifr.ifr_ifindex = ifindex;
-
- int rc = ioctl(s, SIOCGIFNAME, &ifr);
- ErrnoRestorer errno_restorer;
- close(s);
- return (rc == -1) ? nullptr : strncpy(ifname, ifr.ifr_name, IFNAMSIZ);
+ ifreq ifr = {.ifr_ifindex = static_cast<int>(ifindex)};
+ return (ioctl(s.get(), SIOCGIFNAME, &ifr) == -1) ? nullptr
+ : strncpy(ifname, ifr.ifr_name, IFNAMSIZ);
}
unsigned if_nametoindex(const char* ifname) {
- int s = socket(AF_INET, SOCK_DGRAM|SOCK_CLOEXEC, 0);
- if (s == -1) return 0;
+ ScopedFd s(socket(AF_INET, SOCK_DGRAM|SOCK_CLOEXEC, 0));
+ if (s.get() == -1) return 0;
- struct ifreq ifr;
- memset(&ifr, 0, sizeof(ifr));
+ ifreq ifr = {};
strncpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name));
ifr.ifr_name[IFNAMSIZ - 1] = 0;
-
- int rc = ioctl(s, SIOCGIFINDEX, &ifr);
- ErrnoRestorer errno_restorer;
- close(s);
- return (rc == -1) ? 0 : ifr.ifr_ifindex;
+ return (ioctl(s.get(), SIOCGIFINDEX, &ifr) == -1) ? 0 : ifr.ifr_ifindex;
}
struct if_list {
diff --git a/libc/bionic/system_property_set.cpp b/libc/bionic/system_property_set.cpp
index c508db1..e981a58 100644
--- a/libc/bionic/system_property_set.cpp
+++ b/libc/bionic/system_property_set.cpp
@@ -46,6 +46,7 @@
#include "private/bionic_defs.h"
#include "private/bionic_macros.h"
+#include "private/ScopedFd.h"
static const char property_service_socket[] = "/dev/socket/" PROP_SERVICE_NAME;
static const char* kServiceVersionPropertyName = "ro.property_service.version";
@@ -53,8 +54,8 @@
class PropertyServiceConnection {
public:
PropertyServiceConnection() : last_error_(0) {
- socket_ = ::socket(AF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0);
- if (socket_ == -1) {
+ socket_.reset(::socket(AF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0));
+ if (socket_.get() == -1) {
last_error_ = errno;
return;
}
@@ -66,15 +67,15 @@
addr.sun_family = AF_LOCAL;
socklen_t alen = namelen + offsetof(sockaddr_un, sun_path) + 1;
- if (TEMP_FAILURE_RETRY(connect(socket_, reinterpret_cast<sockaddr*>(&addr), alen)) == -1) {
+ if (TEMP_FAILURE_RETRY(connect(socket_.get(),
+ reinterpret_cast<sockaddr*>(&addr), alen)) == -1) {
last_error_ = errno;
- close(socket_);
- socket_ = -1;
+ socket_.reset();
}
}
bool IsValid() {
- return socket_ != -1;
+ return socket_.get() != -1;
}
int GetLastError() {
@@ -82,18 +83,12 @@
}
bool RecvInt32(int32_t* value) {
- int result = TEMP_FAILURE_RETRY(recv(socket_, value, sizeof(*value), MSG_WAITALL));
+ int result = TEMP_FAILURE_RETRY(recv(socket_.get(), value, sizeof(*value), MSG_WAITALL));
return CheckSendRecvResult(result, sizeof(*value));
}
int socket() {
- return socket_;
- }
-
- ~PropertyServiceConnection() {
- if (socket_ != -1) {
- close(socket_);
- }
+ return socket_.get();
}
private:
@@ -109,7 +104,7 @@
return last_error_ == 0;
}
- int socket_;
+ ScopedFd socket_;
int last_error_;
friend class SocketWriter;