From 834326ce7af9c78d779c0691603815f5106f0c36 Mon Sep 17 00:00:00 2001
From: Yurii Zubrytskyi <zyy@google.com>
Date: Wed, 20 Dec 2017 01:01:01 -0800
Subject: [PATCH] Get rid of unneeded allocations in Extract...() APIs

Both Extract...() functions don't need dynamic allocation
for the writers, as those are strictly scoped. This CL
changes heap allocation to stack allocation.

Test: zip_archive_test

Change-Id: Id727e4b9848235cd063cc67ecbe052d21ca21326
---
 libziparchive/zip_archive.cc | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/libziparchive/zip_archive.cc b/libziparchive/zip_archive.cc
index 5ccbcc2a5..f9f8c73d9 100644
--- a/libziparchive/zip_archive.cc
+++ b/libziparchive/zip_archive.cc
@@ -783,12 +783,12 @@ class FileWriter : public zip_archive::Writer {
   // block device).
   //
   // Returns a valid FileWriter on success, |nullptr| if an error occurred.
-  static std::unique_ptr<FileWriter> Create(int fd, const ZipEntry* entry) {
+  static 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;
+      return FileWriter{};
     }
 
     int result = 0;
@@ -808,7 +808,7 @@ class FileWriter : public zip_archive::Writer {
         ALOGW("Zip: unable to allocate %" PRId64 " bytes at offset %" PRId64 ": %s",
               static_cast<int64_t>(declared_length), static_cast<int64_t>(current_offset),
               strerror(errno));
-        return std::unique_ptr<FileWriter>(nullptr);
+        return FileWriter{};
       }
     }
 #endif  // __linux__
@@ -816,7 +816,7 @@ class FileWriter : public zip_archive::Writer {
     struct stat sb;
     if (fstat(fd, &sb) == -1) {
       ALOGW("Zip: unable to fstat file: %s", strerror(errno));
-      return std::unique_ptr<FileWriter>(nullptr);
+      return FileWriter{};
     }
 
     // Block device doesn't support ftruncate(2).
@@ -825,13 +825,22 @@ class FileWriter : public zip_archive::Writer {
       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 FileWriter{};
       }
     }
 
-    return std::unique_ptr<FileWriter>(new FileWriter(fd, declared_length));
+    return FileWriter(fd, declared_length);
   }
 
+  FileWriter(FileWriter&& other)
+      : fd_(other.fd_),
+        declared_length_(other.declared_length_),
+        total_bytes_written_(other.total_bytes_written_) {
+    other.fd_ = -1;
+  }
+
+  bool IsValid() const { return fd_ != -1; }
+
   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_,
@@ -850,10 +859,10 @@ class FileWriter : public zip_archive::Writer {
   }
 
  private:
-  FileWriter(const int fd, const size_t declared_length)
+  explicit FileWriter(const int fd = -1, const size_t declared_length = 0)
       : Writer(), fd_(fd), declared_length_(declared_length), total_bytes_written_(0) {}
 
-  const int fd_;
+  int fd_;
   const size_t declared_length_;
   size_t total_bytes_written_;
 };
@@ -1066,17 +1075,17 @@ int32_t ExtractToWriter(ZipArchiveHandle handle, ZipEntry* entry, zip_archive::W
 }
 
 int32_t ExtractToMemory(ZipArchiveHandle handle, ZipEntry* entry, uint8_t* begin, uint32_t size) {
-  std::unique_ptr<zip_archive::Writer> writer(new MemoryWriter(begin, size));
-  return ExtractToWriter(handle, entry, writer.get());
+  MemoryWriter writer(begin, size);
+  return ExtractToWriter(handle, entry, &writer);
 }
 
 int32_t ExtractEntryToFile(ZipArchiveHandle handle, ZipEntry* entry, int fd) {
-  std::unique_ptr<zip_archive::Writer> writer(FileWriter::Create(fd, entry));
-  if (writer.get() == nullptr) {
+  auto writer = FileWriter::Create(fd, entry);
+  if (!writer.IsValid()) {
     return kIoError;
   }
 
-  return ExtractToWriter(handle, entry, writer.get());
+  return ExtractToWriter(handle, entry, &writer);
 }
 
 const char* ErrorCodeString(int32_t error_code) {