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

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