Eliminate resource leaks from zip::ZipFiles and

third_party/zlib/google/zip_internal.cc

 // Copyright (c) 2011 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.
 
 #include "third_party/zlib/google/zip_internal.h"
 
 #include <algorithm>
 
 #include "base/files/file_util.h"
 #include "base/logging.h"
@@ skipping 72 matching lines @@
   FILE* file = NULL;
   const char* mode_fopen = NULL;
 
   if ((mode & ZLIB_FILEFUNC_MODE_READWRITEFILTER) == ZLIB_FILEFUNC_MODE_READ)
     mode_fopen = "rb";
   else if (mode & ZLIB_FILEFUNC_MODE_EXISTING)
     mode_fopen = "r+b";
   else if (mode & ZLIB_FILEFUNC_MODE_CREATE)
     mode_fopen = "wb";
 
-  if ((filename != NULL) && (mode_fopen != NULL))
-    file = fdopen(*static_cast<int*>(opaque), mode_fopen);
+  if ((filename != NULL) && (mode_fopen != NULL)) {
+    int fd = dup(*static_cast<int*>(opaque));
+    if (fd != -1)
+      file = fdopen(fd, mode_fopen);
+  }
 
   return file;
 }
 
-// We don't actually close the file stream since that would close
-// the underlying file descriptor, and we don't own it. However we do need to
-// flush buffers and free |opaque| since we malloc'ed it in FillFdOpenFileFunc.
 int CloseFileFunc(void* opaque, void* stream) {
-  fflush(static_cast<FILE*>(stream));
+  fclose(static_cast<FILE*>(stream));
   free(opaque);
   return 0;
 }
 
 // Fills |pzlib_filecunc_def| appropriately to handle the zip file
 // referred to by |fd|.
 void FillFdOpenFileFunc(zlib_filefunc_def* pzlib_filefunc_def, int fd) {
   fill_fopen_filefunc(pzlib_filefunc_def);
   pzlib_filefunc_def->zopen_file = FdOpenFileFunc;
   pzlib_filefunc_def->zclose_file = CloseFileFunc;
@@ skipping 10 matching lines @@
   file_ret.hf = static_cast<HANDLE>(opaque);
   file_ret.error = 0;
   if (file_ret.hf == INVALID_HANDLE_VALUE)
     return NULL;
 
   void* ret = malloc(sizeof(WIN32FILE_IOWIN));
   if (ret != NULL)
     *(static_cast<WIN32FILE_IOWIN*>(ret)) = file_ret;
   return ret;
 }
+
+int HandleCloseFileFunc(void* opaque, void* stream) {

[James Hawkins, 2014/11/17 23:10:38]

Can we provide better clarity about what the difference between CloseFileFunc
and HandleCloseFileFunc are?

[jeremyspiegel, 2014/11/17 23:13:50]

Would renaming CloseFileFunc to FdCloseFileFunc help? (The difference is that
CloseFileFunc is used on POSIX while HandleCloseFileFunc is used on Windows)

[James Hawkins, 2014/11/17 23:28:39]

Yeah, I think so.  It'd also be good to document the ownership and lifecycles of
these objects (that you're changing).

+  free(stream); // malloc'ed in HandleOpenFileFunc()
+  return 0;
+}
 #endif
 
 // A struct that contains data required for zlib functions to extract files from
 // a zip archive stored in memory directly. The following I/O API functions
 // expect their opaque parameters refer to this struct.
 struct ZipBuffer {
   const char* data;  // weak
   size_t length;
   size_t offset;
 };
@@ skipping 131 matching lines @@
   // Passing dummy "fd" filename to zlib.
   return unzOpen2("fd", &zip_funcs);
 }
 #endif
 
 #if defined(OS_WIN)
 unzFile OpenHandleForUnzipping(HANDLE zip_handle) {
   zlib_filefunc_def zip_funcs;
   fill_win32_filefunc(&zip_funcs);
   zip_funcs.zopen_file = HandleOpenFileFunc;
+  zip_funcs.zclose_file = HandleCloseFileFunc;
   zip_funcs.opaque = zip_handle;
   return unzOpen2("fd", &zip_funcs);
 }
 #endif
 
 // static
 unzFile PrepareMemoryForUnzipping(const std::string& data) {
   if (data.empty())
     return NULL;
 
@@ skipping 74 matching lines @@
                     0,  // versionMadeBy
                     LANGUAGE_ENCODING_FLAG)) {  // flagBase
     DLOG(ERROR) << "Could not open zip file entry " << str_path;
     return false;
   }
   return true;
 }
 
 }  // namespace internal
 }  // namespace zip