Bug 842681 - Refactor the linker Zip code and allow to use an existing memory buffer as a Zip file. r=mwu

mozglue/android/APKOpen.cpp

@@ -212,7 +212,7 @@ loadGeckoLibs(const char *apkName)
   struct rusage usage1;
   getrusage(RUSAGE_THREAD, &usage1);
   
-  RefPtr<Zip> zip = new Zip(apkName);
+  RefPtr<Zip> zip = ZipCollection::GetZip(apkName);
 
 #ifdef MOZ_CRASHREPORTER
   file_ids = (char *)extractBuf("lib.id", zip);
@@ -261,7 +261,7 @@ static int loadSQLiteLibs(const char *apkName)
 {
   chdir(getenv("GRE_HOME"));
 
-  RefPtr<Zip> zip = new Zip(apkName);
+  RefPtr<Zip> zip = ZipCollection::GetZip(apkName);
   if (!lib_mapping) {
     lib_mapping = (struct mapping_info *)calloc(MAX_MAPPING_INFO, sizeof(*lib_mapping));
   }
@@ -294,7 +294,7 @@ loadNSSLibs(const char *apkName)
 {
   chdir(getenv("GRE_HOME"));
 
-  RefPtr<Zip> zip = new Zip(apkName);
+  RefPtr<Zip> zip = ZipCollection::GetZip(apkName);
   if (!lib_mapping) {
     lib_mapping = (struct mapping_info *)calloc(MAX_MAPPING_INFO, sizeof(*lib_mapping));
   }

mozglue/linker/ElfLoader.cpp

@@ -260,7 +260,7 @@ ElfLoader::Load(const char *path, int flags, LibHandle *parent)
   if ((subpath = strchr(path, '!'))) {
     char *zip_path = strndup(path, subpath - path);
     while (*(++subpath) == '/') { }
-    zip = zips.GetZip(zip_path);
+    zip = ZipCollection::GetZip(zip_path);
     Zip::Stream s;
     if (zip && zip->GetStream(subpath, &s)) {
       /* When the MOZ_LINKER_EXTRACT environment variable is set to "1",

mozglue/linker/ElfLoader.h

@@ -412,9 +412,6 @@ private:
   /* Keep track of all registered destructors */
   std::vector<DestructorCaller> destructors;
 
-  /* Keep track of Zips used for library loading */
-  ZipCollection zips;
-
   /* Forward declaration, see further below */
   class DebuggerHelper;
 public:

mozglue/linker/Zip.cpp

@@ -12,49 +12,73 @@
 #include "Logging.h"
 #include "Zip.h"
 
-Zip::Zip(const char *filename, ZipCollection *collection)
-: name(strdup(filename))
-, mapped(MAP_FAILED)
-, nextDir(NULL)
-, entries(NULL)
-, parent(collection)
+mozilla::TemporaryRef<Zip>
+Zip::Create(const char *filename)
 {
   /* Open and map the file in memory */
-  AutoCloseFD fd(open(name, O_RDONLY));
+  AutoCloseFD fd(open(filename, O_RDONLY));
   if (fd == -1) {
     log("Error opening %s: %s", filename, strerror(errno));
-    return;
+    return NULL;
   }
   struct stat st;
   if (fstat(fd, &st) == -1) {
     log("Error stating %s: %s", filename, strerror(errno));
-    return;
+    return NULL;
   }
-  size = st.st_size;
+  size_t size = st.st_size;
   if (size <= sizeof(CentralDirectoryEnd)) {
     log("Error reading %s: too short", filename);
-    return;
+    return NULL;
   }
-  mapped = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, 0);
+  void *mapped = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, 0);
   if (mapped == MAP_FAILED) {
     log("Error mmapping %s: %s", filename, strerror(errno));
-    return;
+    return NULL;
   }
   debug("Mapped %s @%p", filename, mapped);
 
-  /* Store the first Local File entry */
-  nextFile = LocalFile::validate(mapped);
+  return Create(filename, mapped, size);
+}
+
+mozilla::TemporaryRef<Zip>
+Zip::Create(const char *filename, void *mapped, size_t size)
+{
+  mozilla::RefPtr<Zip> zip = new Zip(filename, mapped, size);
+
+  // If neither the first Local File entry nor central directory entries
+  // have been found, the zip was invalid.
+  if (!zip->nextFile && !zip->entries) {
+    log("%s - Invalid zip", filename);
+    return NULL;
+  }
+
+  ZipCollection::Singleton.Register(zip);
+  return zip;
+}
+
+Zip::Zip(const char *filename, void *mapped, size_t size)
+: name(filename ? strdup(filename) : NULL)
+, mapped(mapped)
+, size(size)
+, nextFile(LocalFile::validate(mapped)) // first Local File entry
+, nextDir(NULL)
+, entries(NULL)
+{
+  // If the first local file entry couldn't be found (which can happen
+  // with optimized jars), check the first central directory entry.
+  if (!nextFile)
+    GetFirstEntry();
 }
 
 Zip::~Zip()
 {
-  if (parent)
-    parent->Forget(this);
-  if (mapped != MAP_FAILED) {
+  ZipCollection::Forget(this);
+  if (name) {
     munmap(mapped, size);
     debug("Unmapped %s @%p", name, mapped);
+    free(name);
   }
-  free(name);
 }
 
 bool
@@ -132,8 +156,8 @@ Zip::GetStream(const char *path, Zip::Stream *out) const
 const Zip::DirectoryEntry *
 Zip::GetFirstEntry() const
 {
-  if (entries || mapped == MAP_FAILED)
-    return entries; // entries is NULL in the second case above
+  if (entries)
+    return entries;
 
   const CentralDirectoryEnd *end = NULL;
   const char *_end = static_cast<const char *>(mapped) + size
@@ -155,26 +179,34 @@ Zip::GetFirstEntry() const
   return entries;
 }
 
+ZipCollection ZipCollection::Singleton;
+
 mozilla::TemporaryRef<Zip>
 ZipCollection::GetZip(const char *path)
 {
   /* Search the list of Zips we already have for a match */
-  for (std::vector<Zip *>::iterator it = zips.begin(); it < zips.end(); ++it) {
-    if (strcmp((*it)->GetName(), path) == 0)
+  for (std::vector<Zip *>::iterator it = Singleton.zips.begin();
+       it < Singleton.zips.end(); ++it) {
+    if ((*it)->GetName() && (strcmp((*it)->GetName(), path) == 0))
       return *it;
   }
-  Zip *zip = new Zip(path, this);
-  zips.push_back(zip);
-  return zip;
+  return Zip::Create(path);
+}
+
+void
+ZipCollection::Register(Zip *zip)
+{
+  Singleton.zips.push_back(zip);
 }
 
 void
 ZipCollection::Forget(Zip *zip)
 {
   debug("ZipCollection::Forget(\"%s\")", zip->GetName());
-  std::vector<Zip *>::iterator it = std::find(zips.begin(), zips.end(), zip);
+  std::vector<Zip *>::iterator it = std::find(Singleton.zips.begin(),
+                                              Singleton.zips.end(), zip);
   if (*it == zip) {
-    zips.erase(it);
+    Singleton.zips.erase(it);
   } else {
     debug("ZipCollection::Forget: didn't find \"%s\" in bookkeeping", zip->GetName());
   }

mozglue/linker/Zip.h

@@ -30,11 +30,28 @@ class Zip: public mozilla::RefCounted<Zip>
 {
 public:
   /**
-   * Create a Zip instance for the given file name. In case of error, the
-   * Zip instance is still created but methods will error out.
+   * Create a Zip instance for the given file name. Returns NULL in case
+   * of failure.
    */
-  Zip(const char *filename, ZipCollection *collection = NULL);
+  static mozilla::TemporaryRef<Zip> Create(const char *filename);
 
+  /**
+   * Create a Zip instance using the given buffer.
+   */
+  static mozilla::TemporaryRef<Zip> Create(void *buffer, size_t size) {
+    return Create(NULL, buffer, size);
+  }
+
+private:
+  static mozilla::TemporaryRef<Zip> Create(const char *filename,
+                                           void *buffer, size_t size);
+
+  /**
+   * Private constructor
+   */
+  Zip(const char *filename, void *buffer, size_t size);
+
+public:
   /**
    * Destructor
    */
@@ -303,9 +320,6 @@ private:
 
   /* Pointer to the Directory entries */
   mutable const DirectoryEntry *entries;
-
-  /* ZipCollection containing this Zip */
-  mutable ZipCollection *parent;
 };
 
 /**
@@ -314,19 +328,27 @@ private:
 class ZipCollection
 {
 public:
+  static ZipCollection Singleton;
+
   /**
    * Get a Zip instance for the given path. If there is an existing one
    * already, return that one, otherwise create a new one.
    */
-  mozilla::TemporaryRef<Zip> GetZip(const char *path);
+  static mozilla::TemporaryRef<Zip> GetZip(const char *path);
 
 protected:
+  friend class Zip;
+  /**
+   * Register the given Zip instance. This method is meant to be called
+   * by Zip::Create.
+   */
+  static void Register(Zip *zip);
+
   /**
    * Forget about the given Zip instance. This method is meant to be called
    * by the Zip destructor.
    */
-  friend Zip::~Zip();
-  void Forget(Zip *zip);
+  static void Forget(Zip *zip);
 
 private:
   /* Zip instances bookkept in this collection */

mozglue/tests/TestZip.cpp

@@ -5,6 +5,7 @@
 #include <cstdio>
 #include <unistd.h>
 #include "Zip.h"
+#include "mozilla/RefPtr.h"
 
 extern "C" void report_mapping() { }
 
@@ -44,19 +45,19 @@ int main(int argc, char *argv[])
   }
   chdir(argv[1]);
   Zip::Stream s;
-  Zip z("test.zip");
+  mozilla::RefPtr<Zip> z = ZipCollection::GetZip("test.zip");
   for (size_t i = 0; i < sizeof(test_entries) / sizeof(*test_entries); i++) {
-    if (!z.GetStream(test_entries[i], &s)) {
+    if (!z->GetStream(test_entries[i], &s)) {
       fprintf(stderr, "TEST-UNEXPECTED-FAIL | TestZip | test.zip: Couldn't get entry \"%s\"\n", test_entries[i]);
       return 1;
     }
   }
   fprintf(stderr, "TEST-PASS | TestZip | test.zip could be accessed fully\n");
 
-  Zip z2("no_central_dir.zip");
+  z = ZipCollection::GetZip("no_central_dir.zip");
   for (size_t i = 0; i < sizeof(no_central_dir_entries)
                          / sizeof(*no_central_dir_entries); i++) {
-    if (!z2.GetStream(no_central_dir_entries[i], &s)) {
+    if (!z->GetStream(no_central_dir_entries[i], &s)) {
       fprintf(stderr, "TEST-UNEXPECTED-FAIL | TestZip | no_central_dir.zip: Couldn't get entry \"%s\"\n", no_central_dir_entries[i]);
       return 1;
     }