Rezip tool used to modify the APK.

build/android/rezip/rezip.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// rezip is a tool which is used to modify zip files. It reads in a
+// zip file and outputs a new zip file after applying various
+// transforms. The tool is used in the Android Chromium build process
+// to modify an APK file (which are zip files). The main application
+// of this is to modify the APK so that the shared library is no
+// longer compressed. Ironically, this saves both transmission and
+// device drive space. It saves transmission space because
+// uncompressed libraries make much smaller deltas with previous
+// versions. It saves device drive space because it is no longer
+// necessary to have both a compressed and uncompressed shared library
+// on the device. To achieve this the uncompressed library is opened
+// directly from within the APK using the "crazy" linker.
+
+#include <assert.h>
+#include <string.h>
+
+#include <iostream>
+#include <sstream>
+#include <string>
+
+#include "third_party/zlib/contrib/minizip/unzip.h"
+#include "third_party/zlib/contrib/minizip/zip.h"
+
+const int kMaxFilenameInZip = 256;
+const int kMaxExtraFieldInZip = 8192;
+const int kBufferSize = 4096;
+// Note do not use sysconf(_SC_PAGESIZE) here as that will give you the
+// page size of the host, this should be the page size of the target.
+const int kPageSize = 4096;

[bulach, 2014/06/17 16:07:08]

nit: how about kTargetPageSize?

[Anton, 2014/06/18 13:30:41]

changed to PageSizeOnDevice

+
+// This is done to avoid having to make a dependency on all of base.
+class LogStream {
+ public:
+  ~LogStream() {
+    stream_.flush();
+    std::cerr << stream_.str() << std::endl;
+  }
+  std::ostream& stream() {
+    return stream_;
+  }
+ private:
+  std::ostringstream stream_;
+};
+
+#define LOG(tag) (LogStream().stream() << #tag << ":")
+
+// Copy the data from the currently opened file in the zipfile we are unzipping
+// into the currently opened file of the zipfile we are zipping.
+static bool CopySubfile(unzFile in_file,

[bulach, 2014/06/17 16:07:08]

nit: I think it's more common to have an anonymous namespace instead of marking
all these functions as static

[Anton, 2014/06/18 13:30:41]

C++ style says either is fine.

+                        zipFile out_file,
+                        const char* in_zip_filename,
+                        const char* out_zip_filename,
+                        const char* in_filename,
+                        const char* out_filename) {
+  char buf[kBufferSize];
+
+  int bytes = 0;
+  while (true) {
+    bytes = unzReadCurrentFile(in_file, buf, sizeof(buf));
+    if (bytes < 0) {
+      LOG(ERROR) << "failed to read from " << in_filename << " in zipfile "
+                 << in_zip_filename;
+      return false;
+    }
+
+    if (bytes == 0) {
+      break;
+    }
+
+    if (ZIP_OK != zipWriteInFileInZip(out_file, buf, bytes)) {
+      LOG(ERROR) << "failed to write from " << out_filename << " in zipfile "
+                 << out_zip_filename;
+      return false;
+    }
+  }
+
+  return true;
+}
+
+static zip_fileinfo BuildOutInfo(const unz_file_info& in_info) {
+  zip_fileinfo out_info;
+  out_info.tmz_date.tm_sec = in_info.tmu_date.tm_sec;
+  out_info.tmz_date.tm_min = in_info.tmu_date.tm_min;
+  out_info.tmz_date.tm_hour = in_info.tmu_date.tm_hour;
+  out_info.tmz_date.tm_mday = in_info.tmu_date.tm_mday;
+  out_info.tmz_date.tm_mon = in_info.tmu_date.tm_mon;
+  out_info.tmz_date.tm_year = in_info.tmu_date.tm_year;
+
+  out_info.dosDate = in_info.dosDate;
+  out_info.internal_fa = in_info.internal_fa;
+  out_info.external_fa = in_info.external_fa;
+  return out_info;
+}
+
+// RAII pattern for closing the unzip file.
+class UnzipCloser {

[bulach, 2014/06/17 16:07:08]

nit: I think these classes are normally named "Scoped..."

[Anton, 2014/06/18 13:30:41]

Done.

+ public:
+  UnzipCloser(unzFile z_file, const char* z_filename)
+      : z_file_(z_file), z_filename_(z_filename) {}
+
+  ~UnzipCloser() {
+    if (unzClose(z_file_) != UNZ_OK) {
+      LOG(ERROR) << "failed to close input zipfile " << z_filename_;
+      exit(1);
+    }
+  }
+
+ private:
+  const char* z_filename_;
+  unzFile z_file_;
+};
+
+// RAII pattern for closing the out zip file.
+class ZipCloser {
+ public:
+  ZipCloser(zipFile z_file, const char* z_filename)
+      : z_file_(z_file), z_filename_(z_filename) {}
+
+  ~ZipCloser() {
+    if (zipClose(z_file_, NULL) != ZIP_OK) {
+      LOG(ERROR) << "failed to close output zipfile" << z_filename_;
+      exit(1);
+    }
+  }
+
+ private:
+  const char* z_filename_;
+  zipFile z_file_;
+};
+
+typedef std::string (*RenameFun)(const char* in_filename);
+typedef int (*AlignFun)(const char* in_filename,
+                        unzFile in_file,
+                        char* extra_buffer,
+                        int size);
+typedef bool (*InflateFun)(const char* filename);
+
+static bool IsPrefixLibraryFilename(const char* filename,
+                                    const char* base_prefix) {
+  // We are basically matching "lib/[^/]*/<base_prefix>lib.*[.]so".
+  // However, we don't have C++11 regex, so we just handroll the test.
+  // Also we exclude "libchromium_android_linker.so" as a match.
+  const std::string filename_str = filename;
+  const std::string prefix = "lib/";
+  const std::string suffix = ".so";
+
+  if (filename_str.length() < suffix.length() + prefix.length()) {
+    // too short
+    return false;
+  }
+
+  if (filename_str.compare(0, prefix.size(), prefix) != 0) {
+    // does not start with "lib/"
+    return false;
+  }
+
+  if (filename_str.compare(filename_str.length() - suffix.length(),
+                           suffix.length(),
+                           suffix) != 0) {
+    // does not end with ".so"
+    return false;
+  }
+
+  const size_t last_slash = filename_str.find_last_of('/');
+  if (last_slash < prefix.length()) {
+    // Only one slash
+    return false;
+  }
+
+  const size_t second_slash = filename_str.find_first_of('/', prefix.length());
+  if (second_slash != last_slash) {
+    // filename_str contains more than two slashes.
+    return false;
+  }
+
+  const std::string libprefix = std::string(base_prefix) + "lib";
+  if (filename_str.compare(last_slash + 1, libprefix.length(), libprefix) !=
+      0) {
+    // basename piece does not start with <base_prefix>"lib"
+    return false;
+  }
+
+  const std::string linker = "libchromium_android_linker.so";

[bulach, 2014/06/17 16:07:08]

nit: it may be nicer to get this as a param passed by the build system

[Anton, 2014/06/18 13:30:41]

Actually I would prefer it passed the filename of the library, rather than the
filename of the linker, but it doesn't and it would be hard to make it
otherwise.

+  if (last_slash + 1 + linker.length() == filename_str.length() &&
+      filename_str.compare(last_slash + 1, linker.length(), linker) == 0) {
+    // Do not match the linker.
+    return false;
+  }
+  return true;
+}
+
+static bool IsLibraryFilename(const char* filename) {
+  return IsPrefixLibraryFilename(filename, "");
+}
+
+static bool IsCrazyLibraryFilename(const char* filename) {
+  return IsPrefixLibraryFilename(filename, "crazy.");
+}
+
+static std::string RenameLibrary(const char* in_filename) {

[rmcilroy, 2014/06/18 09:17:03]

Add a comment to describe what this function does.  Also, the function name
isn't very descriptive - either call it something like
RenameLibrariesForCrazyLinker, or add pull out basename_prefix as an argument to
make this a generic function which adds a prefix to library files.

[Anton, 2014/06/18 13:30:41]

Done.

+  if (!IsLibraryFilename(in_filename)) {
+    // Don't rename
+    return in_filename;
+  }
+
+  std::string filename_str = in_filename;
+  size_t last_slash = filename_str.find_last_of('/');
+  if (last_slash == std::string::npos ||
+      last_slash == filename_str.length() - 1) {
+    return in_filename;
+  }
+
+  // We rename the library, so that the Android Package Manager
+  // no longer extracts the library.
+  const std::string basename_prefix = "crazy.";
+  return filename_str.substr(0, last_slash + 1) + basename_prefix +
+         filename_str.substr(last_slash + 1);
+}
+
+static int PageAlignCrazyLibrary(const char* in_filename,

[rmcilroy, 2014/06/18 09:17:02]

Comment describing what the function does please.

[Anton, 2014/06/18 13:30:41]

Done.

+                                 unzFile in_file,
+                                 char* extra_buffer,
+                                 int extra_size) {
+  if (!IsCrazyLibraryFilename(in_filename)) {
+    return extra_size;
+  }
+  const ZPOS64_T pos = unzGetCurrentFileZStreamPos64(in_file);
+  const int padding = kPageSize - (pos % kPageSize);
+  if (padding == kPageSize) {
+    return extra_size;
+  }
+
+  assert(extra_size < kMaxExtraFieldInZip - padding);
+  memset(extra_buffer + extra_size, 0, padding);
+  return extra_size + padding;
+}
+
+// As only the read side API provides offsets, we check that we added the
+// correct amount of padding by reading the zip file we just generated.
+static bool CheckPageAlign(const char* out_zip_filename) {
+  unzFile in_file = unzOpen(out_zip_filename);

[bulach, 2014/06/17 16:07:08]

nit: this is repeated 315-320, how about rolling into the unzipCloser?

[Anton, 2014/06/18 13:30:41]

Done.

+  if (in_file == NULL) {
+    LOG(ERROR) << "failed to open zipfile " << out_zip_filename;
+    return false;
+  }
+  UnzipCloser unzipCloser(in_file, out_zip_filename);
+
+  int err = 0;
+  bool checked = false;
+  while (true) {
+    char in_filename[kMaxFilenameInZip + 1];
+    // Get info and extra field for current file.
+    unz_file_info in_info;
+    err = unzGetCurrentFileInfo(in_file,
+                                &in_info,
+                                in_filename,
+                                sizeof(in_filename) - 1,
+                                NULL,
+                                0,
+                                NULL,
+                                0);
+    if (err != UNZ_OK) {
+      LOG(ERROR) << "failed to get filename" << out_zip_filename;
+      return false;
+    }
+    assert(in_info.size_filename <= kMaxFilenameInZip);
+    in_filename[in_info.size_filename] = '\0';
+
+    if (IsCrazyLibraryFilename(in_filename)) {
+      err = unzOpenCurrentFile(in_file);
+      if (err != UNZ_OK) {
+        LOG(ERROR) << "failed to open subfile" << out_zip_filename << " "
+                   << in_filename;
+        return false;
+      }
+
+      const ZPOS64_T pos = unzGetCurrentFileZStreamPos64(in_file);
+      const int alignment = pos % kPageSize;
+      checked = (alignment == 0);
+      if (!checked) {
+        LOG(ERROR) << "Failed to page align library " << in_filename
+                   << ", position " << pos << " alignment " << alignment;
+      }
+
+      err = unzCloseCurrentFile(in_file);
+      if (err != UNZ_OK) {
+        LOG(ERROR) << "failed to close subfile" << out_zip_filename << " "
+                   << in_filename;
+        return false;
+      }
+    }
+
+    const int next = unzGoToNextFile(in_file);
+    if (next == UNZ_END_OF_LIST_OF_FILE) {
+      break;
+    }
+    if (next != UNZ_OK) {
+      LOG(ERROR) << "failed to go to next file" << out_zip_filename;
+      return false;
+    }
+  }
+  return checked;
+}
+
+// Copy files from one archive to another applying alignment, rename and
+// inflate transformations if given.
+static bool Rezip(const char* in_zip_filename,
+                  const char* out_zip_filename,
+                  AlignFun align_fun,
+                  RenameFun rename_fun,
+                  InflateFun inflate_fun) {
+  unzFile in_file = unzOpen(in_zip_filename);
+  if (in_file == NULL) {
+    LOG(ERROR) << "failed to open zipfile " << in_zip_filename;
+    return false;
+  }
+  UnzipCloser unzipCloser(in_file, in_zip_filename);
+
+  zipFile out_file = zipOpen(out_zip_filename, APPEND_STATUS_CREATE);
+  if (unzGoToFirstFile(in_file) != UNZ_OK) {
+    LOG(ERROR) << "failed to go to first file in " << in_zip_filename;
+    return false;
+  }
+  ZipCloser zipCloser(out_file, out_zip_filename);
+
+  int err = 0;
+  while (true) {

[rmcilroy, 2014/06/18 09:17:02]

I'm not keen on the while (true) with the break in the middle of the loop -
could you pull "next" out and loop as: 
do {
  ...
} while ((err = unzGoToNextFile(in_file)) == UNZ_OK);

if (err !=  UNZ_END_OF_LIST_OF_FILE) {
  LOG(ERROR) << "failed to go to next file" << in_zip_filename;
  return false;
}
return true;

Or something similar?

[Anton, 2014/06/18 13:30:41]

Earlier reviewer made me remove the do { } while () loop, so I am not putting it
back now.

+    char in_filename[kMaxFilenameInZip + 1];
+    // Get info and extra field for current file.
+    char extra_buffer[kMaxExtraFieldInZip];
+    unz_file_info in_info;
+    err = unzGetCurrentFileInfo(in_file,
+                                &in_info,
+                                in_filename,
+                                sizeof(in_filename) - 1,
+                                &extra_buffer,
+                                sizeof(extra_buffer),
+                                NULL,
+                                0);
+    if (err != UNZ_OK) {
+      LOG(ERROR) << "failed to get filename " << in_zip_filename;
+      return false;
+    }
+    assert(in_info.size_filename <= kMaxFilenameInZip);
+    in_filename[in_info.size_filename] = '\0';
+
+    std::string out_filename = in_filename;
+    if (rename_fun != NULL) {
+      out_filename = rename_fun(in_filename);
+    }
+
+    bool inflate = false;
+    if (inflate_fun != NULL) {
+      inflate = inflate_fun(in_filename);

[rmcilroy, 2014/06/18 09:17:02]

rename inflate_fun to inflate_filename_filter_fun and InflateFun to
InflateFilenameFilterFun so that it is obvious that this is a filter for files
which are to be inflated, rather than a function which does the inflating.

[Anton, 2014/06/18 13:30:41]

It is not a filter, so I changed it to InflatePredicateFun

+    }
+
+    // Open the current file.
+    int method = 0;
+    int level = 0;
+    int raw = !inflate;
+    err = unzOpenCurrentFile2(in_file, &method, &level, raw);
+    if (inflate) {
+      method = Z_NO_COMPRESSION;
+      level = 0;
+    }
+
+    if (err != UNZ_OK) {
+      LOG(ERROR) << "failed to open subfile " << in_zip_filename << " "
+                 << in_filename;
+      return false;
+    }
+
+    // Get the extra field from the local header.
+    char local_extra_buffer[kMaxExtraFieldInZip];
+    int local_extra_size = unzGetLocalExtrafield(
+        in_file, &local_extra_buffer, sizeof(local_extra_buffer));
+
+    if (align_fun != NULL) {
+      local_extra_size =
+          align_fun(in_filename, in_file, local_extra_buffer, local_extra_size);
+    }
+
+    const char* local_extra = local_extra_size > 0 ? local_extra_buffer : NULL;
+    const char* extra = in_info.size_file_extra > 0 ? extra_buffer : NULL;
+
+    // Build the output info structure from the input info structure.
+    const zip_fileinfo out_info = BuildOutInfo(in_info);
+
+    const int ret = zipOpenNewFileInZip4(out_file,
+                                         out_filename.c_str(),
+                                         &out_info,
+                                         local_extra,
+                                         local_extra_size,
+                                         extra,
+                                         in_info.size_file_extra,
+                                         /* comment */ NULL,
+                                         method,
+                                         level,
+                                         /* raw */ 1,
+                                         /* windowBits */ 0,
+                                         /* memLevel */ 0,
+                                         /* strategy */ 0,
+                                         /* password */ NULL,
+                                         /* crcForCrypting */ 0,
+                                         in_info.version,
+                                         /*flagBase */ 0);

[bulach, 2014/06/17 16:07:08]

nit: space after /*

[Anton, 2014/06/18 13:30:41]

Done.

+
+    if (ZIP_OK != ret) {
+      LOG(ERROR) << "failed to open subfile " << out_zip_filename << " "
+                 << out_filename;
+      return false;
+    }
+
+    if (!CopySubfile(in_file,
+                     out_file,
+                     in_zip_filename,
+                     out_zip_filename,
+                     in_filename,
+                     out_filename.c_str())) {
+      return false;
+    }
+
+    if (ZIP_OK != zipCloseFileInZipRaw(
+                      out_file, in_info.uncompressed_size, in_info.crc)) {
+      LOG(ERROR) << "failed to close subfile " << out_zip_filename << " "
+                 << out_filename;
+      return false;
+    }
+
+    err = unzCloseCurrentFile(in_file);
+    if (err != UNZ_OK) {
+      LOG(ERROR) << "failed to close subfile " << in_zip_filename << " "
+                 << in_filename;
+      return false;
+    }
+    const int next = unzGoToNextFile(in_file);
+    if (next == UNZ_END_OF_LIST_OF_FILE) {
+      break;
+    }
+    if (next != UNZ_OK) {
+      LOG(ERROR) << "failed to go to next file" << in_zip_filename;
+      return false;
+    }
+  }
+
+  return true;
+}
+
+int main(int argc, const char* argv[]) {
+  if (argc != 4) {
+    LOG(ERROR) << "Usage: <action> <in_zipfile> <out_zipfile>";
+    LOG(ERROR) << " <action> is 'inflatealign', 'dropdescriptors' or 'rename'";

[rmcilroy, 2014/06/18 09:17:02]

Please add some description of what the various actions do.

[Anton, 2014/06/18 13:30:41]

Done.

+    exit(1);
+  }
+
+  const char* action = argv[1];
+  const char* in_zip_filename = argv[2];
+  const char* out_zip_filename = argv[3];
+
+  InflateFun inflate_fun = NULL;
+  AlignFun align_fun = NULL;
+  RenameFun rename_fun = NULL;
+  bool checkPageAlign = false;

[bulach, 2014/06/17 16:07:08]

nit: check_page_align

[Anton, 2014/06/18 13:30:41]

Done.

+  if (strcmp("inflatealign", action) == 0) {

[rmcilroy, 2014/06/18 09:17:03]

The functions themselves are specific to the crazy linker, but the tool and the
actions sound very generic.  Could you change the actions to specify that this
is for crazy linker (e.g., inflatealigncrazylibs, renamecrazylibs)?

[Anton, 2014/06/18 13:30:41]

I prefer my non-ugly names.

[rmcilroy, 2014/06/18 14:04:20]

This is fine with the added usage instructions (although I wouldn't call
inflatealign a non-ugly name).

+    inflate_fun = &IsCrazyLibraryFilename;
+    align_fun = &PageAlignCrazyLibrary;
+    checkPageAlign = true;
+  } else if (strcmp("rename", action) == 0) {
+    rename_fun = &RenameLibrary;
+  } else if (strcmp("dropdescriptors", action) == 0) {
+    // Minizip does not know about data descriptors, so the default
+    // copying action will drop the descriptors. This should be fine
+    // as data descriptors are redundant information.
+    // Note we need to explicitly drop the descriptors before trying to
+    // do alignment otherwise we will miscalculate the position because
+    // we don't know about the data descriptors.
+  } else {
+    LOG(ERROR) << "Usage: <action> should be 'inflatealign', "
+                  "'dropdescriptors' or 'rename'";
+    exit(1);
+  }
+
+  if (!Rezip(in_zip_filename,
+             out_zip_filename,
+             align_fun,
+             rename_fun,
+             inflate_fun)) {
+    exit(1);
+  }
+  if (checkPageAlign && !CheckPageAlign(out_zip_filename)) {
+    exit(1);
+  }
+  return 0;
+}