]> git.ipfire.org Git - thirdparty/libarchive.git/commitdiff
Fix a number of memory leaks:
authorTim Kientzle <kientzle@gmail.com>
Fri, 10 Apr 2009 05:36:25 +0000 (01:36 -0400)
committerTim Kientzle <kientzle@gmail.com>
Fri, 10 Apr 2009 05:36:25 +0000 (01:36 -0400)
 * write_set_compression_{gzip,bzip2,lzma,xz} need to call finish
   even if nothing is ever written so that the config data
   can be cleaned up.  Set the finish entry in the initial
   setup function (instead of the init function), have the
   finish function only close out the compression if there
   is a state, update the tests to exercise set up and
   tear down without any intermediate open and/or writing.
 * read_open_{fd,filename} would leak data if the fd or file
   didn't exist.  Do all of the fd/file checks before trying
   to allocate data.
 * archive_entry leaked the sourcepath field
 * support_format_mtree leaked the list of active default fields
 * several tests leaked archive_entry objects
 * test/read_open_memory has a special mode to exercise
   providing a minimal set of callbacks; unfortunately it
   omitted the close callback, which caused a leak.
 * Add a chokepoint to test/main.c where we can do something
   for every assertion.  I temporarily added a line # printf here
   to help identify the location of certain crashes.

All of this needs to be merged to libarchive/2.7 branch.

SVN-Revision: 944

16 files changed:
libarchive/archive_entry.c
libarchive/archive_read_open_fd.c
libarchive/archive_read_open_filename.c
libarchive/archive_read_support_format_mtree.c
libarchive/archive_write_disk.c
libarchive/archive_write_set_compression_bzip2.c
libarchive/archive_write_set_compression_gzip.c
libarchive/archive_write_set_compression_xz.c
libarchive/test/main.c
libarchive/test/read_open_memory.c
libarchive/test/test_extattr_freebsd.c
libarchive/test/test_write_compress_bzip2.c
libarchive/test/test_write_compress_gzip.c
libarchive/test/test_write_compress_lzma.c
libarchive/test/test_write_compress_xz.c
libarchive/test/test_write_disk_sparse.c

index 77c81a137bfd387c5435be1c462ffa8e018292cc..ffbd92f6e281086fb9a9ec2b617d3d1d5f2e3413 100644 (file)
@@ -370,6 +370,7 @@ archive_entry_clear(struct archive_entry *entry)
        aes_clean(&entry->ae_gname);
        aes_clean(&entry->ae_hardlink);
        aes_clean(&entry->ae_pathname);
+       aes_clean(&entry->ae_sourcepath);
        aes_clean(&entry->ae_symlink);
        aes_clean(&entry->ae_uname);
        archive_entry_acl_clear(entry);
index 3e348f7b1427482fca44e2953e5dda89dda34c65..c7f9d693b43337d584154e7d8a56355e00504878 100644 (file)
@@ -66,6 +66,11 @@ archive_read_open_fd(struct archive *a, int fd, size_t block_size)
        struct read_fd_data *mine;
        void *b;
 
+       if (fstat(fd, &st) != 0) {
+               archive_set_error(a, errno, "Can't stat fd %d", fd);
+               return (ARCHIVE_FATAL);
+       }
+
        mine = (struct read_fd_data *)malloc(sizeof(*mine));
        b = malloc(block_size);
        if (mine == NULL || b == NULL) {
@@ -77,11 +82,6 @@ archive_read_open_fd(struct archive *a, int fd, size_t block_size)
        mine->block_size = block_size;
        mine->buffer = b;
        mine->fd = fd;
-       /* lseek() hardly ever works, so disable it by default.  See below. */
-       if (fstat(mine->fd, &st) != 0) {
-               archive_set_error(a, errno, "Can't stat fd %d", mine->fd);
-               return (ARCHIVE_FATAL);
-       }
        /*
         * Skip support is a performance optimization for anything
         * that supports lseek().  On FreeBSD, only regular files and
index e813a710df73ecdaadc3b58d462819c699a81916..04efb0f5a9ab6117bb0611e48c8207d245c8dac4 100644 (file)
@@ -82,10 +82,21 @@ archive_read_open_filename(struct archive *a, const char *filename,
        struct stat st;
        struct read_file_data *mine;
        void *b;
+       int fd;
 
        if (filename == NULL || filename[0] == '\0')
                return (archive_read_open_fd(a, 0, block_size));
 
+       fd = open(filename, O_RDONLY | O_BINARY);
+       if (fd < 0) {
+               archive_set_error(a, errno, "Failed to open '%s'", filename);
+               return (ARCHIVE_FATAL);
+       }
+       if (fstat(fd, &st) != 0) {
+               archive_set_error(a, errno, "Can't stat '%s'", filename);
+               return (ARCHIVE_FATAL);
+       }
+
        mine = (struct read_file_data *)malloc(sizeof(*mine) + strlen(filename));
        b = malloc(block_size);
        if (mine == NULL || b == NULL) {
@@ -97,17 +108,7 @@ archive_read_open_filename(struct archive *a, const char *filename,
        strcpy(mine->filename, filename);
        mine->block_size = block_size;
        mine->buffer = b;
-       mine->fd = open(mine->filename, O_RDONLY | O_BINARY);
-       if (mine->fd < 0) {
-               archive_set_error(a, errno, "Failed to open '%s'",
-                   mine->filename);
-               return (ARCHIVE_FATAL);
-       }
-       if (fstat(mine->fd, &st) != 0) {
-               archive_set_error(a, errno, "Can't stat '%s'",
-                   mine->filename);
-               return (ARCHIVE_FATAL);
-       }
+       mine->fd = fd;
        /* Remember mode so close can decide whether to flush. */
        mine->st_mode = st.st_mode;
        /* If we're reading a file from disk, ensure that we don't
index 447f9226808b2e83aea0228bd94aedad4407c5bf..0f0e116e0e0079d86f31ba8ea09af223e6ebcf98 100644 (file)
@@ -404,10 +404,13 @@ read_mtree(struct archive_read *a, struct mtree *mtree)
                len = readline(a, mtree, &p, 256);
                if (len == 0) {
                        mtree->this_entry = mtree->entries;
+                       free_options(global);
                        return (ARCHIVE_OK);
                }
-               if (len < 0)
+               if (len < 0) {
+                       free_options(global);
                        return (len);
+               }
                /* Leading whitespace is never significant, ignore it. */
                while (*p == ' ' || *p == '\t') {
                        ++p;
@@ -432,13 +435,16 @@ read_mtree(struct archive_read *a, struct mtree *mtree)
                } else
                        break;
 
-               if (r != ARCHIVE_OK)
+               if (r != ARCHIVE_OK) {
+                       free_options(global);
                        return r;
+               }
        }
 
        archive_set_error(&a->archive, ARCHIVE_ERRNO_FILE_FORMAT,
            "Can't parse line %ju", counter);
-       return ARCHIVE_FATAL;
+       free_options(global);
+       return (ARCHIVE_FATAL);
 }
 
 /*
index 0d65a3b979a5275b530d8592602042bb5106ccd9..54fab828a303eccb0e375a7ae04efabf042f1208 100644 (file)
@@ -1278,6 +1278,8 @@ _archive_write_finish(struct archive *_a)
                (a->cleanup_gid)(a->lookup_gid_data);
        if (a->cleanup_uid != NULL && a->lookup_uid_data != NULL)
                (a->cleanup_uid)(a->lookup_uid_data);
+       if (a->entry)
+               archive_entry_free(a->entry);
        archive_string_free(&a->_name_data);
        archive_string_free(&a->archive.error_string);
        archive_string_free(&a->path_safe);
index d6d82352f1474d1cbbc50d42e32cd06be9cc4e85..17f6a0a4d3c91d622eea095628817a21eedb6b8b 100644 (file)
@@ -99,6 +99,7 @@ archive_write_set_compression_bzip2(struct archive *_a)
                return (ARCHIVE_FATAL);
        }
        a->compressor.config = config;
+       a->compressor.finish = archive_compressor_bzip2_finish;
        config->compression_level = 9; /* default */
        a->compressor.init = &archive_compressor_bzip2_init;
        a->compressor.options = &archive_compressor_bzip2_options;
@@ -145,7 +146,6 @@ archive_compressor_bzip2_init(struct archive_write *a)
        state->stream.next_out = state->compressed;
        state->stream.avail_out = state->compressed_buffer_size;
        a->compressor.write = archive_compressor_bzip2_write;
-       a->compressor.finish = archive_compressor_bzip2_finish;
 
        /* Initialize compression library */
        ret = BZ2_bzCompressInit(&(state->stream),
@@ -255,83 +255,88 @@ archive_compressor_bzip2_finish(struct archive_write *a)
        ssize_t bytes_written;
        unsigned tocopy;
 
-       state = (struct private_data *)a->compressor.data;
        ret = ARCHIVE_OK;
-       if (a->client_writer == NULL) {
-               archive_set_error(&a->archive, ARCHIVE_ERRNO_PROGRAMMER,
-                   "No write callback is registered?\n"
-                   "This is probably an internal programming error.");
-               ret = ARCHIVE_FATAL;
-               goto cleanup;
-       }
+       state = (struct private_data *)a->compressor.data;
+       if (state != NULL) {
+               if (a->client_writer == NULL) {
+                       archive_set_error(&a->archive,
+                           ARCHIVE_ERRNO_PROGRAMMER,
+                           "No write callback is registered?\n"
+                           "This is probably an internal programming error.");
+                       ret = ARCHIVE_FATAL;
+                       goto cleanup;
+               }
 
-       /* By default, always pad the uncompressed data. */
-       if (a->pad_uncompressed) {
-               tocopy = a->bytes_per_block -
-                   (state->total_in % a->bytes_per_block);
-               while (tocopy > 0 && tocopy < (unsigned)a->bytes_per_block) {
-                       SET_NEXT_IN(state, a->nulls);
-                       state->stream.avail_in = tocopy < a->null_length ?
-                           tocopy : a->null_length;
-                       state->total_in += state->stream.avail_in;
-                       tocopy -= state->stream.avail_in;
-                       ret = drive_compressor(a, state, 0);
-                       if (ret != ARCHIVE_OK)
-                               goto cleanup;
+               /* By default, always pad the uncompressed data. */
+               if (a->pad_uncompressed) {
+                       tocopy = a->bytes_per_block -
+                           (state->total_in % a->bytes_per_block);
+                       while (tocopy > 0 && tocopy < (unsigned)a->bytes_per_block) {
+                               SET_NEXT_IN(state, a->nulls);
+                               state->stream.avail_in = tocopy < a->null_length ?
+                                   tocopy : a->null_length;
+                               state->total_in += state->stream.avail_in;
+                               tocopy -= state->stream.avail_in;
+                               ret = drive_compressor(a, state, 0);
+                               if (ret != ARCHIVE_OK)
+                                       goto cleanup;
+                       }
                }
-       }
 
-       /* Finish compression cycle. */
-       if ((ret = drive_compressor(a, state, 1)))
-               goto cleanup;
-
-       /* Optionally, pad the final compressed block. */
-       block_length = state->stream.next_out - state->compressed;
-
-
-       /* Tricky calculation to determine size of last block. */
-       target_block_length = block_length;
-       if (a->bytes_in_last_block <= 0)
-               /* Default or Zero: pad to full block */
-               target_block_length = a->bytes_per_block;
-       else
-               /* Round length to next multiple of bytes_in_last_block. */
-               target_block_length = a->bytes_in_last_block *
-                   ( (block_length + a->bytes_in_last_block - 1) /
-                       a->bytes_in_last_block);
-       if (target_block_length > a->bytes_per_block)
-               target_block_length = a->bytes_per_block;
-       if (block_length < target_block_length) {
-               memset(state->stream.next_out, 0,
-                   target_block_length - block_length);
-               block_length = target_block_length;
-       }
+               /* Finish compression cycle. */
+               if ((ret = drive_compressor(a, state, 1)))
+                       goto cleanup;
+
+               /* Optionally, pad the final compressed block. */
+               block_length = state->stream.next_out - state->compressed;
+
+               /* Tricky calculation to determine size of last block. */
+               target_block_length = block_length;
+               if (a->bytes_in_last_block <= 0)
+                       /* Default or Zero: pad to full block */
+                       target_block_length = a->bytes_per_block;
+               else
+                       /* Round length to next multiple of bytes_in_last_block. */
+                       target_block_length = a->bytes_in_last_block *
+                           ( (block_length + a->bytes_in_last_block - 1) /
+                               a->bytes_in_last_block);
+               if (target_block_length > a->bytes_per_block)
+                       target_block_length = a->bytes_per_block;
+               if (block_length < target_block_length) {
+                       memset(state->stream.next_out, 0,
+                           target_block_length - block_length);
+                       block_length = target_block_length;
+               }
 
-       /* Write the last block */
-       bytes_written = (a->client_writer)(&a->archive, a->client_data,
-           state->compressed, block_length);
+               /* Write the last block */
+               bytes_written = (a->client_writer)(&a->archive, a->client_data,
+                   state->compressed, block_length);
 
-       /* TODO: Handle short write of final block. */
-       if (bytes_written <= 0)
-               ret = ARCHIVE_FATAL;
-       else {
-               a->archive.raw_position += ret;
-               ret = ARCHIVE_OK;
-       }
+               /* TODO: Handle short write of final block. */
+               if (bytes_written <= 0)
+                       ret = ARCHIVE_FATAL;
+               else {
+                       a->archive.raw_position += ret;
+                       ret = ARCHIVE_OK;
+               }
 
-       /* Cleanup: shut down compressor, release memory, etc. */
+               /* Cleanup: shut down compressor, release memory, etc. */
 cleanup:
-       switch (BZ2_bzCompressEnd(&(state->stream))) {
-       case BZ_OK:
-               break;
-       default:
-               archive_set_error(&a->archive, ARCHIVE_ERRNO_PROGRAMMER,
-                   "Failed to clean up compressor");
-               ret = ARCHIVE_FATAL;
-       }
+               switch (BZ2_bzCompressEnd(&(state->stream))) {
+               case BZ_OK:
+                       break;
+               default:
+                       archive_set_error(&a->archive, ARCHIVE_ERRNO_PROGRAMMER,
+                           "Failed to clean up compressor");
+                       ret = ARCHIVE_FATAL;
+               }
 
-       free(state->compressed);
-       free(state);
+               free(state->compressed);
+               free(state);
+       }
+       /* Free configuration data even if we were never fully initialized. */
+       free(a->compressor.config);
+       a->compressor.config = NULL;
        return (ret);
 }
 
index 35303fc02f050f676eb8328cd9e5f0acefe32ba5..a8b1c33360de8ba0bf7f811b5920357c2a18d9ac 100644 (file)
@@ -102,6 +102,7 @@ archive_write_set_compression_gzip(struct archive *_a)
                return (ARCHIVE_FATAL);
        }
        a->compressor.config = config;
+       a->compressor.finish = &archive_compressor_gzip_finish;
        config->compression_level = Z_DEFAULT_COMPRESSION;
        a->compressor.init = &archive_compressor_gzip_init;
        a->compressor.options = &archive_compressor_gzip_options;
@@ -186,7 +187,6 @@ archive_compressor_gzip_init(struct archive_write *a)
        state->stream.avail_out -= 10;
 
        a->compressor.write = archive_compressor_gzip_write;
-       a->compressor.finish = archive_compressor_gzip_finish;
 
        /* Initialize compression library. */
        ret = deflateInit2(&(state->stream),
@@ -281,7 +281,6 @@ archive_compressor_gzip_write(struct archive_write *a, const void *buff,
        return (ARCHIVE_OK);
 }
 
-
 /*
  * Finish the compression...
  */
@@ -296,113 +295,118 @@ archive_compressor_gzip_finish(struct archive_write *a)
 
        state = (struct private_data *)a->compressor.data;
        ret = 0;
-       if (a->client_writer == NULL) {
-               archive_set_error(&a->archive, ARCHIVE_ERRNO_PROGRAMMER,
-                   "No write callback is registered?  "
-                   "This is probably an internal programming error.");
-               ret = ARCHIVE_FATAL;
-               goto cleanup;
-       }
+       if (state != NULL) {
+               if (a->client_writer == NULL) {
+                       archive_set_error(&a->archive,
+                           ARCHIVE_ERRNO_PROGRAMMER,
+                           "No write callback is registered?  "
+                           "This is probably an internal programming error.");
+                       ret = ARCHIVE_FATAL;
+                       goto cleanup;
+               }
 
-       /* By default, always pad the uncompressed data. */
-       if (a->pad_uncompressed) {
-               tocopy = a->bytes_per_block -
-                   (state->total_in % a->bytes_per_block);
-               while (tocopy > 0 && tocopy < (unsigned)a->bytes_per_block) {
-                       SET_NEXT_IN(state, a->nulls);
-                       state->stream.avail_in = tocopy < a->null_length ?
-                           tocopy : a->null_length;
-                       state->crc = crc32(state->crc, a->nulls,
-                           state->stream.avail_in);
-                       state->total_in += state->stream.avail_in;
-                       tocopy -= state->stream.avail_in;
-                       ret = drive_compressor(a, state, 0);
-                       if (ret != ARCHIVE_OK)
+               /* By default, always pad the uncompressed data. */
+               if (a->pad_uncompressed) {
+                       tocopy = a->bytes_per_block -
+                           (state->total_in % a->bytes_per_block);
+                       while (tocopy > 0 && tocopy < (unsigned)a->bytes_per_block) {
+                               SET_NEXT_IN(state, a->nulls);
+                               state->stream.avail_in = tocopy < a->null_length ?
+                                   tocopy : a->null_length;
+                               state->crc = crc32(state->crc, a->nulls,
+                                   state->stream.avail_in);
+                               state->total_in += state->stream.avail_in;
+                               tocopy -= state->stream.avail_in;
+                               ret = drive_compressor(a, state, 0);
+                               if (ret != ARCHIVE_OK)
+                                       goto cleanup;
+                       }
+               }
+
+               /* Finish compression cycle */
+               if (((ret = drive_compressor(a, state, 1))) != ARCHIVE_OK)
+                       goto cleanup;
+
+               /* Build trailer: 4-byte CRC and 4-byte length. */
+               trailer[0] = (state->crc)&0xff;
+               trailer[1] = (state->crc >> 8)&0xff;
+               trailer[2] = (state->crc >> 16)&0xff;
+               trailer[3] = (state->crc >> 24)&0xff;
+               trailer[4] = (state->total_in)&0xff;
+               trailer[5] = (state->total_in >> 8)&0xff;
+               trailer[6] = (state->total_in >> 16)&0xff;
+               trailer[7] = (state->total_in >> 24)&0xff;
+
+               /* Add trailer to current block. */
+               tocopy = 8;
+               if (tocopy > state->stream.avail_out)
+                       tocopy = state->stream.avail_out;
+               memcpy(state->stream.next_out, trailer, tocopy);
+               state->stream.next_out += tocopy;
+               state->stream.avail_out -= tocopy;
+
+               /* If it overflowed, flush and start a new block. */
+               if (tocopy < 8) {
+                       bytes_written = (a->client_writer)(&a->archive, a->client_data,
+                           state->compressed, state->compressed_buffer_size);
+                       if (bytes_written <= 0) {
+                               ret = ARCHIVE_FATAL;
                                goto cleanup;
+                       }
+                       a->archive.raw_position += bytes_written;
+                       state->stream.next_out = state->compressed;
+                       state->stream.avail_out = state->compressed_buffer_size;
+                       memcpy(state->stream.next_out, trailer + tocopy, 8-tocopy);
+                       state->stream.next_out += 8-tocopy;
+                       state->stream.avail_out -= 8-tocopy;
                }
-       }
 
-       /* Finish compression cycle */
-       if (((ret = drive_compressor(a, state, 1))) != ARCHIVE_OK)
-               goto cleanup;
-
-       /* Build trailer: 4-byte CRC and 4-byte length. */
-       trailer[0] = (state->crc)&0xff;
-       trailer[1] = (state->crc >> 8)&0xff;
-       trailer[2] = (state->crc >> 16)&0xff;
-       trailer[3] = (state->crc >> 24)&0xff;
-       trailer[4] = (state->total_in)&0xff;
-       trailer[5] = (state->total_in >> 8)&0xff;
-       trailer[6] = (state->total_in >> 16)&0xff;
-       trailer[7] = (state->total_in >> 24)&0xff;
-
-       /* Add trailer to current block. */
-       tocopy = 8;
-       if (tocopy > state->stream.avail_out)
-               tocopy = state->stream.avail_out;
-       memcpy(state->stream.next_out, trailer, tocopy);
-       state->stream.next_out += tocopy;
-       state->stream.avail_out -= tocopy;
-
-       /* If it overflowed, flush and start a new block. */
-       if (tocopy < 8) {
+               /* Optionally, pad the final compressed block. */
+               block_length = state->stream.next_out - state->compressed;
+
+               /* Tricky calculation to determine size of last block. */
+               target_block_length = block_length;
+               if (a->bytes_in_last_block <= 0)
+                       /* Default or Zero: pad to full block */
+                       target_block_length = a->bytes_per_block;
+               else
+                       /* Round length to next multiple of bytes_in_last_block. */
+                       target_block_length = a->bytes_in_last_block *
+                           ( (block_length + a->bytes_in_last_block - 1) /
+                               a->bytes_in_last_block);
+               if (target_block_length > a->bytes_per_block)
+                       target_block_length = a->bytes_per_block;
+               if (block_length < target_block_length) {
+                       memset(state->stream.next_out, 0,
+                           target_block_length - block_length);
+                       block_length = target_block_length;
+               }
+
+               /* Write the last block */
                bytes_written = (a->client_writer)(&a->archive, a->client_data,
-                   state->compressed, state->compressed_buffer_size);
+                   state->compressed, block_length);
                if (bytes_written <= 0) {
                        ret = ARCHIVE_FATAL;
                        goto cleanup;
                }
                a->archive.raw_position += bytes_written;
-               state->stream.next_out = state->compressed;
-               state->stream.avail_out = state->compressed_buffer_size;
-               memcpy(state->stream.next_out, trailer + tocopy, 8-tocopy);
-               state->stream.next_out += 8-tocopy;
-               state->stream.avail_out -= 8-tocopy;
-       }
-
-       /* Optionally, pad the final compressed block. */
-       block_length = state->stream.next_out - state->compressed;
-
-
-       /* Tricky calculation to determine size of last block. */
-       target_block_length = block_length;
-       if (a->bytes_in_last_block <= 0)
-               /* Default or Zero: pad to full block */
-               target_block_length = a->bytes_per_block;
-       else
-               /* Round length to next multiple of bytes_in_last_block. */
-               target_block_length = a->bytes_in_last_block *
-                   ( (block_length + a->bytes_in_last_block - 1) /
-                       a->bytes_in_last_block);
-       if (target_block_length > a->bytes_per_block)
-               target_block_length = a->bytes_per_block;
-       if (block_length < target_block_length) {
-               memset(state->stream.next_out, 0,
-                   target_block_length - block_length);
-               block_length = target_block_length;
-       }
 
-       /* Write the last block */
-       bytes_written = (a->client_writer)(&a->archive, a->client_data,
-           state->compressed, block_length);
-       if (bytes_written <= 0) {
-               ret = ARCHIVE_FATAL;
-               goto cleanup;
-       }
-       a->archive.raw_position += bytes_written;
-
-       /* Cleanup: shut down compressor, release memory, etc. */
-cleanup:
-       switch (deflateEnd(&(state->stream))) {
-       case Z_OK:
-               break;
-       default:
-               archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
-                   "Failed to clean up compressor");
-               ret = ARCHIVE_FATAL;
+               /* Cleanup: shut down compressor, release memory, etc. */
+       cleanup:
+               switch (deflateEnd(&(state->stream))) {
+               case Z_OK:
+                       break;
+               default:
+                       archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
+                           "Failed to clean up compressor");
+                       ret = ARCHIVE_FATAL;
+               }
+               free(state->compressed);
+               free(state);
        }
-       free(state->compressed);
-       free(state);
+       /* Clean up config area even if we never initialized. */
+       free(a->compressor.config);
+       a->compressor.config = NULL;
        return (ret);
 }
 
index 96c0577d569e7d6cd3aecb9f9bd14e57a546bfdc..8396252527cf0c4d09c1bb3179ac8ee79932072d 100644 (file)
@@ -104,6 +104,7 @@ archive_write_set_compression_xz(struct archive *_a)
                return (ARCHIVE_FATAL);
        }
        a->compressor.config = config;
+       a->compressor.finish = archive_compressor_xz_finish;
        config->compression_level = LZMA_PRESET_DEFAULT;
        a->compressor.init = &archive_compressor_xz_init;
        a->compressor.options = &archive_compressor_xz_options;
@@ -197,7 +198,6 @@ archive_compressor_xz_init(struct archive_write *a)
                return (ARCHIVE_FATAL);
        }
        a->compressor.write = archive_compressor_xz_write;
-       a->compressor.finish = archive_compressor_xz_finish;
 
        /* Initialize compression library. */
        if (lzma_lzma_preset(&state->lzma_opt, config->compression_level)) {
@@ -287,71 +287,76 @@ archive_compressor_xz_finish(struct archive_write *a)
        struct private_data *state;
        unsigned tocopy;
 
+       ret = ARCHIVE_OK;
        state = (struct private_data *)a->compressor.data;
-       ret = 0;
-       if (a->client_writer == NULL) {
-               archive_set_error(&a->archive, ARCHIVE_ERRNO_PROGRAMMER,
-                   "No write callback is registered?  "
-                   "This is probably an internal programming error.");
-               ret = ARCHIVE_FATAL;
-               goto cleanup;
-       }
-
-       /* By default, always pad the uncompressed data. */
-       if (a->pad_uncompressed) {
-               tocopy = a->bytes_per_block -
-                   (state->total_in % a->bytes_per_block);
-               while (tocopy > 0 && tocopy < (unsigned)a->bytes_per_block) {
-                       state->stream.next_in = a->nulls;
-                       state->stream.avail_in = tocopy < a->null_length ?
-                           tocopy : a->null_length;
-                       state->total_in += state->stream.avail_in;
-                       tocopy -= state->stream.avail_in;
-                       ret = drive_compressor(a, state, 0);
-                       if (ret != ARCHIVE_OK)
-                               goto cleanup;
+       if (state != NULL) {
+               if (a->client_writer == NULL) {
+                       archive_set_error(&a->archive,
+                           ARCHIVE_ERRNO_PROGRAMMER,
+                           "No write callback is registered?  "
+                           "This is probably an internal programming error.");
+                       ret = ARCHIVE_FATAL;
+                       goto cleanup;
                }
-       }
 
-       /* Finish compression cycle */
-       if (((ret = drive_compressor(a, state, 1))) != ARCHIVE_OK)
-               goto cleanup;
+               /* By default, always pad the uncompressed data. */
+               if (a->pad_uncompressed) {
+                       tocopy = a->bytes_per_block -
+                           (state->total_in % a->bytes_per_block);
+                       while (tocopy > 0 && tocopy < (unsigned)a->bytes_per_block) {
+                               state->stream.next_in = a->nulls;
+                               state->stream.avail_in = tocopy < a->null_length ?
+                                   tocopy : a->null_length;
+                               state->total_in += state->stream.avail_in;
+                               tocopy -= state->stream.avail_in;
+                               ret = drive_compressor(a, state, 0);
+                               if (ret != ARCHIVE_OK)
+                                       goto cleanup;
+                       }
+               }
 
-       /* Optionally, pad the final compressed block. */
-       block_length = state->stream.next_out - state->compressed;
+               /* Finish compression cycle */
+               if (((ret = drive_compressor(a, state, 1))) != ARCHIVE_OK)
+                       goto cleanup;
+
+               /* Optionally, pad the final compressed block. */
+               block_length = state->stream.next_out - state->compressed;
+
+               /* Tricky calculation to determine size of last block. */
+               target_block_length = block_length;
+               if (a->bytes_in_last_block <= 0)
+                       /* Default or Zero: pad to full block */
+                       target_block_length = a->bytes_per_block;
+               else
+                       /* Round length to next multiple of bytes_in_last_block. */
+                       target_block_length = a->bytes_in_last_block *
+                           ( (block_length + a->bytes_in_last_block - 1) /
+                               a->bytes_in_last_block);
+               if (target_block_length > a->bytes_per_block)
+                       target_block_length = a->bytes_per_block;
+               if (block_length < target_block_length) {
+                       memset(state->stream.next_out, 0,
+                           target_block_length - block_length);
+                       block_length = target_block_length;
+               }
 
-       /* Tricky calculation to determine size of last block. */
-       target_block_length = block_length;
-       if (a->bytes_in_last_block <= 0)
-               /* Default or Zero: pad to full block */
-               target_block_length = a->bytes_per_block;
-       else
-               /* Round length to next multiple of bytes_in_last_block. */
-               target_block_length = a->bytes_in_last_block *
-                   ( (block_length + a->bytes_in_last_block - 1) /
-                       a->bytes_in_last_block);
-       if (target_block_length > a->bytes_per_block)
-               target_block_length = a->bytes_per_block;
-       if (block_length < target_block_length) {
-               memset(state->stream.next_out, 0,
-                   target_block_length - block_length);
-               block_length = target_block_length;
-       }
+               /* Write the last block */
+               bytes_written = (a->client_writer)(&a->archive, a->client_data,
+                   state->compressed, block_length);
+               if (bytes_written <= 0) {
+                       ret = ARCHIVE_FATAL;
+                       goto cleanup;
+               }
+               a->archive.raw_position += bytes_written;
 
-       /* Write the last block */
-       bytes_written = (a->client_writer)(&a->archive, a->client_data,
-           state->compressed, block_length);
-       if (bytes_written <= 0) {
-               ret = ARCHIVE_FATAL;
-               goto cleanup;
+               /* Cleanup: shut down compressor, release memory, etc. */
+       cleanup:
+               lzma_end(&(state->stream));
+               free(state->compressed);
+               free(state);
        }
-       a->archive.raw_position += bytes_written;
-
-       /* Cleanup: shut down compressor, release memory, etc. */
-cleanup:
-       lzma_end(&(state->stream));
-       free(state->compressed);
-       free(state);
+       free(a->compressor.config);
+       a->compressor.config = NULL;
        return (ret);
 }
 
index 809af5005aa610b22592cbb0f6ef7fb254cf4e4b..830b6a9a1a9b5687ebaf2298f135e6c2e87f596c 100644 (file)
@@ -127,6 +127,20 @@ static struct line {
        int critical;
 }  failed_lines[1000];
 
+/*
+ * Called at the beginning of each assert() function.
+ */
+static void
+count_assertion(const char *file, int line)
+{
+       (void)file; /* UNUSED */
+       (void)line; /* UNUSED */
+       ++assertions;
+       /* Uncomment to print file:line after every assertion.
+        * Verbose, but occasionally useful in tracking down crashes. */
+       /* printf("Checked %s:%d\n", file, line); */
+}
+
 /*
  * Count this failure; return the number of previous failures.
  */
@@ -262,7 +276,7 @@ failure(const char *fmt, ...)
 int
 test_assert(const char *file, int line, int value, const char *condition, void *extra)
 {
-       ++assertions;
+       count_assertion(file, line);
        if (value) {
                msg[0] = '\0';
                return (value);
@@ -281,7 +295,7 @@ int
 test_assert_equal_int(const char *file, int line,
     int v1, const char *e1, int v2, const char *e2, void *extra)
 {
-       ++assertions;
+       count_assertion(file, line);
        if (v1 == v2) {
                msg[0] = '\0';
                return (1);
@@ -328,7 +342,7 @@ test_assert_equal_string(const char *file, int line,
     const char *v2, const char *e2,
     void *extra)
 {
-       ++assertions;
+       count_assertion(file, line);
        if (v1 == NULL || v2 == NULL) {
                if (v1 == v2) {
                        msg[0] = '\0';
@@ -381,7 +395,7 @@ test_assert_equal_wstring(const char *file, int line,
     const wchar_t *v2, const char *e2,
     void *extra)
 {
-       ++assertions;
+       count_assertion(file, line);
        if (v1 == NULL) {
                if (v2 == NULL) {
                        msg[0] = '\0';
@@ -456,7 +470,7 @@ test_assert_equal_mem(const char *file, int line,
     const char *v2, const char *e2,
     size_t l, const char *ld, void *extra)
 {
-       ++assertions;
+       count_assertion(file, line);
        if (v1 == NULL || v2 == NULL) {
                if (v1 == v2) {
                        msg[0] = '\0';
@@ -723,6 +737,16 @@ struct { void (*func)(void); const char *name; } tests[] = {
        #include "list.h"
 };
 
+/*
+ * This is well-intentioned, but sometimes the standard libraries
+ * leave open file descriptors and expect to be able to come back to
+ * them (e.g., for username lookups or logging).  Closing these
+ * descriptors out from under those libraries creates havoc.
+ *
+ * Maybe there's some reasonably portable way to tell if a descriptor
+ * is open without using close()?
+ */
+#if 0
 static void
 close_descriptors(int warn)
 {
@@ -739,6 +763,7 @@ close_descriptors(int warn)
                report_failure(NULL);
        }
 }
+#endif
 
 /*
  * Each test is run in a private work dir.  Those work dirs
@@ -782,11 +807,12 @@ static int test_run(int i, const char *tmpdir)
        /* Explicitly reset the locale before each test. */
        setlocale(LC_ALL, "C");
        /* Make sure there are no stray descriptors going into the test. */
-       close_descriptors(0);
+       /* TODO: Find a better way to identify file descriptor leaks. */
+       //close_descriptors(0);
        /* Run the actual test. */
        (*tests[i].func)();
        /* Close stray descriptors, record as errors against this test. */
-       close_descriptors(1);
+       //close_descriptors(1);
        /* Summarize the results of this test. */
        summarize();
        /* If there were no failures, we can remove the work dir. */
index 3f07d08ec490a4959bc18b37e7634a2e100eb666..bc633eca1bbe1df13546bb3fb1798cd6f32002f7 100644 (file)
@@ -96,7 +96,7 @@ read_open_memory_internal(struct archive *a, void *buff,
                            memory_read, memory_read_skip, memory_read_close));
        else
                return (archive_read_open2(a, mine, NULL,
-                           memory_read, NULL, NULL));
+                           memory_read, NULL, memory_read_close));
 }
 
 /*
index 7d2abb9aa62421ebf86482e5be5e648caf2aede3..19881c2ece5922c669d6fb529477156a67a85e7f 100644 (file)
@@ -169,5 +169,6 @@ DEFINE_TEST(test_extattr_freebsd)
        assertEqualMem(xval, "12345", xsize);
        assertEqualIntA(a, ARCHIVE_OK, archive_read_close(a));
        assertEqualInt(ARCHIVE_OK, archive_read_finish(a));
+       archive_entry_free(ae);
 #endif
 }
index 256288f81ec0d5f716fa9cdce87af59dc3ec85c5..54c8a50965c1783f40b1092f38765434a5638158 100644 (file)
@@ -194,6 +194,32 @@ DEFINE_TEST(test_write_compress_bzip2)
        assert(0 == archive_read_close(a));
        assert(0 == archive_read_finish(a));
 
+       /*
+        * Test various premature shutdown scenarios to make sure we
+        * don't crash or leak memory.
+        */
+       assert((a = archive_write_new()) != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_compression_bzip2(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_finish(a));
+
+       assert((a = archive_write_new()) != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_compression_bzip2(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_close(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_finish(a));
+
+       assert((a = archive_write_new()) != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_format_ustar(a));
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_compression_bzip2(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_close(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_finish(a));
+
+       assert((a = archive_write_new()) != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_format_ustar(a));
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_compression_bzip2(a));
+       assertA(0 == archive_write_open_memory(a, buff, buffsize, &used2));
+       assertEqualInt(ARCHIVE_OK, archive_write_close(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_finish(a));
+
        /*
         * Clean up.
         */
index 4b9c84ca65d10231109de8375cd3d82dee516eff..9e42b137a04c160adb4b9849bf2f3236a33de7ae 100644 (file)
@@ -218,6 +218,32 @@ DEFINE_TEST(test_write_compress_gzip)
        }
        assertEqualInt(ARCHIVE_OK, archive_read_finish(a));
 
+       /*
+        * Test various premature shutdown scenarios to make sure we
+        * don't crash or leak memory.
+        */
+       assert((a = archive_write_new()) != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_compression_gzip(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_finish(a));
+
+       assert((a = archive_write_new()) != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_compression_gzip(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_close(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_finish(a));
+
+       assert((a = archive_write_new()) != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_format_ustar(a));
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_compression_gzip(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_close(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_finish(a));
+
+       assert((a = archive_write_new()) != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_format_ustar(a));
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_compression_gzip(a));
+       assertA(0 == archive_write_open_memory(a, buff, buffsize, &used2));
+       assertEqualInt(ARCHIVE_OK, archive_write_close(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_finish(a));
+
        /*
         * Clean up.
         */
index 4c3169ab2eb3d6dfafb76ae2f2c4e11e8ac83c81..639e58f6c1d43bc010e0090db9ab03f34bb1047e 100644 (file)
@@ -211,6 +211,32 @@ DEFINE_TEST(test_write_compress_lzma)
        }
        assertEqualInt(ARCHIVE_OK, archive_read_finish(a));
 
+       /*
+        * Test various premature shutdown scenarios to make sure we
+        * don't crash or leak memory.
+        */
+       assert((a = archive_write_new()) != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_compression_lzma(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_finish(a));
+
+       assert((a = archive_write_new()) != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_compression_lzma(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_close(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_finish(a));
+
+       assert((a = archive_write_new()) != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_format_ustar(a));
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_compression_lzma(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_close(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_finish(a));
+
+       assert((a = archive_write_new()) != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_format_ustar(a));
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_compression_lzma(a));
+       assertA(0 == archive_write_open_memory(a, buff, buffsize, &used2));
+       assertEqualInt(ARCHIVE_OK, archive_write_close(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_finish(a));
+
        /*
         * Clean up.
         */
index 60effec28581919250e0537384fdde2b63727a68..988fb6e879df2853f0aed467cd8cad1f713e1b22 100644 (file)
@@ -219,6 +219,32 @@ DEFINE_TEST(test_write_compress_xz)
        }
        assertEqualInt(ARCHIVE_OK, archive_read_finish(a));
 
+       /*
+        * Test various premature shutdown scenarios to make sure we
+        * don't crash or leak memory.
+        */
+       assert((a = archive_write_new()) != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_compression_xz(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_finish(a));
+
+       assert((a = archive_write_new()) != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_compression_xz(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_close(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_finish(a));
+
+       assert((a = archive_write_new()) != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_format_ustar(a));
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_compression_xz(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_close(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_finish(a));
+
+       assert((a = archive_write_new()) != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_format_ustar(a));
+       assertEqualIntA(a, ARCHIVE_OK, archive_write_set_compression_xz(a));
+       assertA(0 == archive_write_open_memory(a, buff, buffsize, &used2));
+       assertEqualInt(ARCHIVE_OK, archive_write_close(a));
+       assertEqualInt(ARCHIVE_OK, archive_write_finish(a));
+
        /*
         * Clean up.
         */
index 6179610854697d528b147c2343b459c2f54de9bb..c9c00d3d1e618d56e6dc323a036871bac8dc1b9c 100644 (file)
@@ -116,6 +116,7 @@ verify_write_data(struct archive *a, int sparse)
        /* XXX more XXX */
 
        assertEqualInt(0, close(fd));
+       archive_entry_free(ae);
        free(buff);
 }
 
@@ -249,6 +250,7 @@ verify_write_data_block(struct archive *a, int sparse)
 
        assertEqualInt(0, close(fd));
        free(buff);
+       archive_entry_free(ae);
 }
 
 DEFINE_TEST(test_write_disk_sparse)