Bug 848385 - Avoid recompressing a szip, and (de)compress in-place (but still with a temporary file). r=nfroyd,khuey
authorMike Hommey <mh+mozilla@glandium.org>
Thu, 11 Apr 2013 09:37:44 +0200
changeset 128399 10f03c8cf438ffdb96a1ba43d238ef46173ed78f
parent 128398 80026f1feb0695d800ec1aea7f4fb79e060d3512
child 128400 70424739336fd9f92103ae82718e46544c2c9958
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersnfroyd, khuey
bugs848385
milestone23.0a1
Bug 848385 - Avoid recompressing a szip, and (de)compress in-place (but still with a temporary file). r=nfroyd,khuey
mozglue/linker/szip.cpp
toolkit/mozapps/installer/packager.mk
--- a/mozglue/linker/szip.cpp
+++ b/mozglue/linker/szip.cpp
@@ -1,15 +1,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include <algorithm>
 #include <map>
 #include <sys/stat.h>
+#include <string>
+#include <sstream>
 #include <cstring>
 #include <cstdlib>
 #include <zlib.h>
 #include <fcntl.h>
 #include <errno.h>
 #include "mozilla/Assertions.h"
 #include "mozilla/Scoped.h"
 #include "SeekableZStream.h"
@@ -201,23 +203,23 @@ private:
 };
 
 /* Decompress a seekable compressed stream */
 int SzipDecompress::run(const char *name, Buffer &origBuf,
                         const char *outName, Buffer &outBuf)
 {
   size_t origSize = origBuf.GetLength();
   if (origSize < sizeof(SeekableZStreamHeader)) {
-    log("%s is not a seekable zstream", name);
-    return 1;
+    log("%s is not compressed", name);
+    return 0;
   }
 
   SeekableZStream zstream;
   if (!zstream.Init(origBuf, origSize))
-    return 1;
+    return 0;
 
   size_t size = zstream.GetUncompressedSize();
 
   /* Give enough room for the uncompressed data */
   if (!outBuf.Resize(size)) {
     log("Error resizing %s: %s", outName, strerror(errno));
     return 1;
   }
@@ -232,16 +234,20 @@ int SzipDecompress::run(const char *name
 int SzipCompress::run(const char *name, Buffer &origBuf,
                       const char *outName, Buffer &outBuf)
 {
   size_t origSize = origBuf.GetLength();
   if (origSize == 0) {
     log("Won't compress %s: it's empty", name);
     return 1;
   }
+  if (SeekableZStreamHeader::validate(origBuf)) {
+    log("Skipping %s: it's already a szip", name);
+    return 0;
+  }
   bool compressed = false;
   log("Size = %" PRIuSize, origSize);
 
   /* Allocate a buffer the size of the uncompressed data: we don't want
    * a compressed file larger than that anyways. */
   if (!outBuf.Resize(origSize)) {
     log("Couldn't allocate output buffer: %s", strerror(errno));
     return 1;
@@ -457,17 +463,17 @@ int main(int argc, char* argv[])
 {
   mozilla::ScopedDeletePtr<SzipAction> action;
   char **firstArg;
   bool compress = true;
   size_t chunkSize = 0;
   SeekableZStream::FilterId filter = SzipCompress::DEFAULT_FILTER;
   size_t dictSize = (size_t) 0;
 
-  for (firstArg = &argv[1]; argc > 3; argc--, firstArg++) {
+  for (firstArg = &argv[1]; argc > 2; argc--, firstArg++) {
     if (!firstArg[0] || firstArg[0][0] != '-')
       break;
     if (strcmp(firstArg[0], "-d") == 0) {
       compress = false;
     } else if (strcmp(firstArg[0], "-c") == 0) {
       firstArg++;
       argc--;
       if (!firstArg[0])
@@ -503,60 +509,74 @@ int main(int argc, char* argv[])
         dictSize = -1;
       } else if (!GetSize(firstArg[0], &dictSize) || (dictSize >= 1 << 16)) {
         log("Invalid dictionary size");
         return 1;
       }
     }
   }
 
-  if (argc != 3 || !firstArg[0] || !firstArg[1] ||
-      (strcmp(firstArg[0], firstArg[1]) == 0)) {
-    log("usage: %s [-d] [-c CHUNKSIZE] [-f FILTER] [-D DICTSIZE] "
-        "in_file out_file", argv[0]);
+  if (argc != 2 || !firstArg[0]) {
+    log("usage: %s [-d] [-c CHUNKSIZE] [-f FILTER] [-D DICTSIZE] file",
+        argv[0]);
     return 1;
   }
 
   if (compress) {
     action = new SzipCompress(chunkSize, filter, dictSize);
   } else {
     if (chunkSize) {
       log("-c is incompatible with -d");
       return 1;
     }
-    if (dictSize != (size_t) -1) {
+    if (dictSize) {
       log("-D is incompatible with -d");
       return 1;
     }
     action = new SzipDecompress();
   }
 
-  FileBuffer origBuf;
-  if (!origBuf.Init(firstArg[0])) {
-    log("Couldn't open %s: %s", firstArg[0], strerror(errno));
-    return 1;
-  }
+  std::stringstream tmpOutStream;
+  tmpOutStream << firstArg[0] << ".sz." << getpid();
+  std::string tmpOut(tmpOutStream.str());
+  int ret;
+  struct stat st;
+  {
+    FileBuffer origBuf;
+    if (!origBuf.Init(firstArg[0])) {
+      log("Couldn't open %s: %s", firstArg[0], strerror(errno));
+      return 1;
+    }
+
+    ret = fstat(origBuf.getFd(), &st);
+    if (ret == -1) {
+      log("Couldn't stat %s: %s", firstArg[0], strerror(errno));
+      return 1;
+    }
 
-  struct stat st;
-  int ret = fstat(origBuf.getFd(), &st);
-  if (ret == -1) {
-    log("Couldn't stat %s: %s", firstArg[0], strerror(errno));
-    return 1;
+    size_t origSize = st.st_size;
+
+    /* Mmap the original file */
+    if (!origBuf.Resize(origSize)) {
+      log("Couldn't mmap %s: %s", firstArg[0], strerror(errno));
+      return 1;
+    }
+
+    /* Create the compressed file */
+    FileBuffer outBuf;
+    if (!outBuf.Init(tmpOut.c_str(), true)) {
+      log("Couldn't open %s: %s", tmpOut.c_str(), strerror(errno));
+      return 1;
+    }
+
+    ret = action->run(firstArg[0], origBuf, tmpOut.c_str(), outBuf);
+    if ((ret == 0) && (fstat(outBuf.getFd(), &st) == -1)) {
+      st.st_size = 0;
+    }
   }
 
-  size_t origSize = st.st_size;
-
-  /* Mmap the original file */
-  if (!origBuf.Resize(origSize)) {
-    log("Couldn't mmap %s: %s", firstArg[0], strerror(errno));
-    return 1;
+  if ((ret == 0) && st.st_size) {
+    rename(tmpOut.c_str(), firstArg[0]);
+  } else {
+    unlink(tmpOut.c_str());
   }
-
-  /* Create the compressed file */
-  FileBuffer outBuf;
-  if (!outBuf.Init(firstArg[1], true)) {
-    log("Couldn't open %s: %s", firstArg[1], strerror(errno));
-    return 1;
-  }
-
-  ret = action->run(firstArg[0], origBuf, firstArg[1], outBuf);
   return ret;
 }
--- a/toolkit/mozapps/installer/packager.mk
+++ b/toolkit/mozapps/installer/packager.mk
@@ -345,17 +345,17 @@ DIST_FILES += libomxplugin.so libomxplug
 endif
 
 ifdef MOZ_ENABLE_SZIP
 SZIP_LIBRARIES := $(filter-out $(MOZ_CHILD_PROCESS_NAME),$(filter %.so,$(DIST_FILES)))
 endif
 
 PKG_SUFFIX      = .apk
 INNER_MAKE_PACKAGE	= \
-  $(foreach lib,$(SZIP_LIBRARIES),host/bin/szip $(MOZ_SZIP_FLAGS) $(STAGEPATH)$(MOZ_PKG_DIR)$(_BINPATH)/$(lib) $(STAGEPATH)$(MOZ_PKG_DIR)$(_BINPATH)/$(lib:.so=.sz) && mv $(STAGEPATH)$(MOZ_PKG_DIR)$(_BINPATH)/$(lib:.so=.sz) $(STAGEPATH)$(MOZ_PKG_DIR)$(_BINPATH)/$(lib) && ) \
+  $(foreach lib,$(SZIP_LIBRARIES),host/bin/szip $(MOZ_SZIP_FLAGS) $(STAGEPATH)$(MOZ_PKG_DIR)$(_BINPATH)/$(lib) && ) \
   make -C $(GECKO_APP_AP_PATH) gecko.ap_ && \
   cp $(GECKO_APP_AP_PATH)/gecko.ap_ $(_ABS_DIST) && \
   ( cd $(STAGEPATH)$(MOZ_PKG_DIR)$(_BINPATH) && \
     mkdir -p lib/$(ABI_DIR) && \
     mv libmozglue.so $(MOZ_CHILD_PROCESS_NAME) lib/$(ABI_DIR) && \
     unzip -o $(_ABS_DIST)/gecko.ap_ && \
     rm $(_ABS_DIST)/gecko.ap_ && \
     $(if $(SZIP_LIBRARIES),$(ZIP) -0 $(_ABS_DIST)/gecko.ap_ $(SZIP_LIBRARIES) && ) \