Bug 1459722 - Remove zxx_stream. r?froydnj draft
authorMike Hommey <mh+mozilla@glandium.org>
Thu, 10 May 2018 11:45:23 +0900
changeset 793448 3a640582bb41865ba982563157de4f8a65fe7cba
parent 792270 59005ba3cd3e7b3f9e8804bea881bf4c3a755d7c
push id109388
push userbmo:mh+mozilla@glandium.org
push dateThu, 10 May 2018 02:47:59 +0000
reviewersfroydnj
bugs1459722
milestone62.0a1
Bug 1459722 - Remove zxx_stream. r?froydnj It was necessary back when we were doing decompression from a signal handler, because we couldn't then have zlib call malloc, but we don't do that anymore, so the whole wrapping is effectively unused. With the wrapping gone, we manually initialize the zalloc, zfree and opaque fields, as specified in the zlib documentation.
mozglue/linker/Mappable.cpp
mozglue/linker/Mappable.h
mozglue/linker/Zip.cpp
mozglue/linker/Zip.h
--- a/mozglue/linker/Mappable.cpp
+++ b/mozglue/linker/Mappable.cpp
@@ -179,17 +179,17 @@ MappableExtractFile::Create(const char *
     /* Map the temporary file for use as inflate buffer */
     MappedPtr buffer(MemoryRange::mmap(nullptr, stream->GetUncompressedSize(),
                                        PROT_WRITE, MAP_SHARED, fd, 0));
     if (buffer == MAP_FAILED) {
       ERROR("Couldn't map %s to decompress library", file.get());
       return nullptr;
     }
 
-    zxx_stream zStream = stream->GetZStream(buffer);
+    z_stream zStream = stream->GetZStream(buffer);
 
     /* Decompress */
     if (inflateInit2(&zStream, -MAX_WBITS) != Z_OK) {
       ERROR("inflateInit failed: %s", zStream.msg);
       return nullptr;
     }
     if (inflate(&zStream, Z_FINISH) != Z_STREAM_END) {
       ERROR("inflate failed: %s", zStream.msg);
--- a/mozglue/linker/Mappable.h
+++ b/mozglue/linker/Mappable.h
@@ -152,12 +152,12 @@ private:
 
   /* Zip reference */
   RefPtr<Zip> zip;
 
   /* Decompression buffer */
   mozilla::UniquePtr<_MappableBuffer> buffer;
 
   /* Zlib data */
-  zxx_stream zStream;
+  z_stream zStream;
 };
 
 #endif /* Mappable_h */
--- a/mozglue/linker/Zip.cpp
+++ b/mozglue/linker/Zip.cpp
@@ -200,21 +200,24 @@ Zip::VerifyCRCs() const
               uint32_t(entry->CRC32));
 
     if (entry->compression == Stream::Type::STORE) {
       crc = crc32(crc, static_cast<const uint8_t*>(file->GetData()),
                   entry->compressedSize);
       DEBUG_LOG(" STORE size=%d crc=%08x", int(entry->compressedSize), crc);
 
     } else if (entry->compression == Stream::Type::DEFLATE) {
-      zxx_stream zstream;
+      z_stream zstream;
       Bytef buffer[1024];
       zstream.avail_in = entry->compressedSize;
       zstream.next_in = reinterpret_cast<Bytef *>(
                         const_cast<void *>(file->GetData()));
+      zstream.zalloc = nullptr;
+      zstream.zfree = nullptr;
+      zstream.opaque = nullptr;
 
       if (inflateInit2(&zstream, -MAX_WBITS) != Z_OK) {
         return false;
       }
 
       for (;;) {
         zstream.avail_out = sizeof(buffer);
         zstream.next_out = buffer;
--- a/mozglue/linker/Zip.h
+++ b/mozglue/linker/Zip.h
@@ -11,150 +11,16 @@
 #include <zlib.h>
 #include <pthread.h>
 #include "Utils.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/RefCounted.h"
 #include "mozilla/RefPtr.h"
 
 /**
- * Helper class wrapping z_stream to avoid malloc() calls during
- * inflate. Do not use for deflate.
- * inflateInit allocates two buffers:
- * - one for its internal state, which is "approximately 10K bytes" according
- *   to inflate.h from zlib.
- * - one for the compression window, which depends on the window size passed
- *   to inflateInit2, but is never greater than 32K (1 << MAX_WBITS).
- * Those buffers are created at instantiation time instead of when calling
- * inflateInit2. When inflateInit2 is called, it will call zxx_stream::Alloc
- * to get both these buffers. zxx_stream::Alloc will choose one of the
- * pre-allocated buffers depending on the requested size.
- */
-class zxx_stream: public z_stream
-{
-public:
-  /* Forward declaration */
-  class StaticAllocator;
-
-  explicit zxx_stream(StaticAllocator *allocator_=nullptr)
-  : allocator(allocator_)
-  {
-    memset(this, 0, sizeof(z_stream));
-    zalloc = Alloc;
-    zfree = Free;
-    opaque = this;
-  }
-
-private:
-  static void *Alloc(void *data, uInt items, uInt size)
-  {
-    zxx_stream *zStream = reinterpret_cast<zxx_stream *>(data);
-    if (zStream->allocator) {
-      return zStream->allocator->Alloc(items, size);
-    }
-    size_t buf_size = items * size;
-    return ::operator new(buf_size);
-  }
-
-  static void Free(void *data, void *ptr)
-  {
-    zxx_stream *zStream = reinterpret_cast<zxx_stream *>(data);
-    if (zStream->allocator) {
-      zStream->allocator->Free(ptr);
-    } else {
-      ::operator delete(ptr);
-    }
-  }
-
-  /**
-   * Helper class for each buffer in StaticAllocator.
-   */
-  template <size_t Size>
-  class ZStreamBuf
-  {
-  public:
-    ZStreamBuf() : inUse(false) { }
-
-    bool get(char*& out)
-    {
-      if (!inUse) {
-        inUse = true;
-        out = buf;
-        return true;
-      } else {
-        return false;
-      }
-    }
-
-    void Release()
-    {
-      memset(buf, 0, Size);
-      inUse = false;
-    }
-
-    bool Equals(const void *other) { return other == buf; }
-
-    static const size_t size = Size;
-
-  private:
-    char buf[Size];
-    bool inUse;
-  };
-
-public:
-  /**
-   * Special allocator that uses static buffers to allocate from.
-   */
-  class StaticAllocator
-  {
-  public:
-    void *Alloc(uInt items, uInt size)
-    {
-      if (items == 1 && size <= stateBuf1.size) {
-        char* res = nullptr;
-        if (stateBuf1.get(res) || stateBuf2.get(res)) {
-          return res;
-        }
-        MOZ_CRASH("ZStreamBuf already in use");
-      } else if (items * size == windowBuf1.size) {
-        char* res = nullptr;
-        if (windowBuf1.get(res) || windowBuf2.get(res)) {
-          return res;
-        }
-        MOZ_CRASH("ZStreamBuf already in use");
-      } else {
-        MOZ_CRASH("No ZStreamBuf for allocation");
-      }
-    }
-
-    void Free(void *ptr)
-    {
-      if (stateBuf1.Equals(ptr)) {
-        stateBuf1.Release();
-      } else if (stateBuf2.Equals(ptr)) {
-        stateBuf2.Release();
-      }else if (windowBuf1.Equals(ptr)) {
-        windowBuf1.Release();
-      } else if (windowBuf2.Equals(ptr)) {
-        windowBuf2.Release();
-      } else {
-        MOZ_CRASH("Pointer doesn't match a ZStreamBuf");
-      }
-    }
-
-    // 0x3000 is an arbitrary size above 10K.
-    ZStreamBuf<0x3000> stateBuf1, stateBuf2;
-    ZStreamBuf<1 << MAX_WBITS> windowBuf1, windowBuf2;
-  };
-
-private:
-  StaticAllocator *allocator;
-};
-
-/**
  * Forward declaration
  */
 class ZipCollection;
 
 /**
  * Class to handle access to Zip archive streams. The Zip archive is mapped
  * in memory, and streams are direct references to that mapped memory.
  * Zip files are assumed to be correctly formed. No boundary checks are
@@ -221,28 +87,31 @@ public:
      */
     const void *GetBuffer() { return compressedBuf; }
     size_t GetSize() { return compressedSize; }
     size_t GetUncompressedSize() { return uncompressedSize; }
     size_t GetCRC32() { return CRC32; }
     Type GetType() { return type; }
 
     /**
-     * Returns a zxx_stream for use with inflate functions using the given
+     * Returns a z_stream for use with inflate functions using the given
      * buffer as inflate output. The caller is expected to allocate enough
      * memory for the Stream uncompressed size.
      */
-    zxx_stream GetZStream(void *buf)
+    z_stream GetZStream(void *buf)
     {
-      zxx_stream zStream;
+      z_stream zStream;
       zStream.avail_in = compressedSize;
       zStream.next_in = reinterpret_cast<Bytef *>(
                         const_cast<void *>(compressedBuf));
       zStream.avail_out = uncompressedSize;
       zStream.next_out = static_cast<Bytef *>(buf);
+      zStream.zalloc = nullptr;
+      zStream.zfree = nullptr;
+      zStream.opaque = nullptr;
       return zStream;
     }
 
   protected:
     friend class Zip;
     const void *compressedBuf;
     size_t compressedSize;
     size_t uncompressedSize;