Revert^2 "Move map_ptr to incfs namspace"
f6a4b40998b6fa71ac08a5fa4c9a9d92fa04b1f0
Change-Id: I6b7d8181c7d68e0da3d6505d3de95269ec7b471b
Merged-In: I6b7d8181c7d68e0da3d6505d3de95269ec7b471b
diff --git a/incfs/tests/util/map_ptr_test.cpp b/incfs/tests/util/map_ptr_test.cpp
index 3ca7fb0..4ed6d0f 100644
--- a/incfs/tests/util/map_ptr_test.cpp
+++ b/incfs/tests/util/map_ptr_test.cpp
@@ -76,8 +76,8 @@
int32_t getReadTimeout() override { return 1; }
- std::unique_ptr<util::IncFsFileMap> GetFileMap(off64_t offset, size_t length) {
- auto map = std::make_unique<util::IncFsFileMap>();
+ std::unique_ptr<IncFsFileMap> GetFileMap(off64_t offset, size_t length) {
+ auto map = std::make_unique<IncFsFileMap>();
return map->Create(fd_, offset, length, nullptr) ? std::move(map) : nullptr;
}
@@ -104,6 +104,22 @@
ASSERT_EQ(1U, p2->second);
}
+TEST_F(MapPtrTest, ReadNull) {
+ auto map = GetFileMap(0U /* offset */, FILE_SIZE);
+ ASSERT_NE(nullptr, map);
+
+ auto p1 = map->data<uint32_t>();
+ ASSERT_TRUE(p1);
+ ASSERT_EQ(0U, p1.value());
+
+ p1 = nullptr;
+ ASSERT_FALSE(p1);
+
+ p1 = map->data<uint32_t>();
+ ASSERT_TRUE(p1);
+ ASSERT_EQ(0U, p1.value());
+}
+
TEST_F(MapPtrTest, ReadAtStartWithOffset) {
auto map = GetFileMap(sizeof(uint32_t) * 4U /* offset */, FILE_SIZE);
ASSERT_NE(nullptr, map);
@@ -183,7 +199,7 @@
auto map = GetFileMap(0U /* offset */, FILE_SIZE);
ASSERT_NE(nullptr, map);
- auto p1 = map->data().offset<TwoValues>(11U * sizeof(uint32_t));
+ auto p1 = map->data().offset(11U * sizeof(uint32_t)).convert<TwoValues>();
ASSERT_TRUE(p1);
ASSERT_EQ(11U, p1->first);
ASSERT_EQ(12U, p1->second);
@@ -245,7 +261,7 @@
ASSERT_NE(nullptr, map);
auto missing_page_start = INCFS_DATA_FILE_BLOCK_SIZE * FILE_MISSING_PAGE;
- auto p1 = map->data().offset<uint32_t>(missing_page_start - off);
+ auto p1 = map->data().offset(missing_page_start - off).convert<uint32_t>();
ASSERT_FALSE(p1);
ASSERT_SIGBUS(p1.value());
@@ -281,7 +297,7 @@
ASSERT_SIGBUS(p6.value());
auto missing_page_end = INCFS_DATA_FILE_BLOCK_SIZE * (FILE_MISSING_PAGE + 1);
- auto p8 = map->data().offset<uint32_t>(missing_page_end - off);
+ auto p8 = map->data().offset(missing_page_end - off).convert<uint32_t>();
ASSERT_TRUE(p8);
ASSERT_EQ(4096U, p8.value());
diff --git a/incfs/util/include/util/map_ptr.h b/incfs/util/include/util/map_ptr.h
index 4f25ead..c5780f9 100644
--- a/incfs/util/include/util/map_ptr.h
+++ b/incfs/util/include/util/map_ptr.h
@@ -30,7 +30,16 @@
class FileMap;
-namespace incfs::util {
+namespace incfs {
+
+// Controls whether not verifying the presence of data before de-referencing the pointer aborts
+// program execution.
+#define LIBINCFS_MAP_PTR_DEBUG false
+#if LIBINCFS_MAP_PTR_DEBUG
+#define LIBINCFS_MAP_PTR_DEBUG_CODE(x) x
+#else
+#define LIBINCFS_MAP_PTR_DEBUG_CODE(x)
+#endif
template <typename T, bool Verified = false>
struct map_ptr;
@@ -43,18 +52,15 @@
//
// This always uses MAP_SHARED.
class IncFsFileMap final {
- // Controls whether not verifying the presence of data before de-referencing the pointer aborts
- // program execution.
- static constexpr bool DEBUG = false;
-
template <typename, bool>
friend struct map_ptr;
-
using bucket_t = uint8_t;
static constexpr size_t kBucketBits = sizeof(bucket_t) * 8U;
public:
IncFsFileMap();
+ IncFsFileMap(IncFsFileMap&&) noexcept;
+ IncFsFileMap& operator =(IncFsFileMap&&) noexcept;
~IncFsFileMap();
// Initializes the map. Does not take ownership of the file descriptor.
@@ -73,6 +79,8 @@
const char* file_name() const;
private:
+ DISALLOW_COPY_AND_ASSIGN(IncFsFileMap);
+
// Returns whether pointers created from this map should run verification of data presence
// to protect against SIGBUS signals.
bool IsVerificationEnabled() const;
@@ -86,7 +94,7 @@
// File descriptor of the memory-mapped file (not owned).
int fd_ = -1;
size_t start_block_offset_ = 0;
- const uint8_t* start_block_ptr_ = 0;
+ const uint8_t* start_block_ptr_ = nullptr;
std::unique_ptr<android::FileMap> map_;
@@ -148,6 +156,12 @@
return safe_ptr_ - other.safe_ptr_;
}
+ const_iterator operator+(int n) const {
+ const_iterator other = *this;
+ other += n;
+ return other;
+ }
+
reference operator*() const { return safe_ptr_; }
const const_iterator& operator++() {
@@ -199,10 +213,10 @@
// Implicit conversion from regular raw pointer
map_ptr& operator=(const T* ptr) {
- map_ = nullptr;
ptr_ = ptr;
+ map_ = nullptr;
verified_block_ = nullptr;
- verified_ = Verified;
+ LIBINCFS_MAP_PTR_DEBUG_CODE(verified_ = Verified);
return *this;
}
@@ -212,10 +226,10 @@
// Copy assignment operator
template <bool V2, bool V1 = Verified, IsUnverified<V1> = 0, IsVerified<V2> = 0>
map_ptr& operator=(const map_ptr<T, V2>& other) {
- map_ = other.map_;
ptr_ = other.ptr_;
+ map_ = other.map_;
verified_block_ = other.verified_block_;
- verified_ = other.verified_;
+ LIBINCFS_MAP_PTR_DEBUG_CODE(verified_ = other.verified_);
return *this;
}
@@ -246,16 +260,15 @@
// Retrieves a map_ptr<T> offset from an original map_ptr<U> by the specified number of `offset`
// bytes.
- template <typename U>
- map_ptr<U> offset(std::ptrdiff_t offset) const {
- return map_ptr<U>(map_,
- reinterpret_cast<const U*>(reinterpret_cast<const uint8_t*>(ptr_) +
+ map_ptr<T> offset(std::ptrdiff_t offset) const {
+ return map_ptr<T>(map_,
+ reinterpret_cast<const T*>(reinterpret_cast<const uint8_t*>(ptr_) +
offset),
verified_block_);
}
// Returns a raw pointer to the value of this pointer.
- const T* unsafe() const { return ptr_; }
+ const T* unsafe_ptr() const { return ptr_; }
// Start T == void methods
@@ -267,19 +280,24 @@
// End T == void methods
// Start T != void methods
+ template <typename T1 = T, NotVoid<T1> = 0, bool V1 = Verified, IsUnverified<V1> = 0>
+ operator bool() const {
+ return verify();
+ }
+
+ template <typename T1 = T, NotVoid<T1> = 0, bool V1 = Verified, IsVerified<V1> = 0>
+ operator bool() const {
+ return ptr_ != nullptr;
+ }
+
template <typename T1 = T, NotVoid<T1> = 0>
const_iterator iterator() const {
return const_iterator(*this);
}
template <typename T1 = T, NotVoid<T1> = 0>
- operator bool() const {
- return Verified ? ptr_ != nullptr : verify() != nullptr;
- }
-
- template <typename T1 = T, NotVoid<T1> = 0>
const map_ptr<T1>& operator++() {
- verified_ = false;
+ LIBINCFS_MAP_PTR_DEBUG_CODE(verified_ = false);
++ptr_;
return *this;
}
@@ -287,7 +305,7 @@
template <typename T1 = T, NotVoid<T1> = 0>
const map_ptr<T1> operator++(int) {
map_ptr<T1> temp = *this;
- verified_ = false;
+ LIBINCFS_MAP_PTR_DEBUG_CODE(verified_ = false);
++ptr_;
return temp;
}
@@ -306,53 +324,56 @@
// The caller should verify the presence of the pointer data before calling this method.
template <typename T1 = T, NotVoid<T1> = 0>
const T1& value() const {
- CHECK(!IncFsFileMap::DEBUG || verified_)
- << "Did not verify presence before de-referencing safe pointer";
+ LIBINCFS_MAP_PTR_DEBUG_CODE(
+ CHECK(verified_) << "Did not verify presence before de-referencing safe pointer");
return *ptr_;
}
// Returns a raw pointer to the value this pointer.
// The caller should verify the presence of the pointer data before calling this method.
template <typename T1 = T, NotVoid<T1> = 0>
- const T1* const& operator->() const {
- CHECK(!IncFsFileMap::DEBUG || verified_)
- << "Did not verify presence before de-referencing safe pointer";
+ const T1* operator->() const {
+ LIBINCFS_MAP_PTR_DEBUG_CODE(
+ CHECK(verified_) << "Did not verify presence before de-referencing safe pointer");
return ptr_;
}
// Verifies the presence of `n` elements of `T`.
//
- // Returns a raw pointer to the value of this pointer if the elements are completely present;
- // otherwise, returns
- // nullptr.
- template <typename N = int, typename T1 = T, NotVoid<T1> = 0>
- const T1* verify(N n = 1) const {
- verified_ = true;
-
+ // Returns true if the elements are completely present; otherwise, returns false.
+ template <typename T1 = T, NotVoid<T1> = 0, bool V1 = Verified, IsUnverified<V1> = 0>
+ bool verify(size_t n = 1) const {
#ifdef __ANDROID__
- if (!map_) {
- return ptr_;
+ if (LIKELY(map_ == nullptr)) {
+ return ptr_ != nullptr;
}
+ if (ptr_ == nullptr) {
+ return false;
+ }
+
+ const size_t verify_size = sizeof(T) * n;
+ LIBINCFS_MAP_PTR_DEBUG_CODE(if (sizeof(T) <= verify_size) verified_ = true;);
+
const auto data_start = reinterpret_cast<const uint8_t*>(ptr_);
- const auto data_end = reinterpret_cast<const uint8_t*>(ptr_ + n);
+ const auto data_end = reinterpret_cast<const uint8_t*>(ptr_) + verify_size;
// If the data is entirely within the block beginning at the previous verified block
// pointer, then the data can safely be used.
if (LIKELY(data_start >= verified_block_ &&
data_end <= verified_block_ + INCFS_DATA_FILE_BLOCK_SIZE)) {
- return ptr_;
+ return true;
}
if (LIKELY(map_->Verify(data_start, data_end, &verified_block_))) {
- return ptr_;
+ return true;
}
- verified_ = false;
- return nullptr;
+ LIBINCFS_MAP_PTR_DEBUG_CODE(verified_ = false);
+ return false;
#else
(void)n;
- return ptr_;
+ return true;
#endif
}
@@ -360,12 +381,9 @@
// The caller should verify the presence of the pointer data before calling this method.
template <typename T1 = T, NotVoid<T1> = 0>
verified_map_ptr<T1> verified() const {
- CHECK(!IncFsFileMap::DEBUG || verified_)
- << "Did not verify presence before de-referencing safe pointer";
return verified_map_ptr<T1>(map_, ptr_, verified_block_);
}
- // End T != void type methods
private:
map_ptr(const IncFsFileMap* map, const T* ptr)
: ptr_(ptr), map_(map), verified_block_(nullptr) {}
@@ -375,9 +393,9 @@
const T* ptr_ = nullptr;
mutable const IncFsFileMap* map_ = nullptr;
mutable const uint8_t* verified_block_;
- mutable bool verified_ = Verified;
+ LIBINCFS_MAP_PTR_DEBUG_CODE(mutable bool verified_ = Verified);
};
-} // namespace incfs::util
+} // namespace incfs
} // namespace android
\ No newline at end of file
diff --git a/incfs/util/map_ptr.cpp b/incfs/util/map_ptr.cpp
index f32bc89..7b0310e 100644
--- a/incfs/util/map_ptr.cpp
+++ b/incfs/util/map_ptr.cpp
@@ -24,9 +24,11 @@
#include "util/map_ptr.h"
-namespace android::incfs::util {
+namespace android::incfs {
IncFsFileMap::IncFsFileMap() = default;
+IncFsFileMap::IncFsFileMap(IncFsFileMap&&) noexcept = default;
+IncFsFileMap& IncFsFileMap::operator =(IncFsFileMap&&) noexcept = default;
IncFsFileMap::~IncFsFileMap() = default;
const void* IncFsFileMap::unsafe_data() const {
@@ -96,7 +98,7 @@
// safely.
for (data_block_index_t curr_index = start_index; curr_index <= end_index; ++curr_index) {
const size_t i = curr_index / kBucketBits;
- const auto present_bit = 1U << (curr_index % kBucketBits);
+ const bucket_t present_bit = 1U << (curr_index % kBucketBits);
std::atomic<bucket_t>& bucket = loaded_blocks_[i];
if (LIKELY(bucket.load(std::memory_order_relaxed) & present_bit)) {
continue;
@@ -105,7 +107,7 @@
// Touch all of the blocks with pread to ensure that the region of data is fully present.
uint8_t value;
const off64_t read_offset = (curr_index * INCFS_DATA_FILE_BLOCK_SIZE) + start_block_offset_;
- if (UNLIKELY(!android::base::ReadFullyAtOffset(fd_, &value, 1U, read_offset))) {
+ if (UNLIKELY(TEMP_FAILURE_RETRY(pread64(fd_, &value, 1U, read_offset)) <= 0)) {
success = false;
break;
}
@@ -132,4 +134,4 @@
}
#endif
-} // namespace android::incfs::util
\ No newline at end of file
+} // namespace android::incfs
\ No newline at end of file
