From 5e6832a24d39f15261be0cad19708274db57a126 Mon Sep 17 00:00:00 2001 From: xunchang Date: Fri, 15 Mar 2019 16:04:32 -0700 Subject: [PATCH] Remove the provider_vtab It's no longer needed with the newly added FuseDataProvider class. Also cleans up the parameters for run_fuse_sideload. Bug: 127071893 Test: unit tests pass, run a sideload Change-Id: I1ccd6798d187cfc6ac9f559ffb3f3edf08dad55c --- fuse_sdcard_install.cpp | 13 ++++------- fuse_sideload/fuse_sideload.cpp | 16 ++++++++------ fuse_sideload/include/fuse_provider.h | 2 +- fuse_sideload/include/fuse_sideload.h | 14 ++++-------- minadbd/fuse_adb_provider.cpp | 15 ------------- minadbd/fuse_adb_provider.h | 2 -- minadbd/minadbd_services.cpp | 6 +++++- tests/component/sideload_test.cpp | 31 +++++++++++++++------------ 8 files changed, 40 insertions(+), 59 deletions(-) diff --git a/fuse_sdcard_install.cpp b/fuse_sdcard_install.cpp index 79ef16bb..2feb4d24 100644 --- a/fuse_sdcard_install.cpp +++ b/fuse_sdcard_install.cpp @@ -20,26 +20,21 @@ #include #include +#include #include "fuse_provider.h" #include "fuse_sideload.h" bool start_sdcard_fuse(const char* path) { - FuseFileDataProvider file_data_reader(path, 65536); + auto file_data_reader = std::make_unique(path, 65536); - if (!file_data_reader) { + if (!file_data_reader->Valid()) { return false; } - provider_vtab vtab; - vtab.read_block = std::bind(&FuseFileDataProvider::ReadBlockAlignedData, &file_data_reader, - std::placeholders::_2, std::placeholders::_3, std::placeholders::_1); - vtab.close = [&file_data_reader]() { file_data_reader.Close(); }; - // The installation process expects to find the sdcard unmounted. Unmount it with MNT_DETACH so // that our open file continues to work but new references see it as unmounted. umount2("/sdcard", MNT_DETACH); - return run_fuse_sideload(vtab, file_data_reader.file_size(), - file_data_reader.fuse_block_size()) == 0; + return run_fuse_sideload(std::move(file_data_reader)) == 0; } diff --git a/fuse_sideload/fuse_sideload.cpp b/fuse_sideload/fuse_sideload.cpp index e812486f..b5b6ac15 100644 --- a/fuse_sideload/fuse_sideload.cpp +++ b/fuse_sideload/fuse_sideload.cpp @@ -76,7 +76,7 @@ using SHA256Digest = std::array; struct fuse_data { android::base::unique_fd ffd; // file descriptor for the fuse socket - provider_vtab vtab; + FuseDataProvider* provider; // Provider of the source data. uint64_t file_size; // bytes @@ -236,7 +236,7 @@ static int fetch_block(fuse_data* fd, uint32_t block) { return 0; } - size_t fetch_size = fd->block_size; + uint32_t fetch_size = fd->block_size; if (block * fd->block_size + fetch_size > fd->file_size) { // If we're reading the last (partial) block of the file, expect a shorter response from the // host, and pad the rest of the block with zeroes. @@ -244,7 +244,7 @@ static int fetch_block(fuse_data* fd, uint32_t block) { memset(fd->block_data + fetch_size, 0, fd->block_size - fetch_size); } - if (!fd->vtab.read_block(block, fd->block_data, fetch_size)) { + if (!fd->provider->ReadBlockAlignedData(fd->block_data, fetch_size, block)) { return -EIO; } @@ -341,12 +341,14 @@ static int handle_read(void* data, fuse_data* fd, const fuse_in_header* hdr) { return NO_STATUS; } -int run_fuse_sideload(const provider_vtab& vtab, uint64_t file_size, uint32_t block_size, - const char* mount_point) { +int run_fuse_sideload(std::unique_ptr&& provider, const char* mount_point) { // If something's already mounted on our mountpoint, try to remove it. (Mostly in case of a // previous abnormal exit.) umount2(mount_point, MNT_FORCE); + uint64_t file_size = provider->file_size(); + uint32_t block_size = provider->fuse_block_size(); + // fs/fuse/inode.c in kernel code uses the greater of 4096 and the passed-in max_read. if (block_size < 4096) { fprintf(stderr, "block size (%u) is too small\n", block_size); @@ -358,7 +360,7 @@ int run_fuse_sideload(const provider_vtab& vtab, uint64_t file_size, uint32_t bl } fuse_data fd = {}; - fd.vtab = vtab; + fd.provider = provider.get(); fd.file_size = file_size; fd.block_size = block_size; fd.file_blocks = (file_size == 0) ? 0 : (((file_size - 1) / block_size) + 1); @@ -480,7 +482,7 @@ int run_fuse_sideload(const provider_vtab& vtab, uint64_t file_size, uint32_t bl } done: - fd.vtab.close(); + provider->Close(); if (umount2(mount_point, MNT_DETACH) == -1) { fprintf(stderr, "fuse_sideload umount failed: %s\n", strerror(errno)); diff --git a/fuse_sideload/include/fuse_provider.h b/fuse_sideload/include/fuse_provider.h index 672af057..499d57aa 100644 --- a/fuse_sideload/include/fuse_provider.h +++ b/fuse_sideload/include/fuse_provider.h @@ -37,7 +37,7 @@ class FuseDataProvider { return fuse_block_size_; } - explicit operator bool() const { + bool Valid() const { return fd_ != -1; } diff --git a/fuse_sideload/include/fuse_sideload.h b/fuse_sideload/include/fuse_sideload.h index 821c7c80..1b7759a7 100644 --- a/fuse_sideload/include/fuse_sideload.h +++ b/fuse_sideload/include/fuse_sideload.h @@ -17,7 +17,9 @@ #ifndef __FUSE_SIDELOAD_H #define __FUSE_SIDELOAD_H -#include +#include + +#include "fuse_provider.h" // Define the filenames created by the sideload FUSE filesystem. static constexpr const char* FUSE_SIDELOAD_HOST_MOUNTPOINT = "/sideload"; @@ -26,15 +28,7 @@ static constexpr const char* FUSE_SIDELOAD_HOST_PATHNAME = "/sideload/package.zi static constexpr const char* FUSE_SIDELOAD_HOST_EXIT_FLAG = "exit"; static constexpr const char* FUSE_SIDELOAD_HOST_EXIT_PATHNAME = "/sideload/exit"; -struct provider_vtab { - // read a block - std::function read_block; - - // close down - std::function close; -}; - -int run_fuse_sideload(const provider_vtab& vtab, uint64_t file_size, uint32_t block_size, +int run_fuse_sideload(std::unique_ptr&& provider, const char* mount_point = FUSE_SIDELOAD_HOST_MOUNTPOINT); #endif diff --git a/minadbd/fuse_adb_provider.cpp b/minadbd/fuse_adb_provider.cpp index cada4bd2..9d19a1ec 100644 --- a/minadbd/fuse_adb_provider.cpp +++ b/minadbd/fuse_adb_provider.cpp @@ -18,14 +18,10 @@ #include #include -#include #include -#include - #include "adb.h" #include "adb_io.h" -#include "fuse_sideload.h" bool FuseAdbDataProvider::ReadBlockAlignedData(uint8_t* buffer, uint32_t fetch_size, uint32_t start_block) const { @@ -45,14 +41,3 @@ bool FuseAdbDataProvider::ReadBlockAlignedData(uint8_t* buffer, uint32_t fetch_s void FuseAdbDataProvider::Close() { WriteFdExactly(fd_, "DONEDONE"); } - -int run_adb_fuse(android::base::unique_fd&& sfd, uint64_t file_size, uint32_t block_size) { - FuseAdbDataProvider adb_data_reader(std::move(sfd), file_size, block_size); - - provider_vtab vtab; - vtab.read_block = std::bind(&FuseAdbDataProvider::ReadBlockAlignedData, &adb_data_reader, - std::placeholders::_2, std::placeholders::_3, std::placeholders::_1); - vtab.close = [&adb_data_reader]() { adb_data_reader.Close(); }; - - return run_fuse_sideload(vtab, file_size, block_size); -} diff --git a/minadbd/fuse_adb_provider.h b/minadbd/fuse_adb_provider.h index e93aa046..3fb689bd 100644 --- a/minadbd/fuse_adb_provider.h +++ b/minadbd/fuse_adb_provider.h @@ -35,6 +35,4 @@ class FuseAdbDataProvider : public FuseDataProvider { void Close() override; }; -int run_adb_fuse(android::base::unique_fd&& sfd, uint64_t file_size, uint32_t block_size); - #endif diff --git a/minadbd/minadbd_services.cpp b/minadbd/minadbd_services.cpp index 3e112854..6fe5c79b 100644 --- a/minadbd/minadbd_services.cpp +++ b/minadbd/minadbd_services.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -30,6 +31,7 @@ #include "adb_unique_fd.h" #include "fdevent.h" #include "fuse_adb_provider.h" +#include "fuse_sideload.h" #include "services.h" #include "sysdeps.h" @@ -44,7 +46,9 @@ static void sideload_host_service(unique_fd sfd, const std::string& args) { printf("sideload-host file size %" PRId64 " block size %d\n", file_size, block_size); - int result = run_adb_fuse(std::move(sfd), file_size, block_size); + auto adb_data_reader = + std::make_unique(std::move(sfd), file_size, block_size); + int result = run_fuse_sideload(std::move(adb_data_reader)); printf("sideload_host finished\n"); exit(result == 0 ? 0 : 1); diff --git a/tests/component/sideload_test.cpp b/tests/component/sideload_test.cpp index 8d290dbc..f5981acb 100644 --- a/tests/component/sideload_test.cpp +++ b/tests/component/sideload_test.cpp @@ -16,13 +16,16 @@ #include +#include #include #include #include #include +#include #include +#include "fuse_provider.h" #include "fuse_sideload.h" TEST(SideloadTest, fuse_device) { @@ -30,14 +33,17 @@ TEST(SideloadTest, fuse_device) { } TEST(SideloadTest, run_fuse_sideload_wrong_parameters) { - provider_vtab vtab; - vtab.close = [](void) {}; + auto provider_small_block = + std::make_unique(android::base::unique_fd(), 4096, 4095); + ASSERT_EQ(-1, run_fuse_sideload(std::move(provider_small_block))); - ASSERT_EQ(-1, run_fuse_sideload(vtab, 4096, 4095)); - ASSERT_EQ(-1, run_fuse_sideload(vtab, 4096, (1 << 22) + 1)); + auto provider_large_block = + std::make_unique(android::base::unique_fd(), 4096, (1 << 22) + 1); + ASSERT_EQ(-1, run_fuse_sideload(std::move(provider_large_block))); - // Too many blocks. - ASSERT_EQ(-1, run_fuse_sideload(vtab, ((1 << 18) + 1) * 4096, 4096)); + auto provider_too_many_blocks = std::make_unique( + android::base::unique_fd(), ((1 << 18) + 1) * 4096, 4096); + ASSERT_EQ(-1, run_fuse_sideload(std::move(provider_too_many_blocks))); } TEST(SideloadTest, run_fuse_sideload) { @@ -50,18 +56,15 @@ TEST(SideloadTest, run_fuse_sideload) { const std::string content = android::base::Join(blocks, ""); ASSERT_EQ(16384U, content.size()); - provider_vtab vtab; - vtab.close = [](void) {}; - vtab.read_block = [&blocks](uint32_t block, uint8_t* buffer, uint32_t fetch_size) { - if (block >= 4) return false; - blocks[block].copy(reinterpret_cast(buffer), fetch_size); - return true; - }; + TemporaryFile temp_file; + ASSERT_TRUE(android::base::WriteStringToFile(content, temp_file.path)); + auto provider = std::make_unique(temp_file.path, 4096); + ASSERT_TRUE(provider->Valid()); TemporaryDir mount_point; pid_t pid = fork(); if (pid == 0) { - ASSERT_EQ(0, run_fuse_sideload(vtab, 16384, 4096, mount_point.path)); + ASSERT_EQ(0, run_fuse_sideload(std::move(provider), mount_point.path)); _exit(EXIT_SUCCESS); }