Minor updates to fd_file.h

* Add a static IsOpenFd() method for checking if fd is open.
* File::IsOpened() now checks when underlying FD is open (not just valid)
* Centralize a definition for kInvalidFd in art projects.

Test: art_libartbase_tests
Change-Id: Ic5db8b6c80e184308c6ad8979ed31585120eb89f

8 files changed, 78 insertions, 41 deletions

diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index 2f90ec5970..135f0f49af 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -535,9 +535,9 @@ class Dex2Oat final {
       opened_dex_files_maps_(),
       opened_dex_files_(),
       avoid_storing_invocation_(false),
-      swap_fd_(kInvalidFd),
-      app_image_fd_(kInvalidFd),
-      profile_file_fd_(kInvalidFd),
+      swap_fd_(File::kInvalidFd),
+      app_image_fd_(File::kInvalidFd),
+      profile_file_fd_(File::kInvalidFd),
       timings_(timings),
       force_determinism_(false),
       check_linkage_conditions_(false),
@@ -770,7 +770,7 @@ class Dex2Oat final {
     }
 
     const bool have_profile_file = !profile_file_.empty();
-    const bool have_profile_fd = profile_file_fd_ != kInvalidFd;
+    const bool have_profile_fd = profile_file_fd_ != File::kInvalidFd;
     if (have_profile_file && have_profile_fd) {
       Usage("Profile file should not be specified with both --profile-file-fd and --profile-file");
     }
@@ -1723,7 +1723,7 @@ class Dex2Oat final {
 
   // If we need to keep the oat file open for the image writer.
   bool ShouldKeepOatFileOpen() const {
-    return IsImage() && oat_fd_ != kInvalidFd;
+    return IsImage() && oat_fd_ != File::kInvalidFd;
   }
 
   // Doesn't return the class loader since it's not meant to be used for image compilation.
diff --git a/dex2oat/linker/image_test.h b/dex2oat/linker/image_test.h
index 163a9f6676..0b5386a534 100644
--- a/dex2oat/linker/image_test.h
+++ b/dex2oat/linker/image_test.h
@@ -338,7 +338,7 @@ inline void ImageTest::DoCompile(ImageHeader::StorageMode storage_mode,
       }
     }
 
-    bool success_image = writer->Write(kInvalidFd,
+    bool success_image = writer->Write(File::kInvalidFd,
                                        image_filenames,
                                        image_filenames.size());
     ASSERT_TRUE(success_image);
diff --git a/dex2oat/linker/image_writer.cc b/dex2oat/linker/image_writer.cc
index f73f563b7e..f4155bcb58 100644
--- a/dex2oat/linker/image_writer.cc
+++ b/dex2oat/linker/image_writer.cc
@@ -387,10 +387,10 @@ class ImageWriter::ImageFileGuard {
 bool ImageWriter::Write(int image_fd,
                         const std::vector<std::string>& image_filenames,
                         size_t component_count) {
-  // If image_fd or oat_fd are not kInvalidFd then we may have empty strings in image_filenames or
-  // oat_filenames.
+  // If image_fd or oat_fd are not File::kInvalidFd then we may have empty strings in
+  // image_filenames or oat_filenames.
   CHECK(!image_filenames.empty());
-  if (image_fd != kInvalidFd) {
+  if (image_fd != File::kInvalidFd) {
     CHECK_EQ(image_filenames.size(), 1u);
   }
   DCHECK(!oat_filenames_.empty());
@@ -428,7 +428,7 @@ bool ImageWriter::Write(int image_fd,
     const std::string& image_filename = image_filenames[i];
     ImageInfo& image_info = GetImageInfo(i);
     ImageFileGuard image_file;
-    if (image_fd != kInvalidFd) {
+    if (image_fd != File::kInvalidFd) {
       // Ignore image_filename, it is supplied only for better diagnostic.
       image_file.reset(new File(image_fd, unix_file::kCheckSafeUsage));
       // Empty the file in case it already exists.
diff --git a/dex2oat/linker/image_writer.h b/dex2oat/linker/image_writer.h
index 505fbeb539..6f847a24d8 100644
--- a/dex2oat/linker/image_writer.h
+++ b/dex2oat/linker/image_writer.h
@@ -31,6 +31,7 @@
 #include "base/bit_utils.h"
 #include "base/dchecked_vector.h"
 #include "base/enums.h"
+#include "base/unix_file/fd_file.h"
 #include "base/hash_map.h"
 #include "base/hash_set.h"
 #include "base/length_prefixed_array.h"
@@ -70,8 +71,6 @@ class ImTable;
 class ImtConflictTable;
 class TimingLogger;
 
-static constexpr int kInvalidFd = -1;
-
 namespace linker {
 
 // Write a Space built during compilation for use during execution.
@@ -138,9 +137,9 @@ class ImageWriter final {
     return GetImageInfo(oat_index).oat_file_begin_;
   }
 
-  // If image_fd is not kInvalidFd, then we use that for the image file. Otherwise we open
+  // If image_fd is not File::kInvalidFd, then we use that for the image file. Otherwise we open
   // the names in image_filenames.
-  // If oat_fd is not kInvalidFd, then we use that for the oat file. Otherwise we open
+  // If oat_fd is not File::kInvalidFd, then we use that for the oat file. Otherwise we open
   // the names in oat_filenames.
   bool Write(int image_fd,
              const std::vector<std::string>& image_filenames,
diff --git a/libartbase/base/unix_file/fd_file.cc b/libartbase/base/unix_file/fd_file.cc
index 8831b9c6b7..2102fec8cd 100644
--- a/libartbase/base/unix_file/fd_file.cc
+++ b/libartbase/base/unix_file/fd_file.cc
@@ -179,7 +179,7 @@ void FdFile::Destroy() {
     }
     DCHECK_GE(guard_state_, GuardState::kClosed);
   }
-  if (fd_ != -1) {
+  if (fd_ != kInvalidFd) {
     if (Close() != 0) {
       PLOG(WARNING) << "Failed to close file with fd=" << fd_ << " path=" << file_path_;
     }
@@ -197,7 +197,7 @@ FdFile::FdFile(FdFile&& other) noexcept
   }
 #endif
   other.guard_state_ = GuardState::kClosed;
-  other.fd_ = -1;
+  other.fd_ = kInvalidFd;
 }
 
 FdFile& FdFile::operator=(FdFile&& other) noexcept {
@@ -220,7 +220,7 @@ FdFile& FdFile::operator=(FdFile&& other) noexcept {
   }
 #endif
   other.guard_state_ = GuardState::kClosed;
-  other.fd_ = -1;
+  other.fd_ = kInvalidFd;
   return *this;
 }
 
@@ -230,7 +230,7 @@ FdFile::~FdFile() {
 
 int FdFile::Release() {
   int tmp_fd = fd_;
-  fd_ = -1;
+  fd_ = kInvalidFd;
   guard_state_ = GuardState::kNoCheck;
 #if defined(__BIONIC__)
   if (tmp_fd >= 0) {
@@ -243,7 +243,7 @@ int FdFile::Release() {
 void FdFile::Reset(int fd, bool check_usage) {
   CHECK_NE(fd, fd_);
 
-  if (fd_ != -1) {
+  if (fd_ != kInvalidFd) {
     Destroy();
   }
   fd_ = fd;
@@ -255,7 +255,7 @@ void FdFile::Reset(int fd, bool check_usage) {
 #endif
 
   if (check_usage) {
-    guard_state_ = fd == -1 ? GuardState::kNoCheck : GuardState::kBase;
+    guard_state_ = fd == kInvalidFd ? GuardState::kNoCheck : GuardState::kBase;
   } else {
     guard_state_ = GuardState::kNoCheck;
   }
@@ -290,10 +290,10 @@ bool FdFile::Open(const std::string& path, int flags) {
 
 bool FdFile::Open(const std::string& path, int flags, mode_t mode) {
   static_assert(O_RDONLY == 0, "Readonly flag has unexpected value.");
-  DCHECK_EQ(fd_, -1) << path;
+  DCHECK_EQ(fd_, kInvalidFd) << path;
   read_only_mode_ = ((flags & O_ACCMODE) == O_RDONLY);
   fd_ = TEMP_FAILURE_RETRY(open(path.c_str(), flags, mode));
-  if (fd_ == -1) {
+  if (fd_ == kInvalidFd) {
     return false;
   }
 
@@ -336,7 +336,7 @@ int FdFile::Close() {
   }
 #endif
 
-  fd_ = -1;
+  fd_ = kInvalidFd;
   file_path_ = "";
   return 0;
 }
@@ -409,7 +409,7 @@ bool FdFile::CheckUsage() const {
 }
 
 bool FdFile::IsOpened() const {
-  return fd_ >= 0;
+  return FdFile::IsOpenFd(fd_);
 }
 
 static ssize_t ReadIgnoreOffset(int fd, void *buf, size_t count, off_t offset) {
@@ -642,4 +642,18 @@ int FdFile::Compare(FdFile* other) {
   return 0;
 }
 
+bool FdFile::IsOpenFd(int fd) {
+  if (fd == kInvalidFd) {
+    return false;
+  }
+  #ifdef _WIN32  // Windows toolchain does not support F_GETFD.
+    return true;
+  #else
+    int saved_errno = errno;
+    bool is_open = (fcntl(fd, F_GETFD) != -1);
+    errno = saved_errno;
+    return is_open;
+  #endif
+}
+
 }  // namespace unix_file
diff --git a/libartbase/base/unix_file/fd_file.h b/libartbase/base/unix_file/fd_file.h
index 4926e4eeea..39471010df 100644
--- a/libartbase/base/unix_file/fd_file.h
+++ b/libartbase/base/unix_file/fd_file.h
@@ -34,6 +34,8 @@ static constexpr bool kCheckSafeUsage = true;
 // Not thread safe.
 class FdFile : public RandomAccessFile {
  public:
+  static constexpr int kInvalidFd = -1;
+
   FdFile() = default;
   // Creates an FdFile using the given file descriptor.
   // Takes ownership of the file descriptor.
@@ -93,7 +95,13 @@ class FdFile : public RandomAccessFile {
   int Fd() const;
   bool ReadOnlyMode() const;
   bool CheckUsage() const;
+
+  // Check whether the underlying file descriptor refers to an open file.
   bool IsOpened() const;
+
+  // Check whether the numeric value of the underlying file descriptor is valid (Fd() != -1).
+  bool IsValid() const { return fd_ != kInvalidFd; }
+
   const std::string& GetPath() const {
     return file_path_;
   }
@@ -122,18 +130,22 @@ class FdFile : public RandomAccessFile {
   void MarkUnchecked();
 
   // Compare against another file. Returns 0 if the files are equivalent, otherwise returns -1 or 1
-  // depending on if the lenghts are different. If the lengths are the same, the function returns
+  // depending on if the lengths are different. If the lengths are the same, the function returns
   // the difference of the first byte that differs.
   int Compare(FdFile* other);
 
+  // Check that `fd` has a valid value (!= kInvalidFd) and refers to an open file.
+  // On Windows, this call only checks that the value of `fd` is valid .
+  static bool IsOpenFd(int fd);
+
  protected:
-  // If the guard state indicates checking (!=kNoCheck), go to the target state "target". Print the
+  // If the guard state indicates checking (!=kNoCheck), go to the target state `target`. Print the
   // given warning if the current state is or exceeds warn_threshold.
   void moveTo(GuardState target, GuardState warn_threshold, const char* warning);
 
-  // If the guard state indicates checking (<kNoCheck), and is below the target state "target", go
-  // to "target." If the current state is higher (excluding kNoCheck) than the trg state, print the
-  // warning.
+  // If the guard state indicates checking (<kNoCheck), and is below the target state `target`, go
+  // to `target`. If the current state is higher (excluding kNoCheck) than the target state, print
+  // the warning.
   void moveUp(GuardState target, const char* warning);
 
   // Forcefully sets the state to the given one. This can overwrite kNoCheck.
@@ -145,7 +157,7 @@ class FdFile : public RandomAccessFile {
 
   GuardState guard_state_ = GuardState::kClosed;
 
-  // Opens file 'file_path' using 'flags' and 'mode'.
+  // Opens file `file_path` using `flags` and `mode`.
   bool Open(const std::string& file_path, int flags);
   bool Open(const std::string& file_path, int flags, mode_t mode);
 
@@ -155,7 +167,7 @@ class FdFile : public RandomAccessFile {
 
   void Destroy();  // For ~FdFile and operator=(&&).
 
-  int fd_ = -1;
+  int fd_ = kInvalidFd;
   std::string file_path_;
   bool read_only_mode_ = false;
 
diff --git a/libartbase/base/unix_file/fd_file_test.cc b/libartbase/base/unix_file/fd_file_test.cc
index 3a9cf59148..f5933370da 100644
--- a/libartbase/base/unix_file/fd_file_test.cc
+++ b/libartbase/base/unix_file/fd_file_test.cc
@@ -46,11 +46,25 @@ TEST_F(FdFileTest, Write) {
 
 TEST_F(FdFileTest, UnopenedFile) {
   FdFile file;
-  EXPECT_EQ(-1, file.Fd());
+  EXPECT_EQ(FdFile::kInvalidFd, file.Fd());
   EXPECT_FALSE(file.IsOpened());
   EXPECT_TRUE(file.GetPath().empty());
 }
 
+TEST_F(FdFileTest, IsOpenFd) {
+  art::ScratchFile scratch_file;
+  FdFile* file = scratch_file.GetFile();
+  ASSERT_TRUE(file->IsOpened());
+  EXPECT_GE(file->Fd(), 0);
+  EXPECT_NE(file->Fd(), FdFile::kInvalidFd);
+  EXPECT_TRUE(FdFile::IsOpenFd(file->Fd()));
+  int old_fd = file->Fd();
+  ASSERT_TRUE(file != nullptr);
+  ASSERT_EQ(file->FlushClose(), 0);
+  EXPECT_FALSE(file->IsOpened());
+  EXPECT_FALSE(FdFile::IsOpenFd(old_fd));
+}
+
 TEST_F(FdFileTest, OpenClose) {
   std::string good_path(GetTmpPath("some-file.txt"));
   FdFile file(good_path, O_CREAT | O_WRONLY, true);
@@ -60,7 +74,7 @@ TEST_F(FdFileTest, OpenClose) {
   EXPECT_FALSE(file.ReadOnlyMode());
   EXPECT_EQ(0, file.Flush());
   EXPECT_EQ(0, file.Close());
-  EXPECT_EQ(-1, file.Fd());
+  EXPECT_EQ(FdFile::kInvalidFd, file.Fd());
   EXPECT_FALSE(file.IsOpened());
   FdFile file2(good_path, O_RDONLY, true);
   EXPECT_TRUE(file2.IsOpened());
diff --git a/profman/profman.cc b/profman/profman.cc
index b2fabb274b..4eae557343 100644
--- a/profman/profman.cc
+++ b/profman/profman.cc
@@ -72,10 +72,8 @@ static std::string CommandLine() {
   return android::base::Join(command, ' ');
 }
 
-static constexpr int kInvalidFd = -1;
-
 static bool FdIsValid(int fd) {
-  return fd != kInvalidFd;
+  return fd != File::kInvalidFd;
 }
 
 static void UsageErrorV(const char* fmt, va_list ap) {
@@ -273,12 +271,12 @@ static void ParseBoolOption(const char* raw_option,
 class ProfMan final {
  public:
   ProfMan() :
-      reference_profile_file_fd_(kInvalidFd),
+      reference_profile_file_fd_(File::kInvalidFd),
       dump_only_(false),
       dump_classes_and_methods_(false),
       generate_boot_image_profile_(false),
       generate_boot_profile_(false),
-      dump_output_to_fd_(kInvalidFd),
+      dump_output_to_fd_(File::kInvalidFd),
       test_profile_num_dex_(kDefaultTestProfileNumDex),
       test_profile_method_percerntage_(kDefaultTestProfileMethodPercentage),
       test_profile_class_percentage_(kDefaultTestProfileClassPercentage),
@@ -706,7 +704,7 @@ class ProfMan final {
       }
     }
     for (const std::string& profile_file : profile_files_) {
-      int ret = DumpOneProfile(kOrdinaryProfile, profile_file, kInvalidFd, &dex_files, &dump);
+      int ret = DumpOneProfile(kOrdinaryProfile, profile_file, File::kInvalidFd, &dex_files, &dump);
       if (ret != 0) {
         return ret;
       }
@@ -725,7 +723,7 @@ class ProfMan final {
     if (!reference_profile_file_.empty()) {
       int ret = DumpOneProfile(kReferenceProfile,
                                reference_profile_file_,
-                               kInvalidFd,
+                               File::kInvalidFd,
                                &dex_files,
                                &dump);
       if (ret != 0) {
@@ -1249,7 +1247,7 @@ class ProfMan final {
       fd = open(reference_profile_file_.c_str(), flags, 0644);
       if (fd < 0) {
         PLOG(ERROR) << "Cannot open " << reference_profile_file_;
-        return kInvalidFd;
+        return File::kInvalidFd;
       }
     }
     return fd;