ART: Add FdFile constructors
Make Open protected, and expose constructors instead. Add a move
constructor and move assignment operator.
Add OS functions that return the FdFile non-pointer version.
Add tests.
Bug: 21192156
Test: m test-art-host
Test: m test-art-target (shamu)
Change-Id: I83e390edde7cd37c900e9d5c3e4d21da22981b3f
diff --git a/runtime/base/unix_file/fd_file.cc b/runtime/base/unix_file/fd_file.cc
index e4097dd..6f0e125 100644
--- a/runtime/base/unix_file/fd_file.cc
+++ b/runtime/base/unix_file/fd_file.cc
@@ -53,7 +53,15 @@
fd_(fd), file_path_(path), auto_close_(true), read_only_mode_(read_only_mode) {
}
-FdFile::~FdFile() {
+FdFile::FdFile(const std::string& path, int flags, mode_t mode, bool check_usage)
+ : fd_(-1), auto_close_(true) {
+ Open(path, flags, mode);
+ if (!check_usage || !IsOpened()) {
+ guard_state_ = GuardState::kNoCheck;
+ }
+}
+
+void FdFile::Destroy() {
if (kCheckSafeUsage && (guard_state_ < GuardState::kNoCheck)) {
if (guard_state_ < GuardState::kFlushed) {
LOG(::art::ERROR) << "File " << file_path_ << " wasn't explicitly flushed before destruction.";
@@ -70,6 +78,28 @@
}
}
+FdFile& FdFile::operator=(FdFile&& other) {
+ if (this == &other) {
+ return *this;
+ }
+
+ if (this->fd_ != other.fd_) {
+ Destroy(); // Free old state.
+ }
+
+ guard_state_ = other.guard_state_;
+ fd_ = other.fd_;
+ file_path_ = std::move(other.file_path_);
+ auto_close_ = other.auto_close_;
+ other.Release(); // Release other.
+
+ return *this;
+}
+
+FdFile::~FdFile() {
+ Destroy();
+}
+
void FdFile::moveTo(GuardState target, GuardState warn_threshold, const char* warning) {
if (kCheckSafeUsage) {
if (guard_state_ < GuardState::kNoCheck) {
diff --git a/runtime/base/unix_file/fd_file.h b/runtime/base/unix_file/fd_file.h
index 16cd44f..d896ee9 100644
--- a/runtime/base/unix_file/fd_file.h
+++ b/runtime/base/unix_file/fd_file.h
@@ -18,7 +18,9 @@
#define ART_RUNTIME_BASE_UNIX_FILE_FD_FILE_H_
#include <fcntl.h>
+
#include <string>
+
#include "base/unix_file/random_access_file.h"
#include "base/macros.h"
@@ -39,6 +41,46 @@
FdFile(int fd, const std::string& path, bool checkUsage);
FdFile(int fd, const std::string& path, bool checkUsage, bool read_only_mode);
+ FdFile(const std::string& path, int flags, bool checkUsage)
+ : FdFile(path, flags, 0640, checkUsage) {}
+ FdFile(const std::string& path, int flags, mode_t mode, bool checkUsage);
+
+ // Move constructor.
+ FdFile(FdFile&& other)
+ : guard_state_(other.guard_state_),
+ fd_(other.fd_),
+ file_path_(std::move(other.file_path_)),
+ auto_close_(other.auto_close_),
+ read_only_mode_(other.read_only_mode_) {
+ other.Release(); // Release the src.
+ }
+
+ // Move assignment operator.
+ FdFile& operator=(FdFile&& other);
+
+ // Release the file descriptor. This will make further accesses to this FdFile invalid. Disables
+ // all further state checking.
+ int Release() {
+ int tmp_fd = fd_;
+ fd_ = -1;
+ guard_state_ = GuardState::kNoCheck;
+ auto_close_ = false;
+ return tmp_fd;
+ }
+
+ void Reset(int fd, bool check_usage) {
+ if (fd_ != -1 && fd_ != fd) {
+ Destroy();
+ }
+ fd_ = fd;
+ if (check_usage) {
+ guard_state_ = fd == -1 ? GuardState::kNoCheck : GuardState::kBase;
+ } else {
+ guard_state_ = GuardState::kNoCheck;
+ }
+ // Keep the auto_close_ state.
+ }
+
// Destroys an FdFile, closing the file descriptor if Close hasn't already
// been called. (If you care about the return value of Close, call it
// yourself; this is meant to handle failure cases and read-only accesses.
@@ -46,10 +88,6 @@
// guarantee that data actually made it to stable storage.)
virtual ~FdFile();
- // 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);
-
// RandomAccessFile API.
int Close() OVERRIDE WARN_UNUSED;
int64_t Read(char* buf, int64_t byte_count, int64_t offset) const OVERRIDE WARN_UNUSED;
@@ -119,10 +157,16 @@
GuardState guard_state_;
+ // 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);
+
private:
template <bool kUseOffset>
bool WriteFullyGeneric(const void* buffer, size_t byte_count, size_t offset);
+ void Destroy(); // For ~FdFile and operator=(&&).
+
int fd_;
std::string file_path_;
bool auto_close_;
diff --git a/runtime/base/unix_file/fd_file_test.cc b/runtime/base/unix_file/fd_file_test.cc
index 9bc87e5..db3a44f 100644
--- a/runtime/base/unix_file/fd_file_test.cc
+++ b/runtime/base/unix_file/fd_file_test.cc
@@ -49,29 +49,28 @@
TEST_F(FdFileTest, OpenClose) {
std::string good_path(GetTmpPath("some-file.txt"));
- FdFile file;
- ASSERT_TRUE(file.Open(good_path, O_CREAT | O_WRONLY));
+ FdFile file(good_path, O_CREAT | O_WRONLY, true);
+ ASSERT_TRUE(file.IsOpened());
EXPECT_GE(file.Fd(), 0);
EXPECT_TRUE(file.IsOpened());
EXPECT_EQ(0, file.Flush());
EXPECT_EQ(0, file.Close());
EXPECT_EQ(-1, file.Fd());
EXPECT_FALSE(file.IsOpened());
- EXPECT_TRUE(file.Open(good_path, O_RDONLY));
- EXPECT_GE(file.Fd(), 0);
- EXPECT_TRUE(file.IsOpened());
+ FdFile file2(good_path, O_RDONLY, true);
+ EXPECT_TRUE(file2.IsOpened());
+ EXPECT_GE(file2.Fd(), 0);
- ASSERT_EQ(file.Close(), 0);
+ ASSERT_EQ(file2.Close(), 0);
ASSERT_EQ(unlink(good_path.c_str()), 0);
}
TEST_F(FdFileTest, ReadFullyEmptyFile) {
// New scratch file, zero-length.
art::ScratchFile tmp;
- FdFile file;
- ASSERT_TRUE(file.Open(tmp.GetFilename(), O_RDONLY));
+ FdFile file(tmp.GetFilename(), O_RDONLY, false);
+ ASSERT_TRUE(file.IsOpened());
EXPECT_GE(file.Fd(), 0);
- EXPECT_TRUE(file.IsOpened());
uint8_t buffer[16];
EXPECT_FALSE(file.ReadFully(&buffer, 4));
}
@@ -84,10 +83,9 @@
TEST_F(FdFileTest, ReadFullyWithOffset) {
// New scratch file, zero-length.
art::ScratchFile tmp;
- FdFile file;
- ASSERT_TRUE(file.Open(tmp.GetFilename(), O_RDWR));
+ FdFile file(tmp.GetFilename(), O_RDWR, false);
+ ASSERT_TRUE(file.IsOpened());
EXPECT_GE(file.Fd(), 0);
- EXPECT_TRUE(file.IsOpened());
char ignore_prefix[20] = {'a', };
NullTerminateCharArray(ignore_prefix);
@@ -113,9 +111,8 @@
TEST_F(FdFileTest, ReadWriteFullyWithOffset) {
// New scratch file, zero-length.
art::ScratchFile tmp;
- FdFile file;
- ASSERT_TRUE(file.Open(tmp.GetFilename(), O_RDWR));
- EXPECT_GE(file.Fd(), 0);
+ FdFile file(tmp.GetFilename(), O_RDWR, false);
+ ASSERT_GE(file.Fd(), 0);
EXPECT_TRUE(file.IsOpened());
const char* test_string = "This is a test string";
@@ -140,8 +137,7 @@
TEST_F(FdFileTest, Copy) {
art::ScratchFile src_tmp;
- FdFile src;
- ASSERT_TRUE(src.Open(src_tmp.GetFilename(), O_RDWR));
+ FdFile src(src_tmp.GetFilename(), O_RDWR, false);
ASSERT_GE(src.Fd(), 0);
ASSERT_TRUE(src.IsOpened());
@@ -151,8 +147,7 @@
ASSERT_EQ(static_cast<int64_t>(sizeof(src_data)), src.GetLength());
art::ScratchFile dest_tmp;
- FdFile dest;
- ASSERT_TRUE(dest.Open(src_tmp.GetFilename(), O_RDWR));
+ FdFile dest(src_tmp.GetFilename(), O_RDWR, false);
ASSERT_GE(dest.Fd(), 0);
ASSERT_TRUE(dest.IsOpened());
@@ -168,4 +163,22 @@
ASSERT_EQ(0, src.Close());
}
+TEST_F(FdFileTest, MoveConstructor) {
+ // New scratch file, zero-length.
+ art::ScratchFile tmp;
+ FdFile file(tmp.GetFilename(), O_RDWR, false);
+ ASSERT_TRUE(file.IsOpened());
+ EXPECT_GE(file.Fd(), 0);
+
+ int old_fd = file.Fd();
+
+ FdFile file2(std::move(file));
+ EXPECT_FALSE(file.IsOpened());
+ EXPECT_TRUE(file2.IsOpened());
+ EXPECT_EQ(old_fd, file2.Fd());
+
+ ASSERT_EQ(file2.Flush(), 0);
+ ASSERT_EQ(file2.Close(), 0);
+}
+
} // namespace unix_file
diff --git a/runtime/os_linux.cc b/runtime/os_linux.cc
index f45e9f6..1d1413b 100644
--- a/runtime/os_linux.cc
+++ b/runtime/os_linux.cc
@@ -53,8 +53,9 @@
File* OS::OpenFileWithFlags(const char* name, int flags) {
CHECK(name != nullptr);
- std::unique_ptr<File> file(new File);
- if (!file->Open(name, flags, 0666)) {
+ bool read_only = (flags == O_RDONLY);
+ std::unique_ptr<File> file(new File(name, flags, 0666, !read_only));
+ if (!file->IsOpened()) {
return nullptr;
}
return file.release();
diff --git a/runtime/trace.cc b/runtime/trace.cc
index b879355..0acc54d 100644
--- a/runtime/trace.cc
+++ b/runtime/trace.cc
@@ -715,8 +715,8 @@
std::string header(os.str());
if (trace_output_mode_ == TraceOutputMode::kStreaming) {
- File file;
- if (!file.Open(streaming_file_name_ + ".sec", O_CREAT | O_WRONLY)) {
+ File file(streaming_file_name_ + ".sec", O_CREAT | O_WRONLY, true);
+ if (!file.IsOpened()) {
LOG(WARNING) << "Could not open secondary trace file!";
return;
}
diff --git a/runtime/utils.cc b/runtime/utils.cc
index 6a50b8e..3f779df 100644
--- a/runtime/utils.cc
+++ b/runtime/utils.cc
@@ -136,8 +136,8 @@
}
bool ReadFileToString(const std::string& file_name, std::string* result) {
- File file;
- if (!file.Open(file_name, O_RDONLY)) {
+ File file(file_name, O_RDONLY, false);
+ if (!file.IsOpened()) {
return false;
}
@@ -155,8 +155,8 @@
}
bool PrintFileToLog(const std::string& file_name, LogSeverity level) {
- File file;
- if (!file.Open(file_name, O_RDONLY)) {
+ File file(file_name, O_RDONLY, false);
+ if (!file.IsOpened()) {
return false;
}
