diff --git a/libziparchive/zip_archive.cc b/libziparchive/zip_archive.cc index 128bad440..6ec8f0d34 100644 --- a/libziparchive/zip_archive.cc +++ b/libziparchive/zip_archive.cc @@ -287,7 +287,7 @@ static const char kTempMappingFileName[] = "zip: ExtractFileToFile"; */ struct ZipArchive { /* open Zip archive */ - int fd; + const int fd; /* mapped central directory area */ off64_t directory_offset; @@ -304,6 +304,25 @@ struct ZipArchive { */ uint32_t hash_table_size; ZipEntryName* hash_table; + + ZipArchive(const int fd) : + fd(fd), + directory_offset(0), + directory_map(NULL), + num_entries(0), + hash_table_size(0), + hash_table(NULL) {} + + ~ZipArchive() { + if (fd >= 0) { + close(fd); + } + + if (directory_map != NULL) { + directory_map->release(); + } + free(hash_table); + } }; // Returns 0 on success and negative values on failure. @@ -661,28 +680,20 @@ static int32_t OpenArchiveInternal(ZipArchive* archive, int32_t OpenArchiveFd(int fd, const char* debug_file_name, ZipArchiveHandle* handle) { - ZipArchive* archive = (ZipArchive*) malloc(sizeof(ZipArchive)); - memset(archive, 0, sizeof(*archive)); + ZipArchive* archive = new ZipArchive(fd); *handle = archive; - - archive->fd = fd; - return OpenArchiveInternal(archive, debug_file_name); } int32_t OpenArchive(const char* fileName, ZipArchiveHandle* handle) { - ZipArchive* archive = (ZipArchive*) malloc(sizeof(ZipArchive)); - memset(archive, 0, sizeof(*archive)); + const int fd = open(fileName, O_RDONLY | O_BINARY, 0); + ZipArchive* archive = new ZipArchive(fd); *handle = archive; - const int fd = open(fileName, O_RDONLY | O_BINARY, 0); if (fd < 0) { ALOGW("Unable to open '%s': %s", fileName, strerror(errno)); return kIoError; - } else { - archive->fd = fd; } - return OpenArchiveInternal(archive, fileName); } @@ -692,16 +703,7 @@ int32_t OpenArchive(const char* fileName, ZipArchiveHandle* handle) { void CloseArchive(ZipArchiveHandle handle) { ZipArchive* archive = (ZipArchive*) handle; ALOGV("Closing archive %p", archive); - - if (archive->fd >= 0) { - close(archive->fd); - } - - if (archive->directory_map != NULL) { - archive->directory_map->release(); - } - free(archive->hash_table); - free(archive); + delete archive; } static int32_t UpdateEntryFromDataDescriptor(int fd, diff --git a/libziparchive/zip_archive_test.cc b/libziparchive/zip_archive_test.cc index 875b6dec7..813a87f2a 100644 --- a/libziparchive/zip_archive_test.cc +++ b/libziparchive/zip_archive_test.cc @@ -26,6 +26,7 @@ static std::string test_data_dir; +static const std::string kMissingZip = "missing.zip"; static const std::string kValidZip = "valid.zip"; static const uint8_t kATxtContents[] = { @@ -58,6 +59,14 @@ TEST(ziparchive, Open) { CloseArchive(handle); } +TEST(ziparchive, OpenMissing) { + ZipArchiveHandle handle; + ASSERT_NE(0, OpenArchiveWrapper(kMissingZip, &handle)); + + // Confirm the file descriptor is not going to be mistaken for a valid one. + ASSERT_EQ(-1, GetFileDescriptor(handle)); +} + TEST(ziparchive, Iteration) { ZipArchiveHandle handle; ASSERT_EQ(0, OpenArchiveWrapper(kValidZip, &handle));