Merge "Const correctness handle / RedactionInfo."
diff --git a/jni/FuseDaemon.cpp b/jni/FuseDaemon.cpp
index 812e6b6..8c96088 100644
--- a/jni/FuseDaemon.cpp
+++ b/jni/FuseDaemon.cpp
@@ -856,11 +856,7 @@
return;
}
- handle* h = new handle(path);
- h->fd = fd;
- h->ri = std::move(ri);
-
- if (h->ri->isRedactionNeeded() || is_file_locked(h->fd, path)) {
+ if (ri->isRedactionNeeded() || is_file_locked(fd, path)) {
// We don't want to use the FUSE VFS cache in two cases:
// 1. When redaction is needed because app A with EXIF access might access
// a region that should have been redacted for app B without EXIF access, but app B on
@@ -874,9 +870,9 @@
// b. Reading from a FUSE fd with caching enabled may not see the latest writes using the
// lower fs fd because those writes did not go through the FUSE layer and reads from FUSE
// after that write may be served from cache
- if (h->ri->isRedactionNeeded()) {
+ if (ri->isRedactionNeeded()) {
TRACE_FUSE(fuse) << "Using direct io for " << path << " because redaction is needed.";
- } else if (is_file_locked(h->fd, path)) {
+ } else {
TRACE_FUSE(fuse) << "Using direct io for " << path << " because the file is locked.";
}
fi->direct_io = true;
@@ -884,7 +880,7 @@
TRACE_FUSE(fuse) << "Using cache for " << path;
}
- h->cached = !fi->direct_io;
+ handle* h = new handle(path, fd, ri.release(), !fi->direct_io);
node->AddHandle(h);
fi->fh = ptr_to_id(h);
@@ -1358,13 +1354,11 @@
return;
}
- handle* h = new handle(child_path);
- h->fd = fd;
- // TODO(b/147274248): Assuming there will be no EXIF to redact.
+ // TODO(b/147274248): Assume there will be no EXIF to redact.
// This prevents crashing during reads but can be a security hole if a malicious app opens an fd
// to the file before all the EXIF content is written. We could special case reads before the
// first close after a file has just been created.
- h->ri = std::make_unique<RedactionInfo>();
+ handle* h = new handle(child_path, fd, new RedactionInfo(), true /* cached */);
fi->fh = ptr_to_id(h);
fi->keep_cache = 1;
diff --git a/jni/RedactionInfo.cpp b/jni/RedactionInfo.cpp
index 3494db7..d943c8a 100644
--- a/jni/RedactionInfo.cpp
+++ b/jni/RedactionInfo.cpp
@@ -56,7 +56,7 @@
*
* This function assumes redaction_ranges_ within RedactionInfo is sorted.
*/
-bool RedactionInfo::hasOverlapWithReadRequest(size_t size, off64_t off) {
+bool RedactionInfo::hasOverlapWithReadRequest(size_t size, off64_t off) const {
if (!isRedactionNeeded() || off > redaction_ranges_.back().second ||
off + size < redaction_ranges_.front().first) {
return false;
@@ -79,11 +79,11 @@
mergeOverlappingRedactionRanges(redaction_ranges_);
}
-int RedactionInfo::size() {
+int RedactionInfo::size() const {
return redaction_ranges_.size();
}
-bool RedactionInfo::isRedactionNeeded() {
+bool RedactionInfo::isRedactionNeeded() const {
return size() > 0;
}
@@ -93,13 +93,13 @@
}
unique_ptr<vector<RedactionRange>> RedactionInfo::getOverlappingRedactionRanges(size_t size,
- off64_t off) {
+ off64_t off) const {
LOG(DEBUG) << "Computing redaction ranges for request: sz = " << size << " off = " << off;
if (hasOverlapWithReadRequest(size, off)) {
auto first_redaction = redaction_ranges_.end();
auto last_redaction = redaction_ranges_.end();
for (auto iter = redaction_ranges_.begin(); iter != redaction_ranges_.end(); ++iter) {
- RedactionRange& rr = *iter;
+ const RedactionRange& rr = *iter;
// Look for the first range that overlaps with the read request
if (first_redaction == redaction_ranges_.end() && off <= rr.second &&
off + size >= rr.first) {
@@ -121,4 +121,4 @@
return std::make_unique<vector<RedactionRange>>();
}
} // namespace fuse
-} // namespace mediaprovider
\ No newline at end of file
+} // namespace mediaprovider
diff --git a/jni/include/libfuse_jni/RedactionInfo.h b/jni/include/libfuse_jni/RedactionInfo.h
index 134ee90..5218e28 100644
--- a/jni/include/libfuse_jni/RedactionInfo.h
+++ b/jni/include/libfuse_jni/RedactionInfo.h
@@ -69,20 +69,20 @@
* relevant redaction ranges, the vector will be empty.
*/
std::unique_ptr<std::vector<RedactionRange>> getOverlappingRedactionRanges(size_t size,
- off64_t off);
+ off64_t off) const;
/**
* Returns whether any ranges need to be redacted.
*/
- bool isRedactionNeeded();
+ bool isRedactionNeeded() const;
/**
* Returns number of redaction ranges.
*/
- int size();
+ int size() const;
private:
std::vector<RedactionRange> redaction_ranges_;
void processRedactionRanges(int redaction_ranges_num, const off64_t* redaction_ranges);
- bool hasOverlapWithReadRequest(size_t size, off64_t off);
+ bool hasOverlapWithReadRequest(size_t size, off64_t off) const;
};
} // namespace fuse
diff --git a/jni/node-inl.h b/jni/node-inl.h
index 5db1413..487583c 100644
--- a/jni/node-inl.h
+++ b/jni/node-inl.h
@@ -34,17 +34,21 @@
namespace fuse {
struct handle {
- explicit handle(const std::string& path) : path(path), fd(-1), ri(nullptr), cached(true){};
+ explicit handle(const std::string& path, int fd, const RedactionInfo* ri, bool cached)
+ : path(path), fd(fd), ri(ri), cached(cached) {
+ CHECK(ri != nullptr);
+ }
+
const std::string path;
- int fd;
- std::unique_ptr<RedactionInfo> ri;
- bool cached;
+ const int fd;
+ const std::unique_ptr<const RedactionInfo> ri;
+ const bool cached;
~handle() { close(fd); }
};
struct dirhandle {
- explicit dirhandle(DIR* dir) : d(dir), next_off(0){};
+ explicit dirhandle(DIR* dir) : d(dir), next_off(0) { CHECK(dir != nullptr); }
DIR* const d;
off_t next_off;
diff --git a/jni/node_test.cpp b/jni/node_test.cpp
index 1c7103c..c72c8ee 100644
--- a/jni/node_test.cpp
+++ b/jni/node_test.cpp
@@ -200,8 +200,7 @@
TEST_F(NodeTest, AddDestroyHandle) {
unique_node_ptr node = CreateNode(nullptr, "/path");
- handle* h = new handle("/path");
- h->cached = true;
+ handle* h = new handle("/path", -1, new mediaprovider::fuse::RedactionInfo, true /* cached */);
node->AddHandle(h);
ASSERT_TRUE(node->HasCachedHandle());
@@ -212,6 +211,7 @@
// the node in question.
EXPECT_DEATH(node->DestroyHandle(h), "");
EXPECT_DEATH(node->DestroyHandle(nullptr), "");
- std::unique_ptr<handle> h2(new handle("/path2"));
+ std::unique_ptr<handle> h2(
+ new handle("/path2", -1, new mediaprovider::fuse::RedactionInfo, true /* cached */));
EXPECT_DEATH(node->DestroyHandle(h2.get()), "");
}