Merge "Add the ScopedFd that we've never quite gotten around to."

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() {
 }
 
 NetlinkConnection::~NetlinkConnection() {
-  ErrnoRestorer errno_restorer;
-  if (fd_ != -1) close(fd_);
   delete[] data_;
 }
 
@@ -59,9 +55,9 @@ bool NetlinkConnection::SendRequest(int type) {
   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 @@ bool NetlinkConnection::SendRequest(int type) {
   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;

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 @@ class NetlinkConnection {
   bool ReadResponses(void callback(void*, nlmsghdr*), void* context);
 
  private:
-  int fd_;
+  ScopedFd fd_;
   char* data_;
   size_t size_;
 };

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 @@ int fchmodat(int dirfd, const char* pathname, mode_t mode, int flags) {
     // 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);

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;
 }

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::GetFile(const char** start, const char** end) {
 }
 
 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;
   }

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 {

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 @@ static const char* kServiceVersionPropertyName = "ro.property_service.version";
 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 @@ class PropertyServiceConnection {
     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 @@ class PropertyServiceConnection {
   }
 
   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 @@ class PropertyServiceConnection {
     return last_error_ == 0;
   }
 
-  int socket_;
+  ScopedFd socket_;
   int last_error_;
 
   friend class SocketWriter;

libc/private/ScopedFd.h

@@ -0,0 +1,64 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *  * Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *  * Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#pragma once
+
+#include <unistd.h>
+
+#include "private/bionic_macros.h"
+#include "private/ErrnoRestorer.h"
+
+class ScopedFd final {
+ public:
+  explicit ScopedFd(int fd) : fd_(fd) {
+  }
+
+  ScopedFd() : fd_(-1) {
+  }
+
+  ~ScopedFd() {
+    reset(-1);
+  }
+
+  void reset(int fd = -1) {
+    if (fd != -1) {
+      ErrnoRestorer e;
+      close(fd_);
+    }
+    fd_ = fd;
+  }
+
+  int get() const {
+    return fd_;
+  }
+
+ private:
+  int fd_;
+
+  BIONIC_DISALLOW_COPY_AND_ASSIGN(ScopedFd);
+};