From b64decdc7361c6c93bd91fdd016a50971c8e537a Mon Sep 17 00:00:00 2001
From: David Brazdil <dbrazdil@google.com>
Date: Tue, 9 Aug 2016 12:10:56 +0100
Subject: [PATCH] Fix setting FdFile::ReadOnlyMode() flag

The Unix flag O_RDONLY is defined as zero and hence its presence
cannot be tested with '(flags & O_RDONLY) != 0'. This used to be
broken in FdFlag when setting its internal `read_only_mode_` flag.

Test: m test-art-host-gtest-fd_file_test
Change-Id: Ib48abfc908c7032f031450a1574130e06f6c3bab
---
 runtime/base/unix_file/fd_file.cc      | 4 ++--
 runtime/base/unix_file/fd_file_test.cc | 7 ++++++-
 runtime/os_linux.cc                    | 2 +-
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/runtime/base/unix_file/fd_file.cc b/runtime/base/unix_file/fd_file.cc
index 6f0e1251d..48e3ceb06 100644
--- a/runtime/base/unix_file/fd_file.cc
+++ b/runtime/base/unix_file/fd_file.cc
@@ -132,14 +132,14 @@ 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.");
   CHECK_EQ(fd_, -1) << path;
-  read_only_mode_ = (flags & O_RDONLY) != 0;
+  read_only_mode_ = ((flags & O_ACCMODE) == O_RDONLY);
   fd_ = TEMP_FAILURE_RETRY(open(path.c_str(), flags, mode));
   if (fd_ == -1) {
     return false;
   }
   file_path_ = path;
-  static_assert(O_RDONLY == 0, "Readonly flag has unexpected value.");
   if (kCheckSafeUsage && (flags & (O_RDWR | O_CREAT | O_WRONLY)) != 0) {
     // Start in the base state (not flushed, not closed).
     guard_state_ = GuardState::kBase;
diff --git a/runtime/base/unix_file/fd_file_test.cc b/runtime/base/unix_file/fd_file_test.cc
index db3a44f9b..99ef6f73b 100644
--- a/runtime/base/unix_file/fd_file_test.cc
+++ b/runtime/base/unix_file/fd_file_test.cc
@@ -53,12 +53,14 @@ TEST_F(FdFileTest, OpenClose) {
   ASSERT_TRUE(file.IsOpened());
   EXPECT_GE(file.Fd(), 0);
   EXPECT_TRUE(file.IsOpened());
+  EXPECT_FALSE(file.ReadOnlyMode());
   EXPECT_EQ(0, file.Flush());
   EXPECT_EQ(0, file.Close());
   EXPECT_EQ(-1, file.Fd());
   EXPECT_FALSE(file.IsOpened());
-  FdFile file2(good_path,  O_RDONLY, true);
+  FdFile file2(good_path, O_RDONLY, true);
   EXPECT_TRUE(file2.IsOpened());
+  EXPECT_TRUE(file2.ReadOnlyMode());
   EXPECT_GE(file2.Fd(), 0);
 
   ASSERT_EQ(file2.Close(), 0);
@@ -70,6 +72,7 @@ TEST_F(FdFileTest, ReadFullyEmptyFile) {
   art::ScratchFile tmp;
   FdFile file(tmp.GetFilename(), O_RDONLY, false);
   ASSERT_TRUE(file.IsOpened());
+  EXPECT_TRUE(file.ReadOnlyMode());
   EXPECT_GE(file.Fd(), 0);
   uint8_t buffer[16];
   EXPECT_FALSE(file.ReadFully(&buffer, 4));
@@ -86,6 +89,7 @@ TEST_F(FdFileTest, ReadFullyWithOffset) {
   FdFile file(tmp.GetFilename(), O_RDWR, false);
   ASSERT_TRUE(file.IsOpened());
   EXPECT_GE(file.Fd(), 0);
+  EXPECT_FALSE(file.ReadOnlyMode());
 
   char ignore_prefix[20] = {'a', };
   NullTerminateCharArray(ignore_prefix);
@@ -114,6 +118,7 @@ TEST_F(FdFileTest, ReadWriteFullyWithOffset) {
   FdFile file(tmp.GetFilename(), O_RDWR, false);
   ASSERT_GE(file.Fd(), 0);
   EXPECT_TRUE(file.IsOpened());
+  EXPECT_FALSE(file.ReadOnlyMode());
 
   const char* test_string = "This is a test string";
   size_t length = strlen(test_string) + 1;
diff --git a/runtime/os_linux.cc b/runtime/os_linux.cc
index 1d1413bd7..1db09b444 100644
--- a/runtime/os_linux.cc
+++ b/runtime/os_linux.cc
@@ -53,7 +53,7 @@ File* OS::CreateEmptyFileWriteOnly(const char* name) {
 
 File* OS::OpenFileWithFlags(const char* name, int flags) {
   CHECK(name != nullptr);
-  bool read_only = (flags == O_RDONLY);
+  bool read_only = ((flags & O_ACCMODE) == O_RDONLY);
   std::unique_ptr<File> file(new File(name, flags, 0666, !read_only));
   if (!file->IsOpened()) {
     return nullptr;
-- 
2.11.0