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
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index 2f90ec5..135f0f4 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -535,9 +535,9 @@
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 @@
}
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 @@
// 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 163a9f6..0b5386a 100644
--- a/dex2oat/linker/image_test.h
+++ b/dex2oat/linker/image_test.h
@@ -338,7 +338,7 @@
}
}
- 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 f73f563..f4155bc 100644
--- a/dex2oat/linker/image_writer.cc
+++ b/dex2oat/linker/image_writer.cc
@@ -387,10 +387,10 @@
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 @@
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 505fbeb..6f847a2 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 ImtConflictTable;
class TimingLogger;
-static constexpr int kInvalidFd = -1;
-
namespace linker {
// Write a Space built during compilation for use during execution.
@@ -138,9 +137,9 @@
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 8831b9c..2102fec 100644
--- a/libartbase/base/unix_file/fd_file.cc
+++ b/libartbase/base/unix_file/fd_file.cc
@@ -179,7 +179,7 @@
}
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 @@
}
#endif
other.guard_state_ = GuardState::kClosed;
- other.fd_ = -1;
+ other.fd_ = kInvalidFd;
}
FdFile& FdFile::operator=(FdFile&& other) noexcept {
@@ -220,7 +220,7 @@
}
#endif
other.guard_state_ = GuardState::kClosed;
- other.fd_ = -1;
+ other.fd_ = kInvalidFd;
return *this;
}
@@ -230,7 +230,7 @@
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 @@
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 @@
#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, 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 @@
}
#endif
- fd_ = -1;
+ fd_ = kInvalidFd;
file_path_ = "";
return 0;
}
@@ -409,7 +409,7 @@
}
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 @@
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 4926e4e..3947101 100644
--- a/libartbase/base/unix_file/fd_file.h
+++ b/libartbase/base/unix_file/fd_file.h
@@ -34,6 +34,8 @@
// 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 @@
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 @@
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 @@
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 @@
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 3a9cf59..f593337 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, 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 @@
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 b2fabb2..4eae557 100644
--- a/profman/profman.cc
+++ b/profman/profman.cc
@@ -72,10 +72,8 @@
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 @@
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 @@
}
}
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 @@
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 @@
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;