This is the mail archive of the cygwin-apps mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Fix memleak in setup


If compress::decompress returns NULL, we need to clean up the allocated specialized compress_* objects -- but NOT delete the passed-in io_stream*. The problem occurs Installer::installOne, but is corrected by modifying the behavior of compress::decompress and the various specialized decompression classes.

Detailed explanation:

From Installer::installOne:

  if ((try_decompress = compress::decompress (pkgfile)) != NULL)
    {
      // if we get here, then try_decompress has taken ownership
      // of pkgfile

      if ((tarstream = archive::extract (try_decompress)) == NULL)
        {
          // error handling stuff
          delete try_decompress;
          // since try_decompress owns pkgfile, this cleans
          // up both try_decompress and pkgfile.
          return;
        }

       // this is success...tarstream now owns try_decompress,
       // which in turn owns pkgfile.  Eventually, tarstream
       // will be cleaned up along with both try_decompress and
       // pkgfile.
    }
  // however, when the IF statement above fails (returns NULL), then
  // without my patch the internal decompress object will NOT be
  // deleted before compress::decompress returns NULL. So,
  // compress::decompress() needs to delete that object, but since
  // we need pkgfile to still be valid in order to do the test below,
  // we have to tell the internal decompress object to relinquish
  // ownership of pkgfile, and NOT delete it during its own destructor.
  // That's why the code here in Installer::installOne is not changed,
  // but rather the fix is contained wholly in compress::decompress
  // and the specialized decompression classes.
  else if ((tarstream = archive::extract (pkgfile)) == NULL)
    {
      /* Not a compressed tarball, not a plain tarball, give up.  */
      delete pkgfile;
      ...


-- Chuck

2008-06-29  Charles Wilson

	* compress.cc (compress::decompress): clean up concrete
	decompressor objects on failure -- but in that case, do
	NOT destroy original io_stream.
	* compress_bz.h (compress_bz::release_original): new method.
	(owns_original): new member variable.
	* compress_bz.cc (compress_bz::release_original): new method.
	(compress_bz::compress_bz): take ownership of parent by default.
	(compress_bz::~compress_bz): only delete original if 
	owns_original is true.
	* compress_gz.h (compress_gz::release_original): new method.
	(owns_original): new member variable.
	* compress_gz.cc (compress_gz::release_original): new method.
	(compress_gz::construct): take ownership of parent by default.
	(compress_gz::~compress_gz): only delete original if 
	owns_original is true.

Index: compress.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/compress.cc,v
retrieving revision 2.4
diff -u -r2.4 compress.cc
--- compress.cc	27 Dec 2004 14:44:35 -0000	2.4
+++ compress.cc	29 Jun 2008 20:56:42 -0000
@@ -42,6 +42,9 @@
 	  compress_gz *rv = new compress_gz (original);
 	  if (!rv->error ())
 	    return rv;
+	  /* else */
+	  rv->release_original();
+	  delete rv;
 	  return NULL;
 	}
       else if (memcmp (magic, "BZh", 3) == 0)
@@ -49,6 +52,9 @@
 	  compress_bz *rv = new compress_bz (original);
 	  if (!rv->error ())
 	    return rv;
+	  /* else */
+	  rv->release_original();
+	  delete rv;
 	  return NULL;
 	}
     }
Index: compress_bz.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/compress_bz.cc,v
retrieving revision 2.12
diff -u -r2.12 compress_bz.cc
--- compress_bz.cc	30 Jul 2007 22:55:49 -0000	2.12
+++ compress_bz.cc	29 Jun 2008 20:56:43 -0000
@@ -33,6 +33,7 @@
       return;
     }
   original = parent;
+  owns_original = true;
 
   initialisedOk = 0;
   bufN = 0;
@@ -202,10 +203,16 @@
   return 0;
 }
 
+void
+compress_bz::release_original ()
+{
+  owns_original = false;
+}
+
 compress_bz::~compress_bz ()
 {
   if (initialisedOk)
     BZ2_bzDecompressEnd (&strm);
-  if (original)
+  if (original && owns_original)
     delete original;
 }
Index: compress_bz.h
===================================================================
RCS file: /cvs/cygwin-apps/setup/compress_bz.h,v
retrieving revision 2.9
diff -u -r2.9 compress_bz.h
--- compress_bz.h	5 Apr 2005 21:37:41 -0000	2.9
+++ compress_bz.h	29 Jun 2008 20:56:43 -0000
@@ -51,10 +51,12 @@
       * over a WAN :} */
   virtual size_t get_size () {return 0;};
   virtual int get_mtime ();
+  virtual void release_original (); /* give up ownership of original io_stream */
   /* if you are still needing these hints... give up now! */
     virtual ~ compress_bz ();
 private:
   io_stream *original;
+  bool owns_original;
   char peekbuf[512];
   size_t peeklen;
   int lasterr;
Index: compress_gz.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/compress_gz.cc,v
retrieving revision 2.12
diff -u -r2.12 compress_gz.cc
--- compress_gz.cc	8 Apr 2008 23:50:54 -0000	2.12
+++ compress_gz.cc	29 Jun 2008 20:56:43 -0000
@@ -54,6 +54,7 @@
 compress_gz::construct (io_stream * parent, const char *openmode)
 {
   original = parent;
+  owns_original = true;
   peeklen = 0;
   int err;
   int level = Z_DEFAULT_COMPRESSION;	/* compression level */
@@ -429,6 +430,12 @@
 }
 
 void
+compress_gz::release_original ()
+{
+  owns_original = false;
+}
+
+void
 compress_gz::destroy ()
 {
   if (msg)
@@ -450,7 +457,7 @@
     free (inbuf);
   if (outbuf)
     free (outbuf);
-  if (original)
+  if (original && owns_original)
     delete original;
 }
 
Index: compress_gz.h
===================================================================
RCS file: /cvs/cygwin-apps/setup/compress_gz.h,v
retrieving revision 2.6
diff -u -r2.6 compress_gz.h
--- compress_gz.h	5 Apr 2005 21:37:41 -0000	2.6
+++ compress_gz.h	29 Jun 2008 20:56:43 -0000
@@ -54,6 +54,7 @@
   /* Use seek EOF, then tell (). get_size won't do this incase you are sucking down
    * over a WAN :} */
   virtual size_t get_size () {return 0;};
+  virtual void release_original (); /* give up ownership of original io_stream */
   /* if you are still needing these hints... give up now! */
   virtual ~ compress_gz ();
 private:
@@ -70,6 +71,7 @@
   void destroy ();
   int do_flush (int);
   io_stream *original;
+  bool owns_original;
   /* from zlib */
   z_stream stream;
   int z_err;			/* error code for last stream operation */

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]