From 752ede3058cc0a5410eafc497c8d5492fae2c2a9 Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Tue, 17 Jan 2006 04:49:04 +0000 Subject: [PATCH] If the attempt to open the archive fails (either the client open routine fails or the first read fails), invoke the client close routine immediately so the client can clean up. Also, don't store the client pointers in this case, so that the client close routine can't accidentally get called more than once. A minor style fix to archive_read_open_fd.c while I'm here. PR: 86453 Thanks to: Andrew Turner for reporting this and suggesting a fix. --- lib/libarchive/archive_read.c | 49 ++++++++++++++++++++------- lib/libarchive/archive_read_open_fd.c | 3 +- 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/lib/libarchive/archive_read.c b/lib/libarchive/archive_read.c index a7ed7335fa5..c6e47e1d845 100644 --- a/lib/libarchive/archive_read.c +++ b/lib/libarchive/archive_read.c @@ -107,8 +107,8 @@ archive_read_set_bytes_per_block(struct archive *a, int bytes_per_block) */ int archive_read_open(struct archive *a, void *client_data, - archive_open_callback *opener, archive_read_callback *reader, - archive_close_callback *closer) + archive_open_callback *client_opener, archive_read_callback *client_reader, + archive_close_callback *client_closer) { const void *buffer; ssize_t bytes_read; @@ -117,36 +117,59 @@ archive_read_open(struct archive *a, void *client_data, __archive_check_magic(a, ARCHIVE_READ_MAGIC, ARCHIVE_STATE_NEW, "archive_read_open"); - if (reader == NULL) + if (client_reader == NULL) __archive_errx(1, "No reader function provided to archive_read_open"); - a->client_reader = reader; - a->client_opener = opener; - a->client_closer = closer; - a->client_data = client_data; + /* + * Set these NULL initially. If the open or initial read fails, + * we'll leave them NULL to indicate that the file is invalid. + * (In particular, this helps ensure that the closer doesn't + * get called more than once.) + */ + a->client_opener = NULL; + a->client_reader = NULL; + a->client_closer = NULL; + a->client_data = NULL; /* Open data source. */ - if (a->client_opener != NULL) { - e =(a->client_opener)(a, a->client_data); - if (e != 0) + if (client_opener != NULL) { + e =(client_opener)(a, client_data); + if (e != 0) { + /* If the open failed, call the closer to clean up. */ + if (client_closer) + (client_closer)(a, client_data); return (e); + } } /* Read first block now for format detection. */ - bytes_read = (a->client_reader)(a, a->client_data, &buffer); + bytes_read = (client_reader)(a, client_data, &buffer); - /* client_reader should have already set error information. */ - if (bytes_read < 0) + if (bytes_read < 0) { + /* If the first read fails, close before returning error. */ + if (client_closer) + (client_closer)(a, client_data); + /* client_reader should have already set error information. */ return (ARCHIVE_FATAL); + } /* An empty archive is a serious error. */ if (bytes_read == 0) { archive_set_error(a, ARCHIVE_ERRNO_FILE_FORMAT, "Empty input file"); + /* Close the empty file. */ + if (client_closer) + (client_closer)(a, client_data); return (ARCHIVE_FATAL); } + /* Now that the client callbacks have worked, remember them. */ + a->client_opener = client_opener; /* Do we need to remember this? */ + a->client_reader = client_reader; + a->client_closer = client_closer; + a->client_data = client_data; + /* Select a decompression routine. */ high_bidder = choose_decompressor(a, buffer, bytes_read); if (high_bidder < 0) diff --git a/lib/libarchive/archive_read_open_fd.c b/lib/libarchive/archive_read_open_fd.c index c5d11950252..c5716e7902f 100644 --- a/lib/libarchive/archive_read_open_fd.c +++ b/lib/libarchive/archive_read_open_fd.c @@ -99,7 +99,8 @@ file_close(struct archive *a, void *client_data) struct read_fd_data *mine = client_data; (void)a; /* UNUSED */ - free(mine->buffer); + if (mine->buffer != NULL) + free(mine->buffer); free(mine); return (ARCHIVE_OK); }