From: Tim Kientzle Date: Fri, 10 Apr 2009 05:50:58 +0000 (-0400) Subject: Merge r944 from libarchive/trunk: Fix X-Git-Tag: v2.7.0~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6e75735eb654bb8fabffa0e486f85ab1cf6fe67b;p=thirdparty%2Flibarchive.git Merge r944 from libarchive/trunk: Fix a number of minor memory leaks. SVN-Revision: 946 --- diff --git a/libarchive/archive_entry.c b/libarchive/archive_entry.c index 77c81a137..ffbd92f6e 100644 --- a/libarchive/archive_entry.c +++ b/libarchive/archive_entry.c @@ -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); diff --git a/libarchive/archive_read_open_fd.c b/libarchive/archive_read_open_fd.c index 3e348f7b1..c7f9d693b 100644 --- a/libarchive/archive_read_open_fd.c +++ b/libarchive/archive_read_open_fd.c @@ -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 diff --git a/libarchive/archive_read_open_filename.c b/libarchive/archive_read_open_filename.c index e813a710d..04efb0f5a 100644 --- a/libarchive/archive_read_open_filename.c +++ b/libarchive/archive_read_open_filename.c @@ -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 diff --git a/libarchive/archive_read_support_format_mtree.c b/libarchive/archive_read_support_format_mtree.c index 447f92268..0f0e116e0 100644 --- a/libarchive/archive_read_support_format_mtree.c +++ b/libarchive/archive_read_support_format_mtree.c @@ -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); } /* diff --git a/libarchive/archive_write_disk.c b/libarchive/archive_write_disk.c index 0d65a3b97..54fab828a 100644 --- a/libarchive/archive_write_disk.c +++ b/libarchive/archive_write_disk.c @@ -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); diff --git a/libarchive/archive_write_set_compression_bzip2.c b/libarchive/archive_write_set_compression_bzip2.c index d6d82352f..17f6a0a4d 100644 --- a/libarchive/archive_write_set_compression_bzip2.c +++ b/libarchive/archive_write_set_compression_bzip2.c @@ -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); } diff --git a/libarchive/archive_write_set_compression_gzip.c b/libarchive/archive_write_set_compression_gzip.c index 35303fc02..a8b1c3336 100644 --- a/libarchive/archive_write_set_compression_gzip.c +++ b/libarchive/archive_write_set_compression_gzip.c @@ -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); } diff --git a/libarchive/archive_write_set_compression_xz.c b/libarchive/archive_write_set_compression_xz.c index 96c0577d5..839625252 100644 --- a/libarchive/archive_write_set_compression_xz.c +++ b/libarchive/archive_write_set_compression_xz.c @@ -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); } diff --git a/libarchive/test/main.c b/libarchive/test/main.c index 809af5005..830b6a9a1 100644 --- a/libarchive/test/main.c +++ b/libarchive/test/main.c @@ -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. */ diff --git a/libarchive/test/read_open_memory.c b/libarchive/test/read_open_memory.c index 3f07d08ec..bc633eca1 100644 --- a/libarchive/test/read_open_memory.c +++ b/libarchive/test/read_open_memory.c @@ -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)); } /* diff --git a/libarchive/test/test_extattr_freebsd.c b/libarchive/test/test_extattr_freebsd.c index 7d2abb9aa..19881c2ec 100644 --- a/libarchive/test/test_extattr_freebsd.c +++ b/libarchive/test/test_extattr_freebsd.c @@ -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 } diff --git a/libarchive/test/test_write_compress_bzip2.c b/libarchive/test/test_write_compress_bzip2.c index 256288f81..54c8a5096 100644 --- a/libarchive/test/test_write_compress_bzip2.c +++ b/libarchive/test/test_write_compress_bzip2.c @@ -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. */ diff --git a/libarchive/test/test_write_compress_gzip.c b/libarchive/test/test_write_compress_gzip.c index 4b9c84ca6..9e42b137a 100644 --- a/libarchive/test/test_write_compress_gzip.c +++ b/libarchive/test/test_write_compress_gzip.c @@ -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. */ diff --git a/libarchive/test/test_write_compress_lzma.c b/libarchive/test/test_write_compress_lzma.c index 4c3169ab2..639e58f6c 100644 --- a/libarchive/test/test_write_compress_lzma.c +++ b/libarchive/test/test_write_compress_lzma.c @@ -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. */ diff --git a/libarchive/test/test_write_compress_xz.c b/libarchive/test/test_write_compress_xz.c index 60effec28..988fb6e87 100644 --- a/libarchive/test/test_write_compress_xz.c +++ b/libarchive/test/test_write_compress_xz.c @@ -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. */ diff --git a/libarchive/test/test_write_disk_sparse.c b/libarchive/test/test_write_disk_sparse.c index 617961085..c9c00d3d1 100644 --- a/libarchive/test/test_write_disk_sparse.c +++ b/libarchive/test/test_write_disk_sparse.c @@ -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)