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 */

Reply via email to