[incfs] Require an ioctl to get filled blocks
+ fix an FD leak in UniqueConrol move
+ fix a potential FD leak in makeControl()
Bug: 152983639
Test: atest libincfs-test
Change-Id: Ic002768a5a9c29ae76d33c46565f8778a4af8752
Merged-In: Ic002768a5a9c29ae76d33c46565f8778a4af8752
diff --git a/incfs/incfs.cpp b/incfs/incfs.cpp
index 3c20701..a8526ed 100644
--- a/incfs/incfs.cpp
+++ b/incfs/incfs.cpp
@@ -305,17 +305,23 @@
}
static IncFsControl* makeControl(const char* root) {
- int cmd = openCmd(root).release();
- if (cmd < 0) {
+ auto cmd = openCmd(root);
+ if (!cmd.ok()) {
return nullptr;
}
- int pendingReads = openPendingReads(root).release();
- if (pendingReads < 0) {
+ auto pendingReads = openPendingReads(root);
+ if (!pendingReads.ok()) {
return nullptr;
}
- int logs = openLog(root).release();
+ auto logs = openLog(root);
// logs may be absent, that's fine
- return IncFs_CreateControl(cmd, pendingReads, logs);
+ auto control = IncFs_CreateControl(cmd.get(), pendingReads.get(), logs.get());
+ if (control) {
+ (void)cmd.release();
+ (void)pendingReads.release();
+ (void)logs.release();
+ }
+ return control;
}
static std::string makeCommandPath(std::string_view root, std::string_view item) {
@@ -910,7 +916,7 @@
return 0;
}
-static IncFsFd openWrite(int cmd, const char* path) {
+static IncFsFd openForSpecialOps(int cmd, const char* path) {
ab::unique_fd fd(::open(path, O_RDONLY | O_CLOEXEC));
if (fd < 0) {
return -errno;
@@ -923,24 +929,24 @@
return fd.release();
}
-IncFsFd IncFs_OpenWriteByPath(const IncFsControl* control, const char* path) {
+IncFsFd IncFs_OpenForSpecialOpsByPath(const IncFsControl* control, const char* path) {
const auto pathRoot = registry().rootFor(path);
const auto cmd = IncFs_GetControlFd(control, CMD);
const auto root = rootForCmd(cmd);
if (root.empty() || root != pathRoot) {
return -EINVAL;
}
- return openWrite(cmd, makeCommandPath(root, path).c_str());
+ return openForSpecialOps(cmd, makeCommandPath(root, path).c_str());
}
-IncFsFd IncFs_OpenWriteById(const IncFsControl* control, IncFsFileId id) {
+IncFsFd IncFs_OpenForSpecialOpsById(const IncFsControl* control, IncFsFileId id) {
const auto cmd = IncFs_GetControlFd(control, CMD);
const auto root = rootForCmd(cmd);
if (root.empty()) {
return -EINVAL;
}
auto name = path::join(root, kIndexDir, toStringImpl(id));
- return openWrite(cmd, makeCommandPath(root, name).c_str());
+ return openForSpecialOps(cmd, makeCommandPath(root, name).c_str());
}
static int writeBlocks(int fd, const incfs_fill_block blocks[], int blocksCount) {
@@ -1089,7 +1095,7 @@
auto outPtr = outStart;
int error = 0;
- int totalBlocks;
+ int dataBlocks;
incfs_get_filled_blocks_args args = {};
for (;;) {
auto start = args.index_out ? args.index_out : startBlockIndex;
@@ -1105,7 +1111,7 @@
return -error;
}
- totalBlocks = args.total_blocks_out;
+ dataBlocks = args.data_blocks_out;
outPtr += args.range_buffer_size_out / sizeof(incfs_filled_range);
if (!res || error == ERANGE) {
break;
@@ -1121,15 +1127,7 @@
filledRanges->endIndex = args.index_out;
auto hashStartPtr = outPtr;
if (outPtr != outStart) {
- // need to know the file size to be able to split the hash blocks from data.
- struct stat st;
- if (::fstat(fd, &st)) {
- return -errno;
- }
- const auto dataBlocks =
- (st.st_size + INCFS_DATA_FILE_BLOCK_SIZE - 1) / INCFS_DATA_FILE_BLOCK_SIZE;
-
- // now figure out the ranges for data block and hash blocks in the output
+ // figure out the ranges for data block and hash blocks in the output
for (; hashStartPtr != outStart; --hashStartPtr) {
if ((hashStartPtr - 1)->begin < dataBlocks) {
break;
@@ -1210,4 +1208,4 @@
: -ENODATA;
}
return -ENODATA;
-}
\ No newline at end of file
+}
diff --git a/incfs/include/incfs.h b/incfs/include/incfs.h
index 9190673..c8a12b0 100644
--- a/incfs/include/incfs.h
+++ b/incfs/include/incfs.h
@@ -56,21 +56,21 @@
class UniqueControl {
public:
- UniqueControl() : mControl(nullptr) {}
- UniqueControl(IncFsControl* control) : mControl(control) {}
- ~UniqueControl();
+ UniqueControl(IncFsControl* control = nullptr) : mControl(control) {}
+ ~UniqueControl() { close(); }
+ UniqueControl(UniqueControl&& other) noexcept
+ : mControl(std::exchange(other.mControl, nullptr)) {}
+ UniqueControl& operator=(UniqueControl&& other) {
+ close();
+ mControl = std::exchange(other.mControl, nullptr);
+ return *this;
+ }
+
IncFsFd cmd() const;
IncFsFd pendingReads() const;
IncFsFd logs() const;
operator IncFsControl*() const { return mControl; };
- UniqueControl(UniqueControl&& other) noexcept {
- mControl = std::exchange(other.mControl, nullptr);
- }
- UniqueControl& operator=(UniqueControl&& other) {
- this->~UniqueControl();
- new (this) UniqueControl(std::move(other));
- return *this;
- }
+ void close();
private:
IncFsControl* mControl;
@@ -137,6 +137,32 @@
IncFsFilledRanges rawFilledRanges_;
};
+class UniqueFd {
+public:
+ explicit UniqueFd(int fd) : fd_(fd) {}
+ UniqueFd() : UniqueFd(-1) {}
+ ~UniqueFd() { close(); }
+ UniqueFd(UniqueFd&& other) : fd_(other.release()) {}
+ UniqueFd& operator=(UniqueFd&& other) {
+ close();
+ fd_ = other.release();
+ return *this;
+ }
+
+ void close() {
+ if (ok()) {
+ ::close(fd_);
+ fd_ = -1;
+ }
+ }
+ bool ok() const { return fd_ >= 0; }
+ int get() const { return fd_; }
+ [[nodiscard]] int release() { return std::exchange(fd_, -1); }
+
+private:
+ int fd_;
+};
+
using Control = UniqueControl;
using FileId = IncFsFileId;
@@ -195,10 +221,8 @@
WaitResult waitForPageReads(const Control& control, std::chrono::milliseconds timeout,
std::vector<ReadInfo>* pageReadsBuffer);
-// Returns a file descriptor that needs to be closed.
-int openWrite(const Control& control, FileId fileId);
-// Returns a file descriptor that needs to be closed.
-int openWrite(const Control& control, std::string_view path);
+UniqueFd openForSpecialOps(const Control& control, FileId fileId);
+UniqueFd openForSpecialOps(const Control& control, std::string_view path);
ErrorCode writeBlocks(Span<const DataBlock> blocks);
std::pair<ErrorCode, FilledRanges> getFilledRanges(int fd);
@@ -206,7 +230,6 @@
std::pair<ErrorCode, FilledRanges> getFilledRanges(int fd, FilledRanges&& resumeFrom);
enum class LoadingState { Full, MissingBlocks };
-
LoadingState isFullyLoaded(int fd);
} // namespace android::incfs
diff --git a/incfs/include/incfs_inline.h b/incfs/include/incfs_inline.h
index 07aabb5..523213f 100644
--- a/incfs/include/incfs_inline.h
+++ b/incfs/include/incfs_inline.h
@@ -95,8 +95,9 @@
return IncFs_FileIdFromString(str.data());
}
-inline UniqueControl::~UniqueControl() {
+inline void UniqueControl::close() {
IncFs_DeleteControl(mControl);
+ mControl = nullptr;
}
inline IncFsFd UniqueControl::cmd() const {
@@ -250,11 +251,11 @@
return WaitResult(err);
}
-inline int openWrite(const Control& control, FileId fileId) {
- return IncFs_OpenWriteById(control, fileId);
+inline UniqueFd openForSpecialOps(const Control& control, FileId fileId) {
+ return UniqueFd(IncFs_OpenForSpecialOpsById(control, fileId));
}
-inline int openWrite(const Control& control, std::string_view path) {
- return IncFs_OpenWriteByPath(control, details::c_str(path));
+inline UniqueFd openForSpecialOps(const Control& control, std::string_view path) {
+ return UniqueFd(IncFs_OpenForSpecialOpsByPath(control, details::c_str(path)));
}
inline ErrorCode writeBlocks(Span<const DataBlock> blocks) {
diff --git a/incfs/include/incfs_ndk.h b/incfs/include/incfs_ndk.h
index 493c5ef..585c431 100644
--- a/incfs/include/incfs_ndk.h
+++ b/incfs/include/incfs_ndk.h
@@ -190,8 +190,8 @@
IncFsErrorCode IncFs_WaitForPageReads(const IncFsControl* control, int32_t timeoutMs,
IncFsReadInfo buffer[], size_t* bufferSize);
-IncFsFd IncFs_OpenWriteByPath(const IncFsControl* control, const char* path);
-IncFsFd IncFs_OpenWriteById(const IncFsControl* control, IncFsFileId id);
+IncFsFd IncFs_OpenForSpecialOpsByPath(const IncFsControl* control, const char* path);
+IncFsFd IncFs_OpenForSpecialOpsById(const IncFsControl* control, IncFsFileId id);
IncFsErrorCode IncFs_WriteBlocks(const IncFsDataBlock blocks[], size_t blocksCount);
diff --git a/incfs/kernel-headers/linux/incrementalfs.h b/incfs/kernel-headers/linux/incrementalfs.h
index 2d535d3..b9f4c7a 100644
--- a/incfs/kernel-headers/linux/incrementalfs.h
+++ b/incfs/kernel-headers/linux/incrementalfs.h
@@ -321,6 +321,9 @@
/* Actual number of blocks in file */
__u32 total_blocks_out;
+ /* The number of data blocks in file */
+ __u32 data_blocks_out;
+
/* Number of bytes written to range buffer */
__u32 range_buffer_size_out;
diff --git a/incfs/tests/incfs_test.cpp b/incfs/tests/incfs_test.cpp
index c8dbf6f..a0cba9f 100644
--- a/incfs/tests/incfs_test.cpp
+++ b/incfs/tests/incfs_test.cpp
@@ -131,35 +131,35 @@
}
void writeTestRanges(int id, int size) {
- auto wfd = openWrite(control_, fileId(id));
- ASSERT_GE(wfd, 0);
+ auto wfd = openForSpecialOps(control_, fileId(id));
+ ASSERT_GE(wfd.get(), 0);
auto lastPage = sizeToPages(size) - 1;
std::vector<char> data(INCFS_DATA_FILE_BLOCK_SIZE);
DataBlock blocks[] = {{
- .fileFd = wfd,
+ .fileFd = wfd.get(),
.pageIndex = 1,
.compression = INCFS_COMPRESSION_KIND_NONE,
.dataSize = (uint32_t)data.size(),
.data = data.data(),
},
{
- .fileFd = wfd,
+ .fileFd = wfd.get(),
.pageIndex = 2,
.compression = INCFS_COMPRESSION_KIND_NONE,
.dataSize = (uint32_t)data.size(),
.data = data.data(),
},
{
- .fileFd = wfd,
+ .fileFd = wfd.get(),
.pageIndex = 10,
.compression = INCFS_COMPRESSION_KIND_NONE,
.dataSize = (uint32_t)data.size(),
.data = data.data(),
},
{
- .fileFd = wfd,
+ .fileFd = wfd.get(),
// last data page
.pageIndex = lastPage,
.compression = INCFS_COMPRESSION_KIND_NONE,
@@ -167,7 +167,7 @@
.data = data.data(),
},
{
- .fileFd = wfd,
+ .fileFd = wfd.get(),
// first hash page
.pageIndex = 0,
.compression = INCFS_COMPRESSION_KIND_NONE,
@@ -176,7 +176,7 @@
.data = data.data(),
},
{
- .fileFd = wfd,
+ .fileFd = wfd.get(),
.pageIndex = 2,
.compression = INCFS_COMPRESSION_KIND_NONE,
.dataSize = (uint32_t)data.size(),
@@ -343,12 +343,12 @@
ASSERT_TRUE(control_.logs() >= 0);
ASSERT_EQ(0,
makeFile(control_, mountPath(test_file_name_), 0555, id, {.size = test_file_size_}));
- auto fd = openWrite(control_, fileId(1));
- ASSERT_GE(fd, 0);
+ auto fd = openForSpecialOps(control_, fileId(1));
+ ASSERT_GE(fd.get(), 0);
std::vector<char> data(INCFS_DATA_FILE_BLOCK_SIZE);
auto block = DataBlock{
- .fileFd = fd,
+ .fileFd = fd.get(),
.pageIndex = 0,
.compression = INCFS_COMPRESSION_KIND_NONE,
.dataSize = (uint32_t)data.size(),
@@ -386,13 +386,12 @@
ASSERT_EQ(0, memcmp(&id, &pending_reads[0].id, sizeof(id)));
ASSERT_EQ(0, (int)pending_reads[0].block);
- auto fd = openWrite(control_, fileId(1));
- ASSERT_GE(fd, 0);
-
+ auto fd = openForSpecialOps(control_, fileId(1));
+ ASSERT_GE(fd.get(), 0);
std::vector<char> data(INCFS_DATA_FILE_BLOCK_SIZE);
auto block = DataBlock{
- .fileFd = fd,
+ .fileFd = fd.get(),
.pageIndex = 0,
.compression = INCFS_COMPRESSION_KIND_NONE,
.dataSize = (uint32_t)data.size(),
@@ -413,6 +412,15 @@
EXPECT_EQ(-EBADF, IncFs_GetFilledRanges(-1, {}, nullptr));
EXPECT_EQ(-EINVAL, IncFs_GetFilledRanges(0, {}, nullptr));
EXPECT_EQ(-EINVAL, IncFs_GetFilledRangesStartingFrom(0, -1, {}, nullptr));
+
+ makeFileWithHash(1);
+ const android::base::unique_fd readFd(
+ open(mountPath(test_file_name_).c_str(), O_RDONLY | O_CLOEXEC | O_BINARY));
+ ASSERT_GE(readFd.get(), 0);
+
+ char buffer[1024];
+ IncFsFilledRanges res;
+ EXPECT_EQ(-EPERM, IncFs_GetFilledRanges(readFd.get(), {buffer, std::size(buffer)}, &res));
}
TEST_F(IncFsTest, GetFilledRanges) {
@@ -422,40 +430,36 @@
char buffer[1024];
const auto bufferSpan = IncFsSpan{.data = buffer, .size = std::size(buffer)};
- const android::base::unique_fd rfd(
- open(mountPath(test_file_name_).c_str(), O_RDONLY | O_CLOEXEC | O_BINARY));
- ASSERT_GE(rfd.get(), 0);
+ auto fd = openForSpecialOps(control_, fileId(1));
+ ASSERT_GE(fd.get(), 0);
IncFsFilledRanges filledRanges;
- EXPECT_EQ(0, IncFs_GetFilledRanges(rfd.get(), IncFsSpan{}, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRanges(fd.get(), IncFsSpan{}, &filledRanges));
EXPECT_EQ(0, filledRanges.dataRangesCount);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(0, IncFs_GetFilledRanges(rfd.get(), bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRanges(fd.get(), bufferSpan, &filledRanges));
EXPECT_EQ(0, filledRanges.dataRangesCount);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(rfd.get(), 0, bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(fd.get(), 0, bufferSpan, &filledRanges));
EXPECT_EQ(0, filledRanges.dataRangesCount);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(rfd.get(), 1, bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(fd.get(), 1, bufferSpan, &filledRanges));
EXPECT_EQ(0, filledRanges.dataRangesCount);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(rfd.get(), 30, bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(fd.get(), 30, bufferSpan, &filledRanges));
EXPECT_EQ(0, filledRanges.dataRangesCount);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(-ENODATA, IncFs_IsFullyLoaded(rfd.get()));
-
- auto wfd = openWrite(control_, fileId(1));
- ASSERT_GE(wfd, 0);
+ EXPECT_EQ(-ENODATA, IncFs_IsFullyLoaded(fd.get()));
// write one block
std::vector<char> data(INCFS_DATA_FILE_BLOCK_SIZE);
auto block = DataBlock{
- .fileFd = wfd,
+ .fileFd = fd.get(),
.pageIndex = 0,
.compression = INCFS_COMPRESSION_KIND_NONE,
.dataSize = (uint32_t)data.size(),
@@ -463,61 +467,61 @@
};
ASSERT_EQ(1, writeBlocks({&block, 1}));
- EXPECT_EQ(0, IncFs_GetFilledRanges(rfd.get(), bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRanges(fd.get(), bufferSpan, &filledRanges));
ASSERT_EQ(1, filledRanges.dataRangesCount);
EXPECT_EQ(0, filledRanges.dataRanges[0].begin);
EXPECT_EQ(1, filledRanges.dataRanges[0].end);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(rfd.get(), 0, bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(fd.get(), 0, bufferSpan, &filledRanges));
ASSERT_EQ(1, filledRanges.dataRangesCount);
EXPECT_EQ(0, filledRanges.dataRanges[0].begin);
EXPECT_EQ(1, filledRanges.dataRanges[0].end);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(rfd.get(), 1, bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(fd.get(), 1, bufferSpan, &filledRanges));
EXPECT_EQ(0, filledRanges.dataRangesCount);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(rfd.get(), 30, bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(fd.get(), 30, bufferSpan, &filledRanges));
EXPECT_EQ(0, filledRanges.dataRangesCount);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(-ENODATA, IncFs_IsFullyLoaded(rfd.get()));
+ EXPECT_EQ(-ENODATA, IncFs_IsFullyLoaded(fd.get()));
// append one more block next to the first one
block.pageIndex = 1;
ASSERT_EQ(1, writeBlocks({&block, 1}));
- EXPECT_EQ(0, IncFs_GetFilledRanges(rfd.get(), bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRanges(fd.get(), bufferSpan, &filledRanges));
ASSERT_EQ(1, filledRanges.dataRangesCount);
EXPECT_EQ(0, filledRanges.dataRanges[0].begin);
EXPECT_EQ(2, filledRanges.dataRanges[0].end);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(rfd.get(), 0, bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(fd.get(), 0, bufferSpan, &filledRanges));
ASSERT_EQ(1, filledRanges.dataRangesCount);
EXPECT_EQ(0, filledRanges.dataRanges[0].begin);
EXPECT_EQ(2, filledRanges.dataRanges[0].end);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(rfd.get(), 1, bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(fd.get(), 1, bufferSpan, &filledRanges));
ASSERT_EQ(1, filledRanges.dataRangesCount);
EXPECT_EQ(1, filledRanges.dataRanges[0].begin);
EXPECT_EQ(2, filledRanges.dataRanges[0].end);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(rfd.get(), 30, bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(fd.get(), 30, bufferSpan, &filledRanges));
EXPECT_EQ(0, filledRanges.dataRangesCount);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(-ENODATA, IncFs_IsFullyLoaded(rfd.get()));
+ EXPECT_EQ(-ENODATA, IncFs_IsFullyLoaded(fd.get()));
// now create a gap between filled blocks
block.pageIndex = 3;
ASSERT_EQ(1, writeBlocks({&block, 1}));
- EXPECT_EQ(0, IncFs_GetFilledRanges(rfd.get(), bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRanges(fd.get(), bufferSpan, &filledRanges));
ASSERT_EQ(2, filledRanges.dataRangesCount);
EXPECT_EQ(0, filledRanges.dataRanges[0].begin);
EXPECT_EQ(2, filledRanges.dataRanges[0].end);
@@ -525,7 +529,7 @@
EXPECT_EQ(4, filledRanges.dataRanges[1].end);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(rfd.get(), 0, bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(fd.get(), 0, bufferSpan, &filledRanges));
ASSERT_EQ(2, filledRanges.dataRangesCount);
EXPECT_EQ(0, filledRanges.dataRanges[0].begin);
EXPECT_EQ(2, filledRanges.dataRanges[0].end);
@@ -533,7 +537,7 @@
EXPECT_EQ(4, filledRanges.dataRanges[1].end);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(rfd.get(), 1, bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(fd.get(), 1, bufferSpan, &filledRanges));
ASSERT_EQ(2, filledRanges.dataRangesCount);
EXPECT_EQ(1, filledRanges.dataRanges[0].begin);
EXPECT_EQ(2, filledRanges.dataRanges[0].end);
@@ -541,44 +545,44 @@
EXPECT_EQ(4, filledRanges.dataRanges[1].end);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(rfd.get(), 2, bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(fd.get(), 2, bufferSpan, &filledRanges));
ASSERT_EQ(1, filledRanges.dataRangesCount);
EXPECT_EQ(3, filledRanges.dataRanges[0].begin);
EXPECT_EQ(4, filledRanges.dataRanges[0].end);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(rfd.get(), 30, bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(fd.get(), 30, bufferSpan, &filledRanges));
EXPECT_EQ(0, filledRanges.dataRangesCount);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(-ENODATA, IncFs_IsFullyLoaded(rfd.get()));
+ EXPECT_EQ(-ENODATA, IncFs_IsFullyLoaded(fd.get()));
// at last fill the whole file and make sure we report it as having a single range
block.pageIndex = 2;
ASSERT_EQ(1, writeBlocks({&block, 1}));
- EXPECT_EQ(0, IncFs_GetFilledRanges(rfd.get(), bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRanges(fd.get(), bufferSpan, &filledRanges));
ASSERT_EQ(1, filledRanges.dataRangesCount);
EXPECT_EQ(0, filledRanges.dataRanges[0].begin);
EXPECT_EQ(4, filledRanges.dataRanges[0].end);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(rfd.get(), 0, bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(fd.get(), 0, bufferSpan, &filledRanges));
ASSERT_EQ(1, filledRanges.dataRangesCount);
EXPECT_EQ(0, filledRanges.dataRanges[0].begin);
EXPECT_EQ(4, filledRanges.dataRanges[0].end);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(rfd.get(), 1, bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(fd.get(), 1, bufferSpan, &filledRanges));
ASSERT_EQ(1, filledRanges.dataRangesCount);
EXPECT_EQ(1, filledRanges.dataRanges[0].begin);
EXPECT_EQ(4, filledRanges.dataRanges[0].end);
- EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(rfd.get(), 30, bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRangesStartingFrom(fd.get(), 30, bufferSpan, &filledRanges));
EXPECT_EQ(0, filledRanges.dataRangesCount);
EXPECT_EQ(0, filledRanges.hashRangesCount);
- EXPECT_EQ(0, IncFs_IsFullyLoaded(rfd.get()));
+ EXPECT_EQ(0, IncFs_IsFullyLoaded(fd.get()));
}
TEST_F(IncFsTest, GetFilledRangesSmallBuffer) {
@@ -587,30 +591,26 @@
{.size = 5 * INCFS_DATA_FILE_BLOCK_SIZE}));
char buffer[1024];
- const android::base::unique_fd rfd(
- open(mountPath(test_file_name_).c_str(), O_RDONLY | O_CLOEXEC | O_BINARY));
- ASSERT_GE(rfd.get(), 0);
-
- auto wfd = openWrite(control_, fileId(1));
- ASSERT_GE(wfd, 0);
+ auto fd = openForSpecialOps(control_, fileId(1));
+ ASSERT_GE(fd.get(), 0);
std::vector<char> data(INCFS_DATA_FILE_BLOCK_SIZE);
DataBlock blocks[] = {DataBlock{
- .fileFd = wfd,
+ .fileFd = fd.get(),
.pageIndex = 0,
.compression = INCFS_COMPRESSION_KIND_NONE,
.dataSize = (uint32_t)data.size(),
.data = data.data(),
},
DataBlock{
- .fileFd = wfd,
+ .fileFd = fd.get(),
.pageIndex = 2,
.compression = INCFS_COMPRESSION_KIND_NONE,
.dataSize = (uint32_t)data.size(),
.data = data.data(),
},
DataBlock{
- .fileFd = wfd,
+ .fileFd = fd.get(),
.pageIndex = 4,
.compression = INCFS_COMPRESSION_KIND_NONE,
.dataSize = (uint32_t)data.size(),
@@ -620,7 +620,7 @@
IncFsSpan bufferSpan = {.data = buffer, .size = sizeof(IncFsBlockRange)};
IncFsFilledRanges filledRanges;
- EXPECT_EQ(-ERANGE, IncFs_GetFilledRanges(rfd.get(), bufferSpan, &filledRanges));
+ EXPECT_EQ(-ERANGE, IncFs_GetFilledRanges(fd.get(), bufferSpan, &filledRanges));
ASSERT_EQ(1, filledRanges.dataRangesCount);
EXPECT_EQ(0, filledRanges.dataRanges[0].begin);
EXPECT_EQ(1, filledRanges.dataRanges[0].end);
@@ -628,7 +628,7 @@
EXPECT_EQ(2, filledRanges.endIndex);
EXPECT_EQ(-ERANGE,
- IncFs_GetFilledRangesStartingFrom(rfd.get(), filledRanges.endIndex, bufferSpan,
+ IncFs_GetFilledRangesStartingFrom(fd.get(), filledRanges.endIndex, bufferSpan,
&filledRanges));
ASSERT_EQ(1, filledRanges.dataRangesCount);
EXPECT_EQ(2, filledRanges.dataRanges[0].begin);
@@ -637,7 +637,7 @@
EXPECT_EQ(4, filledRanges.endIndex);
EXPECT_EQ(0,
- IncFs_GetFilledRangesStartingFrom(rfd.get(), filledRanges.endIndex, bufferSpan,
+ IncFs_GetFilledRangesStartingFrom(fd.get(), filledRanges.endIndex, bufferSpan,
&filledRanges));
ASSERT_EQ(1, filledRanges.dataRangesCount);
EXPECT_EQ(4, filledRanges.dataRanges[0].begin);
@@ -651,14 +651,13 @@
ASSERT_GT(size, 0);
ASSERT_NO_FATAL_FAILURE(writeTestRanges(1, size));
- const android::base::unique_fd rfd(
- open(mountPath(test_file_name_).c_str(), O_RDONLY | O_CLOEXEC | O_BINARY));
- ASSERT_GE(rfd.get(), 0);
+ auto fd = openForSpecialOps(control_, fileId(1));
+ ASSERT_GE(fd.get(), 0);
char buffer[1024];
IncFsSpan bufferSpan = {.data = buffer, .size = sizeof(buffer)};
IncFsFilledRanges filledRanges;
- EXPECT_EQ(0, IncFs_GetFilledRanges(rfd.get(), bufferSpan, &filledRanges));
+ EXPECT_EQ(0, IncFs_GetFilledRanges(fd.get(), bufferSpan, &filledRanges));
ASSERT_EQ(3, filledRanges.dataRangesCount);
auto lastPage = sizeToPages(size) - 1;
EXPECT_EQ(lastPage, filledRanges.dataRanges[2].begin);
@@ -676,12 +675,11 @@
ASSERT_GT(size, 0);
ASSERT_NO_FATAL_FAILURE(writeTestRanges(1, size));
- const android::base::unique_fd rfd(
- open(mountPath(test_file_name_).c_str(), O_RDONLY | O_CLOEXEC | O_BINARY));
- ASSERT_GE(rfd.get(), 0);
+ auto fd = openForSpecialOps(control_, fileId(1));
+ ASSERT_GE(fd.get(), 0);
// simply get all ranges
- auto [res, ranges] = getFilledRanges(rfd.get());
+ auto [res, ranges] = getFilledRanges(fd.get());
EXPECT_EQ(res, 0);
EXPECT_EQ(size_t(5), ranges.totalSize());
ASSERT_EQ(size_t(3), ranges.dataRanges().size());
@@ -696,7 +694,7 @@
// now check how buffer size limiting works.
FilledRanges::RangeBuffer buf(ranges.totalSize() - 1);
- auto [res2, ranges2] = getFilledRanges(rfd.get(), std::move(buf));
+ auto [res2, ranges2] = getFilledRanges(fd.get(), std::move(buf));
ASSERT_EQ(-ERANGE, res2);
EXPECT_EQ(ranges.totalSize() - 1, ranges2.totalSize());
ASSERT_EQ(size_t(3), ranges2.dataRanges().size());
@@ -705,7 +703,7 @@
EXPECT_EQ(size_t(1), ranges2.hashRanges()[0].size());
// and now check the resumption from the previous result
- auto [res3, ranges3] = getFilledRanges(rfd.get(), std::move(ranges2));
+ auto [res3, ranges3] = getFilledRanges(fd.get(), std::move(ranges2));
ASSERT_EQ(0, res3);
EXPECT_EQ(ranges.totalSize(), ranges3.totalSize());
ASSERT_EQ(size_t(3), ranges3.dataRanges().size());
@@ -715,14 +713,11 @@
EXPECT_EQ(2, ranges3.hashRanges()[1].begin);
EXPECT_EQ(size_t(1), ranges3.hashRanges()[1].size());
- EXPECT_EQ(LoadingState::MissingBlocks, isFullyLoaded(rfd.get()));
+ EXPECT_EQ(LoadingState::MissingBlocks, isFullyLoaded(fd.get()));
{
- auto wfd = openWrite(control_, fileId(1));
- ASSERT_GE(wfd, 0);
-
std::vector<char> data(INCFS_DATA_FILE_BLOCK_SIZE);
- DataBlock block = {.fileFd = wfd,
+ DataBlock block = {.fileFd = fd.get(),
.pageIndex = 1,
.compression = INCFS_COMPRESSION_KIND_NONE,
.dataSize = (uint32_t)data.size(),
@@ -737,5 +732,5 @@
ASSERT_EQ(1, writeBlocks({&block, 1}));
}
}
- EXPECT_EQ(LoadingState::Full, isFullyLoaded(rfd.get()));
+ EXPECT_EQ(LoadingState::Full, isFullyLoaded(fd.get()));
}
diff --git a/libdataloader/DataLoaderConnector.cpp b/libdataloader/DataLoaderConnector.cpp
index 33f374a..7e35ebf 100644
--- a/libdataloader/DataLoaderConnector.cpp
+++ b/libdataloader/DataLoaderConnector.cpp
@@ -376,7 +376,9 @@
offsetBytes, lengthBytes, incomingFd);
}
- int openWrite(FileId fid) const { return android::incfs::openWrite(mControl, fid); }
+ android::incfs::UniqueFd openForSpecialOps(FileId fid) const {
+ return android::incfs::openForSpecialOps(mControl, fid);
+ }
int writeBlocks(Span<const IncFsDataBlock> blocks) const {
return android::incfs::writeBlocks(blocks);
@@ -555,10 +557,10 @@
return connector->writeData(name, offsetBytes, lengthBytes, incomingFd);
}
-int DataLoader_FilesystemConnector_openWrite(DataLoaderFilesystemConnectorPtr ifs,
- IncFsFileId fid) {
+int DataLoader_FilesystemConnector_openForSpecialOps(DataLoaderFilesystemConnectorPtr ifs,
+ IncFsFileId fid) {
auto connector = static_cast<DataLoaderConnector*>(ifs);
- return connector->openWrite(fid);
+ return connector->openForSpecialOps(fid).release();
}
int DataLoader_FilesystemConnector_writeBlocks(DataLoaderFilesystemConnectorPtr ifs,
diff --git a/libdataloader/include/dataloader.h b/libdataloader/include/dataloader.h
index 1bb44fd..d7ddf34 100644
--- a/libdataloader/include/dataloader.h
+++ b/libdataloader/include/dataloader.h
@@ -107,8 +107,7 @@
};
struct FilesystemConnector : public DataLoaderFilesystemConnector {
- // Returns a file descriptor that needs to be closed.
- int openWrite(FileId fid);
+ android::incfs::UniqueFd openForSpecialOps(FileId fid);
int writeBlocks(DataBlocks blocks);
RawMetadata getRawMetadata(FileId fid);
};
diff --git a/libdataloader/include/dataloader_inline.h b/libdataloader/include/dataloader_inline.h
index 5f6f49a..6616611 100644
--- a/libdataloader/include/dataloader_inline.h
+++ b/libdataloader/include/dataloader_inline.h
@@ -114,8 +114,8 @@
RawMetadata&& metadata)
: mLocation(location), mName(std::move(name)), mSize(size), mMetadata(std::move(metadata)) {}
-inline int FilesystemConnector::openWrite(FileId fid) {
- return DataLoader_FilesystemConnector_openWrite(this, fid);
+inline android::incfs::UniqueFd FilesystemConnector::openForSpecialOps(FileId fid) {
+ return android::incfs::UniqueFd(DataLoader_FilesystemConnector_openForSpecialOps(this, fid));
}
inline int FilesystemConnector::writeBlocks(DataBlocks blocks) {
diff --git a/libdataloader/include/dataloader_ndk.h b/libdataloader/include/dataloader_ndk.h
index bb9cbde..341247b 100644
--- a/libdataloader/include/dataloader_ndk.h
+++ b/libdataloader/include/dataloader_ndk.h
@@ -102,7 +102,9 @@
jlong offsetBytes, jlong lengthBytes,
jobject incomingFd);
-int DataLoader_FilesystemConnector_openWrite(DataLoaderFilesystemConnectorPtr, IncFsFileId fid);
+// Returns a newly opened file descriptor and gives the ownership to the caller.
+int DataLoader_FilesystemConnector_openForSpecialOps(DataLoaderFilesystemConnectorPtr,
+ IncFsFileId fid);
int DataLoader_FilesystemConnector_writeBlocks(DataLoaderFilesystemConnectorPtr,
const IncFsDataBlock blocks[], int blocksCount);
