16 files changed, 250 insertions, 136 deletions

diff --git a/Android.bp b/Android.bp
index 4c7ce52e9..a6986bb2c 100644
--- a/Android.bp
+++ b/Android.bp
@@ -174,7 +174,7 @@ cc_library_static {
     srcs: [
         "adb_install.cpp",
         "fsck_unshare_blocks.cpp",
-        "fuse_sdcard_provider.cpp",
+        "fuse_sdcard_install.cpp",
         "install.cpp",
         "recovery.cpp",
         "roots.cpp",
diff --git a/fuse_sdcard_install.cpp b/fuse_sdcard_install.cpp
new file mode 100644
index 000000000..79ef16bbc
--- /dev/null
+++ b/fuse_sdcard_install.cpp
@@ -0,0 +1,45 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "fuse_sdcard_install.h"
+
+#include <sys/mount.h>
+#include <unistd.h>
+
+#include <functional>
+
+#include "fuse_provider.h"
+#include "fuse_sideload.h"
+
+bool start_sdcard_fuse(const char* path) {
+  FuseFileDataProvider file_data_reader(path, 65536);
+
+  if (!file_data_reader) {
+    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;
+}
diff --git a/fuse_sdcard_provider.h b/fuse_sdcard_install.h
index bdc60f2ba..58f84a614 100644
--- a/fuse_sdcard_provider.h
+++ b/fuse_sdcard_install.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014 The Android Open Source Project
+ * Copyright (C) 2019 The Android Open Source Project
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -14,9 +14,6 @@
  * limitations under the License.
  */
 
-#ifndef __FUSE_SDCARD_PROVIDER_H
-#define __FUSE_SDCARD_PROVIDER_H
+#pragma once
 
 bool start_sdcard_fuse(const char* path);
-
-#endif
diff --git a/fuse_sdcard_provider.cpp b/fuse_sdcard_provider.cpp
deleted file mode 100644
index 46bdf1774..000000000
--- a/fuse_sdcard_provider.cpp
+++ /dev/null
@@ -1,83 +0,0 @@
-/*
- * Copyright (C) 2014 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include "fuse_sdcard_provider.h"
-
-#include <errno.h>
-#include <fcntl.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <sys/mount.h>
-#include <sys/stat.h>
-#include <unistd.h>
-
-#include <functional>
-
-#include <android-base/file.h>
-
-#include "fuse_sideload.h"
-
-struct file_data {
-  int fd;  // the underlying sdcard file
-
-  uint64_t file_size;
-  uint32_t block_size;
-};
-
-static int read_block_file(const file_data& fd, uint32_t block, uint8_t* buffer,
-                           uint32_t fetch_size) {
-  off64_t offset = static_cast<off64_t>(block) * fd.block_size;
-  if (TEMP_FAILURE_RETRY(lseek64(fd.fd, offset, SEEK_SET)) == -1) {
-    fprintf(stderr, "seek on sdcard failed: %s\n", strerror(errno));
-    return -EIO;
-  }
-
-  if (!android::base::ReadFully(fd.fd, buffer, fetch_size)) {
-    fprintf(stderr, "read on sdcard failed: %s\n", strerror(errno));
-    return -EIO;
-  }
-
-  return 0;
-}
-
-bool start_sdcard_fuse(const char* path) {
-  struct stat sb;
-  if (stat(path, &sb) == -1) {
-    fprintf(stderr, "failed to stat %s: %s\n", path, strerror(errno));
-    return false;
-  }
-
-  file_data fd;
-  fd.fd = open(path, O_RDONLY);
-  if (fd.fd == -1) {
-    fprintf(stderr, "failed to open %s: %s\n", path, strerror(errno));
-    return false;
-  }
-  fd.file_size = sb.st_size;
-  fd.block_size = 65536;
-
-  provider_vtab vtab;
-  vtab.read_block = std::bind(&read_block_file, fd, std::placeholders::_1, std::placeholders::_2,
-                              std::placeholders::_3);
-  vtab.close = [&fd]() { close(fd.fd); };
-
-  // 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, fd.file_size, fd.block_size) == 0;
-}
diff --git a/fuse_sideload/Android.bp b/fuse_sideload/Android.bp
index 90c4c22c3..8548548d2 100644
--- a/fuse_sideload/Android.bp
+++ b/fuse_sideload/Android.bp
@@ -16,14 +16,17 @@ cc_library {
     name: "libfusesideload",
     recovery_available: true,
 
+    defaults: [
+        "recovery_defaults",
+    ],
+
     cflags: [
         "-D_XOPEN_SOURCE",
         "-D_GNU_SOURCE",
-        "-Wall",
-        "-Werror",
     ],
 
     srcs: [
+        "fuse_provider.cpp",
         "fuse_sideload.cpp",
     ],
 
diff --git a/fuse_sideload/fuse_provider.cpp b/fuse_sideload/fuse_provider.cpp
new file mode 100644
index 000000000..58786f5f3
--- /dev/null
+++ b/fuse_sideload/fuse_provider.cpp
@@ -0,0 +1,71 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "fuse_provider.h"
+
+#include <errno.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include <functional>
+
+#include <android-base/file.h>
+
+#include "fuse_sideload.h"
+
+FuseFileDataProvider::FuseFileDataProvider(const std::string& path, uint32_t block_size) {
+  struct stat sb;
+  if (stat(path.c_str(), &sb) == -1) {
+    fprintf(stderr, "failed to stat %s: %s\n", path.c_str(), strerror(errno));
+    return;
+  }
+
+  fd_.reset(open(path.c_str(), O_RDONLY));
+  if (fd_ == -1) {
+    fprintf(stderr, "failed to open %s: %s\n", path.c_str(), strerror(errno));
+    return;
+  }
+  file_size_ = sb.st_size;
+  fuse_block_size_ = block_size;
+}
+
+bool FuseFileDataProvider::ReadBlockAlignedData(uint8_t* buffer, uint32_t fetch_size,
+                                                uint32_t start_block) const {
+  uint64_t offset = static_cast<uint64_t>(start_block) * fuse_block_size_;
+  if (fetch_size > file_size_ || offset > file_size_ - fetch_size) {
+    fprintf(stderr,
+            "Out of bound read, start block: %" PRIu32 ", fetch size: %" PRIu32
+            ", file size %" PRIu64 "\n",
+            start_block, fetch_size, file_size_);
+    return false;
+  }
+
+  if (!android::base::ReadFullyAtOffset(fd_, buffer, fetch_size, offset)) {
+    fprintf(stderr, "Failed to read fetch size: %" PRIu32 " bytes data at offset %" PRIu64 ": %s\n",
+            fetch_size, offset, strerror(errno));
+    return false;
+  }
+
+  return true;
+}
+
+void FuseFileDataProvider::Close() {
+  fd_.reset();
+}
diff --git a/fuse_sideload/fuse_sideload.cpp b/fuse_sideload/fuse_sideload.cpp
index 1c7e98f01..e812486f8 100644
--- a/fuse_sideload/fuse_sideload.cpp
+++ b/fuse_sideload/fuse_sideload.cpp
@@ -244,8 +244,9 @@ static int fetch_block(fuse_data* fd, uint32_t block) {
     memset(fd->block_data + fetch_size, 0, fd->block_size - fetch_size);
   }
 
-  int result = fd->vtab.read_block(block, fd->block_data, fetch_size);
-  if (result < 0) return result;
+  if (!fd->vtab.read_block(block, fd->block_data, fetch_size)) {
+    return -EIO;
+  }
 
   fd->curr_block = block;
 
diff --git a/fuse_sideload/include/fuse_provider.h b/fuse_sideload/include/fuse_provider.h
new file mode 100644
index 000000000..672af0577
--- /dev/null
+++ b/fuse_sideload/include/fuse_provider.h
@@ -0,0 +1,73 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <stdint.h>
+
+#include <string>
+
+#include <android-base/unique_fd.h>
+
+// This is the base class to read data from source and provide the data to FUSE.
+class FuseDataProvider {
+ public:
+  FuseDataProvider(android::base::unique_fd&& fd, uint64_t file_size, uint32_t block_size)
+      : fd_(std::move(fd)), file_size_(file_size), fuse_block_size_(block_size) {}
+
+  virtual ~FuseDataProvider() = default;
+
+  uint64_t file_size() const {
+    return file_size_;
+  }
+  uint32_t fuse_block_size() const {
+    return fuse_block_size_;
+  }
+
+  explicit operator bool() const {
+    return fd_ != -1;
+  }
+
+  // Reads |fetch_size| bytes data starting from |start_block|. Puts the result in |buffer|.
+  virtual bool ReadBlockAlignedData(uint8_t* buffer, uint32_t fetch_size,
+                                    uint32_t start_block) const = 0;
+
+  virtual void Close() = 0;
+
+ protected:
+  FuseDataProvider() = default;
+
+  // The underlying source to read data from.
+  android::base::unique_fd fd_;
+  // Size in bytes of the file to read.
+  uint64_t file_size_ = 0;
+  // Block size passed to the fuse, this is different from the block size of the block device.
+  uint32_t fuse_block_size_ = 0;
+};
+
+// This class reads data from a file.
+class FuseFileDataProvider : public FuseDataProvider {
+ public:
+  FuseFileDataProvider(android::base::unique_fd&& fd, uint64_t file_size, uint32_t block_size)
+      : FuseDataProvider(std::move(fd), file_size, block_size) {}
+
+  FuseFileDataProvider(const std::string& path, uint32_t block_size);
+
+  bool ReadBlockAlignedData(uint8_t* buffer, uint32_t fetch_size,
+                            uint32_t start_block) const override;
+
+  void Close() override;
+};
diff --git a/fuse_sideload/include/fuse_sideload.h b/fuse_sideload/include/fuse_sideload.h
index 1b34cbdb0..821c7c808 100644
--- a/fuse_sideload/include/fuse_sideload.h
+++ b/fuse_sideload/include/fuse_sideload.h
@@ -28,7 +28,7 @@ static constexpr const char* FUSE_SIDELOAD_HOST_EXIT_PATHNAME = "/sideload/exit"
 
 struct provider_vtab {
   // read a block
-  std::function<int(uint32_t block, uint8_t* buffer, uint32_t fetch_size)> read_block;
+  std::function<bool(uint32_t block, uint8_t* buffer, uint32_t fetch_size)> read_block;
 
   // close down
   std::function<void(void)> close;
diff --git a/minadbd/Android.bp b/minadbd/Android.bp
index a95d979a5..9b889f4a2 100644
--- a/minadbd/Android.bp
+++ b/minadbd/Android.bp
@@ -66,6 +66,7 @@ cc_test {
 
     static_libs: [
         "libminadbd_services",
+        "libfusesideload",
         "libadbd",
     ],
 
diff --git a/minadbd/fuse_adb_provider.cpp b/minadbd/fuse_adb_provider.cpp
index 9bd3f2392..cada4bd2a 100644
--- a/minadbd/fuse_adb_provider.cpp
+++ b/minadbd/fuse_adb_provider.cpp
@@ -27,30 +27,32 @@
 #include "adb_io.h"
 #include "fuse_sideload.h"
 
-int read_block_adb(const adb_data& ad, uint32_t block, uint8_t* buffer, uint32_t fetch_size) {
-  if (!WriteFdFmt(ad.sfd, "%08u", block)) {
+bool FuseAdbDataProvider::ReadBlockAlignedData(uint8_t* buffer, uint32_t fetch_size,
+                                               uint32_t start_block) const {
+  if (!WriteFdFmt(fd_, "%08u", start_block)) {
     fprintf(stderr, "failed to write to adb host: %s\n", strerror(errno));
-    return -EIO;
+    return false;
   }
 
-  if (!ReadFdExactly(ad.sfd, buffer, fetch_size)) {
+  if (!ReadFdExactly(fd_, buffer, fetch_size)) {
     fprintf(stderr, "failed to read from adb host: %s\n", strerror(errno));
-    return -EIO;
+    return false;
   }
 
-  return 0;
+  return true;
 }
 
-int run_adb_fuse(int sfd, uint64_t file_size, uint32_t block_size) {
-  adb_data ad;
-  ad.sfd = sfd;
-  ad.file_size = file_size;
-  ad.block_size = block_size;
+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(read_block_adb, ad, std::placeholders::_1, std::placeholders::_2,
-                              std::placeholders::_3);
-  vtab.close = [&ad]() { WriteFdExactly(ad.sfd, "DONEDONE"); };
+  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 36d86d539..e93aa0468 100644
--- a/minadbd/fuse_adb_provider.h
+++ b/minadbd/fuse_adb_provider.h
@@ -19,14 +19,22 @@
 
 #include <stdint.h>
 
-struct adb_data {
-  int sfd;  // file descriptor for the adb channel
+#include "android-base/unique_fd.h"
 
-  uint64_t file_size;
-  uint32_t block_size;
+#include "fuse_provider.h"
+
+// This class reads data from adb server.
+class FuseAdbDataProvider : public FuseDataProvider {
+ public:
+  FuseAdbDataProvider(android::base::unique_fd&& fd, uint64_t file_size, uint32_t block_size)
+      : FuseDataProvider(std::move(fd), file_size, block_size) {}
+
+  bool ReadBlockAlignedData(uint8_t* buffer, uint32_t fetch_size,
+                            uint32_t start_block) const override;
+
+  void Close() override;
 };
 
-int read_block_adb(const adb_data& ad, uint32_t block, uint8_t* buffer, uint32_t fetch_size);
-int run_adb_fuse(int sfd, uint64_t file_size, uint32_t block_size);
+int run_adb_fuse(android::base::unique_fd&& sfd, uint64_t file_size, uint32_t block_size);
 
 #endif
diff --git a/minadbd/fuse_adb_provider_test.cpp b/minadbd/fuse_adb_provider_test.cpp
index 00250e505..0b097129e 100644
--- a/minadbd/fuse_adb_provider_test.cpp
+++ b/minadbd/fuse_adb_provider_test.cpp
@@ -21,19 +21,19 @@
 
 #include <string>
 
+#include <android-base/unique_fd.h>
 #include <gtest/gtest.h>
 
 #include "adb_io.h"
 #include "fuse_adb_provider.h"
 
 TEST(fuse_adb_provider, read_block_adb) {
-  adb_data data = {};
-  int sockets[2];
+  android::base::unique_fd device_socket;
+  android::base::unique_fd host_socket;
 
-  ASSERT_EQ(0, socketpair(AF_UNIX, SOCK_STREAM, 0, sockets));
-  data.sfd = sockets[0];
+  ASSERT_TRUE(android::base::Socketpair(AF_UNIX, SOCK_STREAM, 0, &device_socket, &host_socket));
+  FuseAdbDataProvider data(std::move(device_socket), 0, 0);
 
-  int host_socket = sockets[1];
   fcntl(host_socket, F_SETFL, O_NONBLOCK);
 
   const char expected_data[] = "foobar";
@@ -46,8 +46,8 @@ TEST(fuse_adb_provider, read_block_adb) {
 
   uint32_t block = 1234U;
   const char expected_block[] = "00001234";
-  ASSERT_EQ(0, read_block_adb(data, block, reinterpret_cast<uint8_t*>(block_data),
-                              sizeof(expected_data) - 1));
+  ASSERT_TRUE(data.ReadBlockAlignedData(reinterpret_cast<uint8_t*>(block_data),
+                                        sizeof(expected_data) - 1, block));
 
   // Check that read_block_adb requested the right block.
   char block_req[sizeof(expected_block)] = {};
@@ -65,26 +65,21 @@ TEST(fuse_adb_provider, read_block_adb) {
   errno = 0;
   ASSERT_EQ(-1, read(host_socket, &tmp, 1));
   ASSERT_EQ(EWOULDBLOCK, errno);
-
-  close(sockets[0]);
-  close(sockets[1]);
 }
 
 TEST(fuse_adb_provider, read_block_adb_fail_write) {
-  adb_data data = {};
-  int sockets[2];
+  android::base::unique_fd device_socket;
+  android::base::unique_fd host_socket;
 
-  ASSERT_EQ(0, socketpair(AF_UNIX, SOCK_STREAM, 0, sockets));
-  data.sfd = sockets[0];
+  ASSERT_TRUE(android::base::Socketpair(AF_UNIX, SOCK_STREAM, 0, &device_socket, &host_socket));
+  FuseAdbDataProvider data(std::move(device_socket), 0, 0);
 
-  ASSERT_EQ(0, close(sockets[1]));
+  host_socket.reset();
 
   // write(2) raises SIGPIPE since the reading end has been closed. Ignore the signal to avoid
   // failing the test.
   signal(SIGPIPE, SIG_IGN);
 
   char buf[1];
-  ASSERT_EQ(-EIO, read_block_adb(data, 0, reinterpret_cast<uint8_t*>(buf), 1));
-
-  close(sockets[0]);
+  ASSERT_FALSE(data.ReadBlockAlignedData(reinterpret_cast<uint8_t*>(buf), 1, 0));
 }
diff --git a/minadbd/minadbd_services.cpp b/minadbd/minadbd_services.cpp
index 9309ed749..3e1128546 100644
--- a/minadbd/minadbd_services.cpp
+++ b/minadbd/minadbd_services.cpp
@@ -44,7 +44,7 @@ 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(sfd, file_size, block_size);
+  int result = run_adb_fuse(std::move(sfd), file_size, block_size);
 
   printf("sideload_host finished\n");
   exit(result == 0 ? 0 : 1);
diff --git a/recovery.cpp b/recovery.cpp
index 0074b6433..63960eb15 100644
--- a/recovery.cpp
+++ b/recovery.cpp
@@ -56,7 +56,7 @@
 #include "common.h"
 #include "device.h"
 #include "fsck_unshare_blocks.h"
-#include "fuse_sdcard_provider.h"
+#include "fuse_sdcard_install.h"
 #include "fuse_sideload.h"
 #include "install.h"
 #include "logging.h"
@@ -709,6 +709,7 @@ static void run_graphics_test() {
   ui->ShowText(true);
 }
 
+// TODO(xunchang) move apply_from_sdcard() to fuse_sdcard_install.cpp
 // How long (in seconds) we wait for the fuse-provided package file to
 // appear, before timing out.
 #define SDCARD_INSTALL_TIMEOUT 10
diff --git a/tests/component/sideload_test.cpp b/tests/component/sideload_test.cpp
index d5e074c63..8d290dbcd 100644
--- a/tests/component/sideload_test.cpp
+++ b/tests/component/sideload_test.cpp
@@ -53,9 +53,9 @@ TEST(SideloadTest, run_fuse_sideload) {
   provider_vtab vtab;
   vtab.close = [](void) {};
   vtab.read_block = [&blocks](uint32_t block, uint8_t* buffer, uint32_t fetch_size) {
-    if (block >= 4) return -1;
+    if (block >= 4) return false;
     blocks[block].copy(reinterpret_cast<char*>(buffer), fetch_size);
-    return 0;
+    return true;
   };
 
   TemporaryDir mount_point;