From 785a128aec74dc7bee7d76adafa69c94a4b268aa Mon Sep 17 00:00:00 2001
From: Narayan Kamath <narayan@google.com>
Date: Fri, 17 Apr 2015 11:53:14 +0100
Subject: [PATCH 1/7] Avoid mapping output to memory while writing to a file.

It's unnecessary, and causes issues when the uncompressed
output is large.

Bug: http://b/21558406
Change-Id: I99cfb3933b094c2826c7e6c6de9aab03478fcc53
(cherry picked from commit f899bd534b2dc51b9db8d27c76394b192fe51155)
---
 libziparchive/zip_archive.cc      | 271 ++++++++++++++++++++----------
 libziparchive/zip_archive_test.cc |  88 ++++++++++
 2 files changed, 269 insertions(+), 90 deletions(-)

diff --git a/libziparchive/zip_archive.cc b/libziparchive/zip_archive.cc
index 858234446..4ba91dfb9 100644
--- a/libziparchive/zip_archive.cc
+++ b/libziparchive/zip_archive.cc
@@ -85,7 +85,8 @@ struct EocdRecord {
   // Length of the central directory comment.
   uint16_t comment_length;
  private:
-  DISALLOW_IMPLICIT_CONSTRUCTORS(EocdRecord);
+  EocdRecord() = default;
+  DISALLOW_COPY_AND_ASSIGN(EocdRecord);
 } __attribute__((packed));
 
 // A structure representing the fixed length fields for a single
@@ -138,7 +139,8 @@ struct CentralDirectoryRecord {
   // beginning of this archive.
   uint32_t local_file_header_offset;
  private:
-  DISALLOW_IMPLICIT_CONSTRUCTORS(CentralDirectoryRecord);
+  CentralDirectoryRecord() = default;
+  DISALLOW_COPY_AND_ASSIGN(CentralDirectoryRecord);
 } __attribute__((packed));
 
 // The local file header for a given entry. This duplicates information
@@ -175,7 +177,8 @@ struct LocalFileHeader {
   // will appear immediately after the entry file name.
   uint16_t extra_field_length;
  private:
-  DISALLOW_IMPLICIT_CONSTRUCTORS(LocalFileHeader);
+  LocalFileHeader() = default;
+  DISALLOW_COPY_AND_ASSIGN(LocalFileHeader);
 } __attribute__((packed));
 
 struct DataDescriptor {
@@ -189,10 +192,10 @@ struct DataDescriptor {
   // Uncompressed size of the entry.
   uint32_t uncompressed_size;
  private:
-  DISALLOW_IMPLICIT_CONSTRUCTORS(DataDescriptor);
+  DataDescriptor() = default;
+  DISALLOW_COPY_AND_ASSIGN(DataDescriptor);
 } __attribute__((packed));
 
-#undef DISALLOW_IMPLICIT_CONSTRUCTORS
 
 static const uint32_t kGPBDDFlagMask = 0x0008;         // mask value that signifies that the entry has a DD
 
@@ -324,35 +327,6 @@ struct ZipArchive {
   }
 };
 
-static int32_t CopyFileToFile(int fd, uint8_t* begin, const uint32_t length, uint64_t *crc_out) {
-  static const uint32_t kBufSize = 32768;
-  uint8_t buf[kBufSize];
-
-  uint32_t count = 0;
-  uint64_t crc = 0;
-  while (count < length) {
-    uint32_t remaining = length - count;
-
-    // Safe conversion because kBufSize is narrow enough for a 32 bit signed
-    // value.
-    ssize_t get_size = (remaining > kBufSize) ? kBufSize : remaining;
-    ssize_t actual = TEMP_FAILURE_RETRY(read(fd, buf, get_size));
-
-    if (actual != get_size) {
-      ALOGW("CopyFileToFile: copy read failed (" ZD " vs " ZD ")", actual, get_size);
-      return kIoError;
-    }
-
-    memcpy(begin + count, buf, get_size);
-    crc = crc32(crc, buf, get_size);
-    count += get_size;
-  }
-
-  *crc_out = crc;
-
-  return 0;
-}
-
 /*
  * Round up to the next highest power of 2.
  *
@@ -972,6 +946,128 @@ int32_t Next(void* cookie, ZipEntry* data, ZipEntryName* name) {
   return kIterationEnd;
 }
 
+class Writer {
+ public:
+  virtual bool Append(uint8_t* buf, size_t buf_size) = 0;
+  virtual ~Writer() {}
+ protected:
+  Writer() = default;
+ private:
+  DISALLOW_COPY_AND_ASSIGN(Writer);
+};
+
+// A Writer that writes data to a fixed size memory region.
+// The size of the memory region must be equal to the total size of
+// the data appended to it.
+class MemoryWriter : public Writer {
+ public:
+  MemoryWriter(uint8_t* buf, size_t size) : Writer(),
+      buf_(buf), size_(size), bytes_written_(0) {
+  }
+
+  virtual bool Append(uint8_t* buf, size_t buf_size) override {
+    if (bytes_written_ + buf_size > size_) {
+      ALOGW("Zip: Unexpected size " ZD " (declared) vs " ZD " (actual)",
+            size_, bytes_written_ + buf_size);
+      return false;
+    }
+
+    memcpy(buf_ + bytes_written_, buf, buf_size);
+    bytes_written_ += buf_size;
+    return true;
+  }
+
+ private:
+  uint8_t* const buf_;
+  const size_t size_;
+  size_t bytes_written_;
+};
+
+// A Writer that appends data to a file |fd| at its current position.
+// The file will be truncated to the end of the written data.
+class FileWriter : public Writer {
+ public:
+
+  // Creates a FileWriter for |fd| and prepare to write |entry| to it,
+  // guaranteeing that the file descriptor is valid and that there's enough
+  // space on the volume to write out the entry completely and that the file
+  // is truncated to the correct length.
+  //
+  // Returns a valid FileWriter on success, |nullptr| if an error occurred.
+  static std::unique_ptr<FileWriter> Create(int fd, const ZipEntry* entry) {
+    const uint32_t declared_length = entry->uncompressed_length;
+    const off64_t current_offset = lseek64(fd, 0, SEEK_CUR);
+    if (current_offset == -1) {
+      ALOGW("Zip: unable to seek to current location on fd %d: %s", fd, strerror(errno));
+      return nullptr;
+    }
+
+    int result = 0;
+#if defined(__linux__)
+    if (declared_length > 0) {
+      // Make sure we have enough space on the volume to extract the compressed
+      // entry. Note that the call to ftruncate below will change the file size but
+      // will not allocate space on disk and this call to fallocate will not
+      // change the file size.
+      result = TEMP_FAILURE_RETRY(fallocate(fd, 0, current_offset, declared_length));
+      if (result == -1) {
+        ALOGW("Zip: unable to allocate space for file to %" PRId64 ": %s",
+              static_cast<int64_t>(declared_length + current_offset), strerror(errno));
+        return std::unique_ptr<FileWriter>(nullptr);
+      }
+    }
+#endif  // __linux__
+
+    result = TEMP_FAILURE_RETRY(ftruncate(fd, declared_length + current_offset));
+    if (result == -1) {
+      ALOGW("Zip: unable to truncate file to %" PRId64 ": %s",
+            static_cast<int64_t>(declared_length + current_offset), strerror(errno));
+      return std::unique_ptr<FileWriter>(nullptr);
+    }
+
+    return std::unique_ptr<FileWriter>(new FileWriter(fd, declared_length));
+  }
+
+  virtual bool Append(uint8_t* buf, size_t buf_size) override {
+    if (total_bytes_written_ + buf_size > declared_length_) {
+      ALOGW("Zip: Unexpected size " ZD " (declared) vs " ZD " (actual)",
+            declared_length_, total_bytes_written_ + buf_size);
+      return false;
+    }
+
+    // Keep track of the start position so we can calculate the
+    // total number of bytes written.
+    const uint8_t* const start = buf;
+    size_t bytes_written = 0;
+    while (buf_size > 0) {
+      ssize_t bytes_written = TEMP_FAILURE_RETRY(write(fd_, buf, buf_size));
+      if (bytes_written == -1) {
+        ALOGW("Zip: unable to write " ZD " bytes to file; %s", buf_size, strerror(errno));
+        return false;
+      }
+
+      buf_size -= bytes_written;
+      buf += bytes_written;
+    }
+
+    total_bytes_written_ += static_cast<size_t>(
+        reinterpret_cast<uintptr_t>(buf) - reinterpret_cast<uintptr_t>(start));
+
+    return true;
+  }
+ private:
+  FileWriter(const int fd, const size_t declared_length) :
+      Writer(),
+      fd_(fd),
+      declared_length_(declared_length),
+      total_bytes_written_(0) {
+  }
+
+  const int fd_;
+  const size_t declared_length_;
+  size_t total_bytes_written_;
+};
+
 // This method is using libz macros with old-style-casts
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wold-style-cast"
@@ -980,9 +1076,8 @@ static inline int zlib_inflateInit2(z_stream* stream, int window_bits) {
 }
 #pragma GCC diagnostic pop
 
-static int32_t InflateToFile(int fd, const ZipEntry* entry,
-                             uint8_t* begin, uint32_t length,
-                             uint64_t* crc_out) {
+static int32_t InflateEntryToWriter(int fd, const ZipEntry* entry,
+                                    Writer* writer, uint64_t* crc_out) {
   const size_t kBufSize = 32768;
   std::vector<uint8_t> read_buf(kBufSize);
   std::vector<uint8_t> write_buf(kBufSize);
@@ -1057,12 +1152,10 @@ static int32_t InflateToFile(int fd, const ZipEntry* entry,
     if (zstream.avail_out == 0 ||
       (zerr == Z_STREAM_END && zstream.avail_out != kBufSize)) {
       const size_t write_size = zstream.next_out - &write_buf[0];
-      // The file might have declared a bogus length.
-      if (write_size + write_count > length) {
-        return -1;
+      if (!writer->Append(&write_buf[0], write_size)) {
+        // The file might have declared a bogus length.
+        return kInconsistentInformation;
       }
-      memcpy(begin + write_count, &write_buf[0], write_size);
-      write_count += write_size;
 
       zstream.next_out = &write_buf[0];
       zstream.avail_out = kBufSize;
@@ -1083,8 +1176,41 @@ static int32_t InflateToFile(int fd, const ZipEntry* entry,
   return 0;
 }
 
-int32_t ExtractToMemory(ZipArchiveHandle handle,
-                        ZipEntry* entry, uint8_t* begin, uint32_t size) {
+static int32_t CopyEntryToWriter(int fd, const ZipEntry* entry, Writer* writer,
+                                 uint64_t *crc_out) {
+  static const uint32_t kBufSize = 32768;
+  std::vector<uint8_t> buf(kBufSize);
+
+  const uint32_t length = entry->uncompressed_length;
+  uint32_t count = 0;
+  uint64_t crc = 0;
+  while (count < length) {
+    uint32_t remaining = length - count;
+
+    // Safe conversion because kBufSize is narrow enough for a 32 bit signed
+    // value.
+    const ssize_t block_size = (remaining > kBufSize) ? kBufSize : remaining;
+    const ssize_t actual = TEMP_FAILURE_RETRY(read(fd, &buf[0], block_size));
+
+    if (actual != block_size) {
+      ALOGW("CopyFileToFile: copy read failed (" ZD " vs " ZD ")", actual, block_size);
+      return kIoError;
+    }
+
+    if (!writer->Append(&buf[0], block_size)) {
+      return kIoError;
+    }
+    crc = crc32(crc, &buf[0], block_size);
+    count += block_size;
+  }
+
+  *crc_out = crc;
+
+  return 0;
+}
+
+int32_t ExtractToWriter(ZipArchiveHandle handle,
+                        ZipEntry* entry, Writer* writer) {
   ZipArchive* archive = reinterpret_cast<ZipArchive*>(handle);
   const uint16_t method = entry->method;
   off64_t data_offset = entry->offset;
@@ -1098,9 +1224,9 @@ int32_t ExtractToMemory(ZipArchiveHandle handle,
   int32_t return_value = -1;
   uint64_t crc = 0;
   if (method == kCompressStored) {
-    return_value = CopyFileToFile(archive->fd, begin, size, &crc);
+    return_value = CopyEntryToWriter(archive->fd, entry, writer, &crc);
   } else if (method == kCompressDeflated) {
-    return_value = InflateToFile(archive->fd, entry, begin, size, &crc);
+    return_value = InflateEntryToWriter(archive->fd, entry, writer, &crc);
   }
 
   if (!return_value && entry->has_data_descriptor) {
@@ -1120,55 +1246,20 @@ int32_t ExtractToMemory(ZipArchiveHandle handle,
   return return_value;
 }
 
+int32_t ExtractToMemory(ZipArchiveHandle handle, ZipEntry* entry,
+                        uint8_t* begin, uint32_t size) {
+  std::unique_ptr<Writer> writer(new MemoryWriter(begin, size));
+  return ExtractToWriter(handle, entry, writer.get());
+}
+
 int32_t ExtractEntryToFile(ZipArchiveHandle handle,
                            ZipEntry* entry, int fd) {
-  const uint32_t declared_length = entry->uncompressed_length;
-
-  const off64_t current_offset = lseek64(fd, 0, SEEK_CUR);
-  if (current_offset == -1) {
-    ALOGW("Zip: unable to seek to current location on fd %d: %s", fd,
-          strerror(errno));
+  std::unique_ptr<Writer> writer(FileWriter::Create(fd, entry));
+  if (writer.get() == nullptr) {
     return kIoError;
   }
 
-  int result = 0;
-#if defined(__linux__)
-  // Make sure we have enough space on the volume to extract the compressed
-  // entry. Note that the call to ftruncate below will change the file size but
-  // will not allocate space on disk.
-  if (declared_length > 0) {
-    result = TEMP_FAILURE_RETRY(fallocate(fd, 0, current_offset, declared_length));
-    if (result == -1) {
-      ALOGW("Zip: unable to allocate space for file to %" PRId64 ": %s",
-            static_cast<int64_t>(declared_length + current_offset), strerror(errno));
-      return kIoError;
-    }
-  }
-#endif  // defined(__linux__)
-
-  result = TEMP_FAILURE_RETRY(ftruncate(fd, declared_length + current_offset));
-  if (result == -1) {
-    ALOGW("Zip: unable to truncate file to %" PRId64 ": %s",
-          static_cast<int64_t>(declared_length + current_offset), strerror(errno));
-    return kIoError;
-  }
-
-  // Don't attempt to map a region of length 0. We still need the
-  // ftruncate() though, since the API guarantees that we will truncate
-  // the file to the end of the uncompressed output.
-  if (declared_length == 0) {
-      return 0;
-  }
-
-  android::FileMap map;
-  if (!map.create(kTempMappingFileName, fd, current_offset, declared_length, false)) {
-    return kMmapFailed;
-  }
-
-  const int32_t error = ExtractToMemory(handle, entry,
-                                        reinterpret_cast<uint8_t*>(map.getDataPtr()),
-                                        map.getDataLength());
-  return error;
+  return ExtractToWriter(handle, entry, writer.get());
 }
 
 const char* ErrorCodeString(int32_t error_code) {
diff --git a/libziparchive/zip_archive_test.cc b/libziparchive/zip_archive_test.cc
index 64faa6de2..f8952ce65 100644
--- a/libziparchive/zip_archive_test.cc
+++ b/libziparchive/zip_archive_test.cc
@@ -23,6 +23,7 @@
 #include <unistd.h>
 #include <vector>
 
+#include <base/file.h>
 #include <gtest/gtest.h>
 
 static std::string test_data_dir;
@@ -228,6 +229,44 @@ static const uint32_t kEmptyEntriesZip[] = {
       0x54557478, 0x13030005, 0x7552e25c, 0x01000b78, 0x00428904, 0x13880400,
       0x4b500000, 0x00000605, 0x00010000, 0x004f0001, 0x00430000, 0x00000000 };
 
+// This is a zip file containing a single entry (ab.txt) that contains
+// 90072 repetitions of the string "ab\n" and has an uncompressed length
+// of 270216 bytes.
+static const uint16_t kAbZip[] = {
+  0x4b50, 0x0403, 0x0014, 0x0000, 0x0008, 0x51d2, 0x4698, 0xc4b0,
+  0x2cda, 0x011b, 0x0000, 0x1f88, 0x0004, 0x0006, 0x001c, 0x6261,
+  0x742e, 0x7478, 0x5455, 0x0009, 0x7c03, 0x3a09, 0x7c55, 0x3a09,
+  0x7555, 0x0b78, 0x0100, 0x8904, 0x0042, 0x0400, 0x1388, 0x0000,
+  0xc2ed, 0x0d31, 0x0000, 0x030c, 0x7fa0, 0x3b2e, 0x22ff, 0xa2aa,
+  0x841f, 0x45fc, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555,
+  0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555,
+  0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555,
+  0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555,
+  0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555,
+  0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555,
+  0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555,
+  0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555,
+  0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555,
+  0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555,
+  0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555,
+  0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555,
+  0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555,
+  0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555,
+  0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555,
+  0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555, 0x5555,
+  0x5555, 0x5555, 0x5555, 0x5555, 0xdd55, 0x502c, 0x014b, 0x1e02,
+  0x1403, 0x0000, 0x0800, 0xd200, 0x9851, 0xb046, 0xdac4, 0x1b2c,
+  0x0001, 0x8800, 0x041f, 0x0600, 0x1800, 0x0000, 0x0000, 0x0100,
+  0x0000, 0xa000, 0x0081, 0x0000, 0x6100, 0x2e62, 0x7874, 0x5574,
+  0x0554, 0x0300, 0x097c, 0x553a, 0x7875, 0x000b, 0x0401, 0x4289,
+  0x0000, 0x8804, 0x0013, 0x5000, 0x054b, 0x0006, 0x0000, 0x0100,
+  0x0100, 0x4c00, 0x0000, 0x5b00, 0x0001, 0x0000, 0x0000
+};
+
+static const uint8_t kAbTxtName[] = { 'a', 'b', '.', 't', 'x', 't' };
+static const uint16_t kAbTxtNameLength = sizeof(kAbTxtName);
+static const size_t kAbUncompressedSize = 270216;
+
 static int make_temporary_file(const char* file_name_pattern) {
   char full_path[1024];
   // Account for differences between the host and the target.
@@ -275,6 +314,55 @@ TEST(ziparchive, EmptyEntries) {
   close(output_fd);
 }
 
+TEST(ziparchive, EntryLargerThan32K) {
+  char temp_file_pattern[] = "entry_larger_than_32k_test_XXXXXX";
+  int fd = make_temporary_file(temp_file_pattern);
+  ASSERT_NE(-1, fd);
+  ASSERT_TRUE(android::base::WriteFully(fd, reinterpret_cast<const uint8_t*>(kAbZip),
+                         sizeof(kAbZip) - 1));
+  ZipArchiveHandle handle;
+  ASSERT_EQ(0, OpenArchiveFd(fd, "EntryLargerThan32KTest", &handle));
+
+  ZipEntry entry;
+  ZipEntryName ab_name;
+  ab_name.name = kAbTxtName;
+  ab_name.name_length = kAbTxtNameLength;
+  ASSERT_EQ(0, FindEntry(handle, ab_name, &entry));
+  ASSERT_EQ(kAbUncompressedSize, entry.uncompressed_length);
+
+  // Extract the entry to memory.
+  std::vector<uint8_t> buffer(kAbUncompressedSize);
+  ASSERT_EQ(0, ExtractToMemory(handle, &entry, &buffer[0], buffer.size()));
+
+  // Extract the entry to a file.
+  char output_file_pattern[] = "entry_larger_than_32k_test_output_XXXXXX";
+  int output_fd = make_temporary_file(output_file_pattern);
+  ASSERT_NE(-1, output_fd);
+  ASSERT_EQ(0, ExtractEntryToFile(handle, &entry, output_fd));
+
+  // Make sure the extracted file size is as expected.
+  struct stat stat_buf;
+  ASSERT_EQ(0, fstat(output_fd, &stat_buf));
+  ASSERT_EQ(kAbUncompressedSize, static_cast<size_t>(stat_buf.st_size));
+
+  // Read the file back to a buffer and make sure the contents are
+  // the same as the memory buffer we extracted directly to.
+  std::vector<uint8_t> file_contents(kAbUncompressedSize);
+  ASSERT_EQ(0, lseek64(output_fd, 0, SEEK_SET));
+  ASSERT_TRUE(android::base::ReadFully(output_fd, &file_contents[0], file_contents.size()));
+  ASSERT_EQ(file_contents, buffer);
+
+  for (int i = 0; i < 90072; ++i) {
+    const uint8_t* line = &file_contents[0] + (3 * i);
+    ASSERT_EQ('a', line[0]);
+    ASSERT_EQ('b', line[1]);
+    ASSERT_EQ('\n', line[2]);
+  }
+
+  close(fd);
+  close(output_fd);
+}
+
 TEST(ziparchive, TrailerAfterEOCD) {
   char temp_file_pattern[] = "trailer_after_eocd_test_XXXXXX";
   int fd = make_temporary_file(temp_file_pattern);

From 4ba18cf3ffae39c86ef3c968871b331f1e7df914 Mon Sep 17 00:00:00 2001
From: Greg Hackmann <ghackmann@google.com>
Date: Mon, 27 Apr 2015 10:14:32 -0700
Subject: [PATCH 2/7] libziparchive: fix clang build

Bug: http://b/21558406
Change-Id: I69105a9cde05b182f65c7e574282bb4b48b66e95
Signed-off-by: Greg Hackmann <ghackmann@google.com>
(cherry picked from commit d6eac24aac0069cb6d00d2c723db5adab1a724c4)
---
 libziparchive/zip_archive.cc | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/libziparchive/zip_archive.cc b/libziparchive/zip_archive.cc
index 4ba91dfb9..34131f1be 100644
--- a/libziparchive/zip_archive.cc
+++ b/libziparchive/zip_archive.cc
@@ -268,8 +268,6 @@ static const int32_t kMmapFailed = -12;
 
 static const int32_t kErrorMessageLowerBound = -13;
 
-static const char kTempMappingFileName[] = "zip: ExtractFileToFile";
-
 /*
  * A Read-only Zip archive.
  *
@@ -1038,7 +1036,6 @@ class FileWriter : public Writer {
     // Keep track of the start position so we can calculate the
     // total number of bytes written.
     const uint8_t* const start = buf;
-    size_t bytes_written = 0;
     while (buf_size > 0) {
       ssize_t bytes_written = TEMP_FAILURE_RETRY(write(fd_, buf, buf_size));
       if (bytes_written == -1) {
@@ -1122,7 +1119,6 @@ static int32_t InflateEntryToWriter(int fd, const ZipEntry* entry,
   const uint32_t uncompressed_length = entry->uncompressed_length;
 
   uint32_t compressed_length = entry->compressed_length;
-  uint32_t write_count = 0;
   do {
     /* read as much as we can */
     if (zstream.avail_in == 0) {

From 67ab5d95058d6fe1bab71b3f69e1934728902995 Mon Sep 17 00:00:00 2001
From: Narayan Kamath <narayan@google.com>
Date: Mon, 27 Apr 2015 16:25:53 +0100
Subject: [PATCH 3/7] Use base::WriteFully in zip_archive.

We're already linking against libbase but we'll have to add
a libbase dependency to every target that includes libziparchive
as a STATIC_LIBRARY dependency, given that there's no way to
express that what we want (except by adding a LOCAL_WHOLE_STATIC_LIBRARY
dependency on libbase to libziparchive but that seems bad too)

Bug: http://b/21558406
Change-Id: I294ad389a9c61a1134a7bc323da25b0004a8f1e0
(cherry picked from commit e97e66ea7c624190afa4639d6ddc60e7d013f46c)
---
 fastboot/Android.mk          |  3 ++-
 libziparchive/zip_archive.cc | 23 +++++++----------------
 2 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/fastboot/Android.mk b/fastboot/Android.mk
index 064882636..23cb40449 100644
--- a/fastboot/Android.mk
+++ b/fastboot/Android.mk
@@ -58,7 +58,8 @@ LOCAL_STATIC_LIBRARIES := \
     libsparse_host \
     libutils \
     liblog \
-    libz
+    libz \
+    libbase
 
 ifneq ($(HOST_OS),windows)
 LOCAL_STATIC_LIBRARIES += libselinux
diff --git a/libziparchive/zip_archive.cc b/libziparchive/zip_archive.cc
index 34131f1be..79c4c53c7 100644
--- a/libziparchive/zip_archive.cc
+++ b/libziparchive/zip_archive.cc
@@ -30,6 +30,7 @@
 #include <memory>
 #include <vector>
 
+#include "base/file.h"
 #include "base/macros.h"  // TEMP_FAILURE_RETRY may or may not be in unistd
 #include "base/memory.h"
 #include "log/log.h"
@@ -1033,24 +1034,14 @@ class FileWriter : public Writer {
       return false;
     }
 
-    // Keep track of the start position so we can calculate the
-    // total number of bytes written.
-    const uint8_t* const start = buf;
-    while (buf_size > 0) {
-      ssize_t bytes_written = TEMP_FAILURE_RETRY(write(fd_, buf, buf_size));
-      if (bytes_written == -1) {
-        ALOGW("Zip: unable to write " ZD " bytes to file; %s", buf_size, strerror(errno));
-        return false;
-      }
-
-      buf_size -= bytes_written;
-      buf += bytes_written;
+    const bool result = android::base::WriteFully(fd_, buf, buf_size);
+    if (result) {
+      total_bytes_written_ += buf_size;
+    } else {
+      ALOGW("Zip: unable to write " ZD " bytes to file; %s", buf_size, strerror(errno));
     }
 
-    total_bytes_written_ += static_cast<size_t>(
-        reinterpret_cast<uintptr_t>(buf) - reinterpret_cast<uintptr_t>(start));
-
-    return true;
+    return result;
   }
  private:
   FileWriter(const int fd, const size_t declared_length) :

From 71aebefe49d8126f522287bd35d17c432a4e6383 Mon Sep 17 00:00:00 2001
From: Badhri Jagan Sridharan <Badhri@google.com>
Date: Tue, 2 Jun 2015 14:47:57 -0700
Subject: [PATCH 4/7] libziparchive: fix fallocate failures

The objective of fallocate call seems to be to
make sure that we have enough enough space left
in the disk to house the uncompressed file.
But, fallocate is only supported in the following
file systems:
btrfs, ext4, ocfs2, and xfs

Return error only when fallocate fails due to
lack of space. The immediate ftruncate call is
going to take of the majority of other errors.

Bug: http://b/21558406
Bug: 21561449
Change-Id: I7083f3c7e5d745bd6e8a190ac9020297d638d9d4
(cherry picked from commit a68d0d1fe48afc7a7a7fd0ee42df1607f21fa996)
---
 libziparchive/zip_archive.cc | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/libziparchive/zip_archive.cc b/libziparchive/zip_archive.cc
index 79c4c53c7..b2a9f88c5 100644
--- a/libziparchive/zip_archive.cc
+++ b/libziparchive/zip_archive.cc
@@ -1008,8 +1008,13 @@ class FileWriter : public Writer {
       // entry. Note that the call to ftruncate below will change the file size but
       // will not allocate space on disk and this call to fallocate will not
       // change the file size.
+      // Note: fallocate is only supported by the following filesystems -
+      // btrfs, ext4, ocfs2, and xfs. Therefore fallocate might fail with
+      // EOPNOTSUPP error when issued in other filesystems.
+      // Hence, check for the return error code before concluding that the
+      // disk does not have enough space.
       result = TEMP_FAILURE_RETRY(fallocate(fd, 0, current_offset, declared_length));
-      if (result == -1) {
+      if (result == -1 && errno == ENOSPC) {
         ALOGW("Zip: unable to allocate space for file to %" PRId64 ": %s",
               static_cast<int64_t>(declared_length + current_offset), strerror(errno));
         return std::unique_ptr<FileWriter>(nullptr);

From e283ca29c53817091a17f44b7c93ba6d6362fd38 Mon Sep 17 00:00:00 2001
From: Elliott Hughes <enh@google.com>
Date: Tue, 2 Jun 2015 13:50:00 -0700
Subject: [PATCH 5/7] Add "fastboot --version".

So bug reporters can actually tell us what they're running.

Bug: http://b/21558406
Bug: http://b/21583225
Change-Id: If2a4ae97b4792aa321566603ce2c354a72d32307
(cherry picked from commit 379646b2ca2cf681be3489eb74a421b3f8c80e26)
---
 fastboot/Android.mk   | 7 +++++--
 fastboot/fastboot.cpp | 8 ++++++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/fastboot/Android.mk b/fastboot/Android.mk
index 23cb40449..66a470a64 100644
--- a/fastboot/Android.mk
+++ b/fastboot/Android.mk
@@ -14,6 +14,8 @@
 
 LOCAL_PATH:= $(call my-dir)
 
+fastboot_version := $(shell git -C $(LOCAL_PATH) rev-parse --short=12 HEAD 2>/dev/null)-android
+
 include $(CLEAR_VARS)
 
 LOCAL_C_INCLUDES := $(LOCAL_PATH)/../mkbootimg \
@@ -25,14 +27,15 @@ LOCAL_MODULE_TAGS := debug
 LOCAL_CONLYFLAGS += -std=gnu99
 LOCAL_CFLAGS += -Wall -Wextra -Werror -Wunreachable-code
 
+LOCAL_CFLAGS += -DFASTBOOT_REVISION='"$(fastboot_version)"'
+
 ifeq ($(HOST_OS),linux)
   LOCAL_SRC_FILES += usb_linux.c util_linux.c
 endif
 
 ifeq ($(HOST_OS),darwin)
   LOCAL_SRC_FILES += usb_osx.c util_osx.c
-  LOCAL_LDLIBS += -lpthread -framework CoreFoundation -framework IOKit \
-	-framework Carbon
+  LOCAL_LDLIBS += -lpthread -framework CoreFoundation -framework IOKit -framework Carbon
   LOCAL_CFLAGS += -Wno-unused-parameter
 endif
 
diff --git a/fastboot/fastboot.cpp b/fastboot/fastboot.cpp
index 51985afac..c9a677281 100644
--- a/fastboot/fastboot.cpp
+++ b/fastboot/fastboot.cpp
@@ -964,8 +964,9 @@ int main(int argc, char **argv)
         {"page_size", required_argument, 0, 'n'},
         {"ramdisk_offset", required_argument, 0, 'r'},
         {"tags_offset", required_argument, 0, 't'},
-        {"help", 0, 0, 'h'},
-        {"unbuffered", 0, 0, 0},
+        {"help", no_argument, 0, 'h'},
+        {"unbuffered", no_argument, 0, 0},
+        {"version", no_argument, 0, 0},
         {0, 0, 0, 0}
     };
 
@@ -1037,6 +1038,9 @@ int main(int argc, char **argv)
             if (strcmp("unbuffered", longopts[longindex].name) == 0) {
                 setvbuf(stdout, NULL, _IONBF, 0);
                 setvbuf(stderr, NULL, _IONBF, 0);
+            } else if (strcmp("version", longopts[longindex].name) == 0) {
+                fprintf(stdout, "fastboot version %s\n", FASTBOOT_REVISION);
+                return 0;
             }
             break;
         default:

From c688c23286f40309e5af1f7f91d6d6f1de3f7749 Mon Sep 17 00:00:00 2001
From: Elliott Hughes <enh@google.com>
Date: Tue, 2 Jun 2015 13:34:07 -0700
Subject: [PATCH 6/7] 'usb' doesn't need to be global in fastboot.

Bug: http://b/21558406
Change-Id: Id014399640865d889918661bae0161b3165eee48
(cherry picked from commit c0ce65f9613eefc7bb2b14daae395bde52339ab7)
---
 fastboot/fastboot.cpp | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/fastboot/fastboot.cpp b/fastboot/fastboot.cpp
index c9a677281..e587590ae 100644
--- a/fastboot/fastboot.cpp
+++ b/fastboot/fastboot.cpp
@@ -59,7 +59,6 @@
 
 char cur_product[FB_RESPONSE_SZ + 1];
 
-static usb_handle *usb = 0;
 static const char *serial = 0;
 static const char *product = 0;
 static const char *cmdline = 0;
@@ -609,7 +608,7 @@ static int64_t get_sparse_limit(struct usb_handle *usb, int64_t size)
  * erase partitions of type ext4 before flashing a filesystem so no stale
  * inodes are left lying around.  Otherwise, e2fsck gets very upset.
  */
-static int needs_erase(const char *part)
+static int needs_erase(usb_handle* usb, const char *part)
 {
     /* The function fb_format_supported() currently returns the value
      * we want, so just call it.
@@ -737,7 +736,7 @@ void do_update(usb_handle *usb, const char *filename, int erase_first)
         int rc = load_buf_fd(usb, fd, &buf);
         if (rc) die("cannot load %s from flash", images[i].img_name);
         do_update_signature(zip, images[i].sig_name);
-        if (erase_first && needs_erase(images[i].part_name)) {
+        if (erase_first && needs_erase(usb, images[i].part_name)) {
             fb_queue_erase(images[i].part_name);
         }
         flash_buf(images[i].part_name, &buf);
@@ -792,7 +791,7 @@ void do_flashall(usb_handle *usb, int erase_first)
             die("could not load %s\n", images[i].img_name);
         }
         do_send_signature(fname);
-        if (erase_first && needs_erase(images[i].part_name)) {
+        if (erase_first && needs_erase(usb, images[i].part_name)) {
             fb_queue_erase(images[i].part_name);
         }
         flash_buf(images[i].part_name, &buf);
@@ -860,7 +859,8 @@ static int64_t parse_num(const char *arg)
     return num;
 }
 
-void fb_perform_format(const char *partition, int skip_if_not_supported,
+void fb_perform_format(usb_handle* usb,
+                       const char *partition, int skip_if_not_supported,
                        const char *type_override, const char *size_override)
 {
     char pTypeBuff[FB_RESPONSE_SZ + 1], pSizeBuff[FB_RESPONSE_SZ + 1];
@@ -1067,7 +1067,7 @@ int main(int argc, char **argv)
         return 0;
     }
 
-    usb = open_device();
+    usb_handle* usb = open_device();
 
     while (argc > 0) {
         if(!strcmp(*argv, "getvar")) {
@@ -1109,10 +1109,10 @@ int main(int argc, char **argv)
             }
             if (type_override && !type_override[0]) type_override = NULL;
             if (size_override && !size_override[0]) size_override = NULL;
-            if (erase_first && needs_erase(argv[1])) {
+            if (erase_first && needs_erase(usb, argv[1])) {
                 fb_queue_erase(argv[1]);
             }
-            fb_perform_format(argv[1], 0, type_override, size_override);
+            fb_perform_format(usb, argv[1], 0, type_override, size_override);
             skip(2);
         } else if(!strcmp(*argv, "signature")) {
             require(2);
@@ -1167,7 +1167,7 @@ int main(int argc, char **argv)
                 skip(2);
             }
             if (fname == 0) die("cannot determine image filename for '%s'", pname);
-            if (erase_first && needs_erase(pname)) {
+            if (erase_first && needs_erase(usb, pname)) {
                 fb_queue_erase(pname);
             }
             do_flash(usb, pname, fname);
@@ -1218,9 +1218,9 @@ int main(int argc, char **argv)
 
     if (wants_wipe) {
         fb_queue_erase("userdata");
-        fb_perform_format("userdata", 1, NULL, NULL);
+        fb_perform_format(usb, "userdata", 1, NULL, NULL);
         fb_queue_erase("cache");
-        fb_perform_format("cache", 1, NULL, NULL);
+        fb_perform_format(usb, "cache", 1, NULL, NULL);
     }
     if (wants_reboot) {
         fb_queue_reboot();

From 966339b2d60c0721a6ec9ebd541e291b1b4c34df Mon Sep 17 00:00:00 2001
From: Elliott Hughes <enh@google.com>
Date: Tue, 2 Jun 2015 21:37:05 -0700
Subject: [PATCH 7/7] Don't say "update package missing" unless we mean it.

unzip_to_file reports failures itself these days, so there's it's unhelpful
of the caller to just guess what might have gone wrong.

Bug: http://b/21558406
Change-Id: I1e3d06c6cf902b8c6ef333dc60fd8f49680a493b
(cherry picked from commit acdbe92c60e662a4913f1fca09c2b8913791376c)
---
 fastboot/fastboot.cpp | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/fastboot/fastboot.cpp b/fastboot/fastboot.cpp
index e587590ae..6724d0571 100644
--- a/fastboot/fastboot.cpp
+++ b/fastboot/fastboot.cpp
@@ -724,13 +724,14 @@ void do_update(usb_handle *usb, const char *filename, int erase_first)
 
     setup_requirements(reinterpret_cast<char*>(data), sz);
 
-    for (size_t i = 0; i < ARRAY_SIZE(images); i++) {
+    for (size_t i = 0; i < ARRAY_SIZE(images); ++i) {
         int fd = unzip_to_file(zip, images[i].img_name);
-        if (fd < 0) {
-            if (images[i].is_optional)
+        if (fd == -1) {
+            if (images[i].is_optional) {
                 continue;
+            }
             CloseArchive(zip);
-            die("update package missing %s", images[i].img_name);
+            exit(1); // unzip_to_file already explained why.
         }
         fastboot_buffer buf;
         int rc = load_buf_fd(usb, fd, &buf);