From 4e8e93b6661f4de86c295b76fff82b8815266780 Mon Sep 17 00:00:00 2001 From: Nanik Tolaram Date: Sun, 8 Feb 2015 22:31:14 +1100 Subject: Remove dead/unused code and realign some of the comments to make it more cleaner and easier to read Change-Id: If536d482c0ed645368084e76d8ec060f05d89137 Signed-off-by: Nanik Tolaram --- minzip/DirUtil.c | 4 ++-- minzip/Hash.c | 11 ----------- minzip/Zip.c | 16 ++++++---------- 3 files changed, 8 insertions(+), 23 deletions(-) (limited to 'minzip') diff --git a/minzip/DirUtil.c b/minzip/DirUtil.c index fe2c880ac..97cb2e0ee 100644 --- a/minzip/DirUtil.c +++ b/minzip/DirUtil.c @@ -85,7 +85,7 @@ dirCreateHierarchy(const char *path, int mode, c--; } if (c == cpath) { -//xxx test this path + //xxx test this path /* No directory component. Act like the path was empty. */ errno = ENOENT; @@ -206,7 +206,7 @@ dirUnlinkHierarchy(const char *path) /* recurse over components */ errno = 0; while ((de = readdir(dir)) != NULL) { -//TODO: don't blow the stack + //TODO: don't blow the stack char dn[PATH_MAX]; if (!strcmp(de->d_name, "..") || !strcmp(de->d_name, ".")) { continue; diff --git a/minzip/Hash.c b/minzip/Hash.c index 8c6ca9bc2..8f8ed68e5 100644 --- a/minzip/Hash.c +++ b/minzip/Hash.c @@ -140,7 +140,6 @@ static bool resizeHash(HashTable* pHashTable, int newSize) int i; assert(countTombStones(pHashTable) == pHashTable->numDeadEntries); - //LOGI("before: dead=%d\n", pHashTable->numDeadEntries); pNewEntries = (HashEntry*) calloc(newSize, sizeof(HashTable)); if (pNewEntries == NULL) @@ -196,7 +195,6 @@ void* mzHashTableLookup(HashTable* pHashTable, unsigned int itemHash, void* item (*cmpFunc)(pEntry->data, item) == 0) { /* match */ - //LOGD("+++ match on entry %d\n", pEntry - pHashTable->pEntries); break; } @@ -206,8 +204,6 @@ void* mzHashTableLookup(HashTable* pHashTable, unsigned int itemHash, void* item break; /* edge case - single-entry table */ pEntry = pHashTable->pEntries; } - - //LOGI("+++ look probing %d...\n", pEntry - pHashTable->pEntries); } if (pEntry->data == NULL) { @@ -228,10 +224,6 @@ void* mzHashTableLookup(HashTable* pHashTable, unsigned int itemHash, void* item abort(); } /* note "pEntry" is now invalid */ - } else { - //LOGW("okay %d/%d/%d\n", - // pHashTable->numEntries, pHashTable->tableSize, - // (pHashTable->tableSize * LOAD_NUMER) / LOAD_DENOM); } /* full table is bad -- search for nonexistent never halts */ @@ -264,7 +256,6 @@ bool mzHashTableRemove(HashTable* pHashTable, unsigned int itemHash, void* item) pEnd = &pHashTable->pEntries[pHashTable->tableSize]; while (pEntry->data != NULL) { if (pEntry->data == item) { - //LOGI("+++ stepping on entry %d\n", pEntry - pHashTable->pEntries); pEntry->data = HASH_TOMBSTONE; pHashTable->numEntries--; pHashTable->numDeadEntries++; @@ -277,8 +268,6 @@ bool mzHashTableRemove(HashTable* pHashTable, unsigned int itemHash, void* item) break; /* edge case - single-entry table */ pEntry = pHashTable->pEntries; } - - //LOGI("+++ del probing %d...\n", pEntry - pHashTable->pEntries); } return false; diff --git a/minzip/Zip.c b/minzip/Zip.c index 5070104d3..aec35e375 100644 --- a/minzip/Zip.c +++ b/minzip/Zip.c @@ -327,10 +327,6 @@ static bool parseZipArchive(ZipArchive* pArchive) #else pEntry = &pArchive->pEntries[i]; #endif - - //LOGI("%d: localHdr=%d fnl=%d el=%d cl=%d\n", - // i, localHdrOffset, fileNameLen, extraLen, commentLen); - pEntry->fileNameLen = fileNameLen; pEntry->fileName = fileName; @@ -923,8 +919,8 @@ bool mzExtractRecursive(const ZipArchive *pArchive, /* Walk through the entries and extract anything whose path begins * with zpath. -//TODO: since the entries are sorted, binary search for the first match -// and stop after the first non-match. + //TODO: since the entries are sorted, binary search for the first match + // and stop after the first non-match. */ unsigned int i; bool seenMatch = false; @@ -933,10 +929,10 @@ bool mzExtractRecursive(const ZipArchive *pArchive, for (i = 0; i < pArchive->numEntries; i++) { ZipEntry *pEntry = pArchive->pEntries + i; if (pEntry->fileNameLen < zipDirLen) { -//TODO: look out for a single empty directory entry that matches zpath, but -// missing the trailing slash. Most zip files seem to include -// the trailing slash, but I think it's legal to leave it off. -// e.g., zpath "a/b/", entry "a/b", with no children of the entry. + //TODO: look out for a single empty directory entry that matches zpath, but + // missing the trailing slash. Most zip files seem to include + // the trailing slash, but I think it's legal to leave it off. + // e.g., zpath "a/b/", entry "a/b", with no children of the entry. /* No chance of matching. */ #if SORT_ENTRIES -- cgit v1.2.3 From 3e700cff53c0fdd4de2a0f89ef7916f05a131351 Mon Sep 17 00:00:00 2001 From: Narayan Kamath Date: Mon, 23 Feb 2015 13:29:16 +0000 Subject: Delete unused functions from minzip. This is in preparation of replacing it with libziparchive and providing shim wrappers. bug: 19472796 Change-Id: I1f2fb59ee7a41434e794e4ed15b754aa2b74a11d --- minzip/Zip.c | 43 +------------------------------------------ minzip/Zip.h | 51 --------------------------------------------------- 2 files changed, 1 insertion(+), 93 deletions(-) (limited to 'minzip') diff --git a/minzip/Zip.c b/minzip/Zip.c index aec35e375..fb462f45c 100644 --- a/minzip/Zip.c +++ b/minzip/Zip.c @@ -484,7 +484,7 @@ const ZipEntry* mzFindZipEntry(const ZipArchive* pArchive, /* * Return true if the entry is a symbolic link. */ -bool mzIsZipEntrySymlink(const ZipEntry* pEntry) +static bool mzIsZipEntrySymlink(const ZipEntry* pEntry) { if ((pEntry->versionMadeBy & 0xff00) == CENVEM_UNIX) { return S_ISLNK(pEntry->externalFileAttributes >> 16); @@ -628,30 +628,6 @@ static bool crcProcessFunction(const unsigned char *data, int dataLen, return true; } -/* - * Check the CRC on this entry; return true if it is correct. - * May do other internal checks as well. - */ -bool mzIsZipEntryIntact(const ZipArchive *pArchive, const ZipEntry *pEntry) -{ - unsigned long crc; - bool ret; - - crc = crc32(0L, Z_NULL, 0); - ret = mzProcessZipEntryContents(pArchive, pEntry, crcProcessFunction, - (void *)&crc); - if (!ret) { - LOGE("Can't calculate CRC for entry\n"); - return false; - } - if (crc != (unsigned long)pEntry->crc32) { - LOGW("CRC for entry %.*s (0x%08lx) != expected (0x%08lx)\n", - pEntry->fileNameLen, pEntry->fileName, crc, pEntry->crc32); - return false; - } - return true; -} - typedef struct { char *buf; int bufLen; @@ -733,23 +709,6 @@ bool mzExtractZipEntryToFile(const ZipArchive *pArchive, return true; } -/* - * Obtain a pointer to the in-memory representation of a stored entry. - */ -bool mzGetStoredEntry(const ZipArchive *pArchive, - const ZipEntry *pEntry, unsigned char **addr, size_t *length) -{ - if (pEntry->compression != STORED) { - LOGE("Can't getStoredEntry for '%s'; not stored\n", - pEntry->fileName); - return false; - } - - *addr = pArchive->addr + pEntry->offset; - *length = pEntry->uncompLen; - return true; -} - typedef struct { unsigned char* buffer; long len; diff --git a/minzip/Zip.h b/minzip/Zip.h index 2054b38a4..54eab3222 100644 --- a/minzip/Zip.h +++ b/minzip/Zip.h @@ -92,49 +92,15 @@ INLINE unsigned int mzZipEntryCount(const ZipArchive* pArchive) { return pArchive->numEntries; } -/* - * Get an entry by index. Returns NULL if the index is out-of-bounds. - */ -INLINE const ZipEntry* -mzGetZipEntryAt(const ZipArchive* pArchive, unsigned int index) -{ - if (index < pArchive->numEntries) { - return pArchive->pEntries + index; - } - return NULL; -} - -/* - * Get the index number of an entry in the archive. - */ -INLINE unsigned int -mzGetZipEntryIndex(const ZipArchive *pArchive, const ZipEntry *pEntry) { - return pEntry - pArchive->pEntries; -} - -/* - * Simple accessors. - */ -INLINE UnterminatedString mzGetZipEntryFileName(const ZipEntry* pEntry) { - UnterminatedString ret; - ret.str = pEntry->fileName; - ret.len = pEntry->fileNameLen; - return ret; -} INLINE long mzGetZipEntryOffset(const ZipEntry* pEntry) { return pEntry->offset; } INLINE long mzGetZipEntryUncompLen(const ZipEntry* pEntry) { return pEntry->uncompLen; } -INLINE long mzGetZipEntryModTime(const ZipEntry* pEntry) { - return pEntry->modTime; -} INLINE long mzGetZipEntryCrc32(const ZipEntry* pEntry) { return pEntry->crc32; } -bool mzIsZipEntrySymlink(const ZipEntry* pEntry); - /* * Type definition for the callback function used by @@ -163,12 +129,6 @@ bool mzProcessZipEntryContents(const ZipArchive *pArchive, bool mzReadZipEntry(const ZipArchive* pArchive, const ZipEntry* pEntry, char* buf, int bufLen); -/* - * Check the CRC on this entry; return true if it is correct. - * May do other internal checks as well. - */ -bool mzIsZipEntryIntact(const ZipArchive *pArchive, const ZipEntry *pEntry); - /* * Inflate and write an entry to a file. */ @@ -182,17 +142,6 @@ bool mzExtractZipEntryToFile(const ZipArchive *pArchive, bool mzExtractZipEntryToBuffer(const ZipArchive *pArchive, const ZipEntry *pEntry, unsigned char* buffer); -/* - * Return a pointer and length for a given entry. The returned region - * should be valid until pArchive is closed, and should be treated as - * read-only. - * - * Only makes sense for entries which are stored (ie, not compressed). - * No guarantees are made regarding alignment of the returned pointer. - */ -bool mzGetStoredEntry(const ZipArchive *pArchive, - const ZipEntry* pEntry, unsigned char **addr, size_t *length); - /* * Inflate all entries under zipDir to the directory specified by * targetDir, which must exist and be a writable directory. -- cgit v1.2.3 From 9c0f5d6b348e37533bdcccf1166d6cbf1ca5c50b Mon Sep 17 00:00:00 2001 From: Narayan Kamath Date: Mon, 23 Feb 2015 14:09:31 +0000 Subject: Remove more dead code from minzip. I've added explanatory comments to mzExtractRecursive because that function will live on as a utility even after we move the zip format related logic to libziparchive. bug: 19472796 (cherry-picked from commit c9ccdfd7a42de08c47ab771b94dc5b9d1f957b95) Change-Id: I8b7fb6fa3eafb2e7ac080ef7a7eceb691b252d8a --- minzip/Zip.c | 164 +++++++++++++++++++++-------------------------------------- minzip/Zip.h | 13 ++--- 2 files changed, 64 insertions(+), 113 deletions(-) (limited to 'minzip') diff --git a/minzip/Zip.c b/minzip/Zip.c index 12e06f6d8..d3ff79be6 100644 --- a/minzip/Zip.c +++ b/minzip/Zip.c @@ -828,7 +828,7 @@ static const char *targetEntryPath(MzPathHelper *helper, ZipEntry *pEntry) */ bool mzExtractRecursive(const ZipArchive *pArchive, const char *zipDir, const char *targetDir, - int flags, const struct utimbuf *timestamp, + const struct utimbuf *timestamp, void (*callback)(const char *fn, void *), void *cookie, struct selabel_handle *sehnd) { @@ -932,30 +932,19 @@ bool mzExtractRecursive(const ZipArchive *pArchive, break; } - /* With DRY_RUN set, invoke the callback but don't do anything else. - */ - if (flags & MZ_EXTRACT_DRY_RUN) { - if (callback != NULL) callback(targetFile, cookie); - continue; - } - - /* Create the file or directory. - */ #define UNZIP_DIRMODE 0755 #define UNZIP_FILEMODE 0644 - if (pEntry->fileName[pEntry->fileNameLen-1] == '/') { - if (!(flags & MZ_EXTRACT_FILES_ONLY)) { - int ret = dirCreateHierarchy( - targetFile, UNZIP_DIRMODE, timestamp, false, sehnd); - if (ret != 0) { - LOGE("Can't create containing directory for \"%s\": %s\n", - targetFile, strerror(errno)); - ok = false; - break; - } - LOGD("Extracted dir \"%s\"\n", targetFile); - } - } else { + /* + * Create the file or directory. We ignore directory entries + * because we recursively create paths to each file entry we encounter + * in the zip archive anyway. + * + * NOTE: A "directory entry" in a zip archive is just a zero length + * entry that ends in a "/". They're not mandatory and many tools get + * rid of them. We need to process them only if we want to preserve + * empty directories from the archive. + */ + if (pEntry->fileName[pEntry->fileNameLen-1] != '/') { /* This is not a directory. First, make sure that * the containing directory exists. */ @@ -968,97 +957,62 @@ bool mzExtractRecursive(const ZipArchive *pArchive, break; } - /* With FILES_ONLY set, we need to ignore metadata entirely, - * so treat symlinks as regular files. + /* + * The entry is a regular file or a symlink. Open the target for writing. + * + * TODO: This behavior for symlinks seems rather bizarre. For a + * symlink foo/bar/baz -> foo/tar/taz, we will create a file called + * "foo/bar/baz" whose contents are the literal "foo/tar/taz". We + * warn about this for now and preserve older behavior. */ - if (!(flags & MZ_EXTRACT_FILES_ONLY) && mzIsZipEntrySymlink(pEntry)) { - /* The entry is a symbolic link. - * The relative target of the symlink is in the - * data section of this entry. - */ - if (pEntry->uncompLen == 0) { - LOGE("Symlink entry \"%s\" has no target\n", - targetFile); - ok = false; - break; - } - char *linkTarget = malloc(pEntry->uncompLen + 1); - if (linkTarget == NULL) { - ok = false; - break; - } - ok = mzReadZipEntry(pArchive, pEntry, linkTarget, - pEntry->uncompLen); - if (!ok) { - LOGE("Can't read symlink target for \"%s\"\n", - targetFile); - free(linkTarget); - break; - } - linkTarget[pEntry->uncompLen] = '\0'; - - /* Make the link. - */ - ret = symlink(linkTarget, targetFile); - if (ret != 0) { - LOGE("Can't symlink \"%s\" to \"%s\": %s\n", - targetFile, linkTarget, strerror(errno)); - free(linkTarget); - ok = false; - break; - } - LOGD("Extracted symlink \"%s\" -> \"%s\"\n", - targetFile, linkTarget); - free(linkTarget); - } else { - /* The entry is a regular file. - * Open the target for writing. - */ - - char *secontext = NULL; + if (mzIsZipEntrySymlink(pEntry)) { + LOGE("Symlink entry \"%.*s\" will be output as a regular file.", + pEntry->fileNameLen, pEntry->fileName); + } - if (sehnd) { - selabel_lookup(sehnd, &secontext, targetFile, UNZIP_FILEMODE); - setfscreatecon(secontext); - } + char *secontext = NULL; - int fd = open(targetFile, O_CREAT|O_WRONLY|O_TRUNC|O_SYNC - , UNZIP_FILEMODE); + if (sehnd) { + selabel_lookup(sehnd, &secontext, targetFile, UNZIP_FILEMODE); + setfscreatecon(secontext); + } - if (secontext) { - freecon(secontext); - setfscreatecon(NULL); - } + int fd = open(targetFile, O_CREAT|O_WRONLY|O_TRUNC|O_SYNC, + UNZIP_FILEMODE); - if (fd < 0) { - LOGE("Can't create target file \"%s\": %s\n", - targetFile, strerror(errno)); - ok = false; - break; - } + if (secontext) { + freecon(secontext); + setfscreatecon(NULL); + } - bool ok = mzExtractZipEntryToFile(pArchive, pEntry, fd); - if (ok) { - ok = (fsync(fd) == 0); - } - if (close(fd) != 0) { - ok = false; - } - if (!ok) { - LOGE("Error extracting \"%s\"\n", targetFile); - ok = false; - break; - } + if (fd < 0) { + LOGE("Can't create target file \"%s\": %s\n", + targetFile, strerror(errno)); + ok = false; + break; + } - if (timestamp != NULL && utime(targetFile, timestamp)) { - LOGE("Error touching \"%s\"\n", targetFile); - ok = false; - break; - } + bool ok = mzExtractZipEntryToFile(pArchive, pEntry, fd); + if (ok) { + ok = (fsync(fd) == 0); + } + if (close(fd) != 0) { + ok = false; + } + if (!ok) { + LOGE("Error extracting \"%s\"\n", targetFile); + ok = false; + break; + } - LOGV("Extracted file \"%s\"\n", targetFile); - ++extractCount; + if (timestamp != NULL && utime(targetFile, timestamp)) { + LOGE("Error touching \"%s\"\n", targetFile); + ok = false; + break; } + + LOGV("Extracted file \"%s\"\n", targetFile); + ++extractCount; } if (callback != NULL) callback(targetFile, cookie); diff --git a/minzip/Zip.h b/minzip/Zip.h index 54eab3222..a2b2c26fc 100644 --- a/minzip/Zip.h +++ b/minzip/Zip.h @@ -143,9 +143,12 @@ bool mzExtractZipEntryToBuffer(const ZipArchive *pArchive, const ZipEntry *pEntry, unsigned char* buffer); /* - * Inflate all entries under zipDir to the directory specified by + * Inflate all files under zipDir to the directory specified by * targetDir, which must exist and be a writable directory. * + * Directory entries and symlinks are not extracted. + * + * * The immediate children of zipDir will become the immediate * children of targetDir; e.g., if the archive contains the entries * @@ -160,21 +163,15 @@ bool mzExtractZipEntryToBuffer(const ZipArchive *pArchive, * /tmp/two * /tmp/d/three * - * flags is zero or more of the following: - * - * MZ_EXTRACT_FILES_ONLY - only unpack files, not directories or symlinks - * MZ_EXTRACT_DRY_RUN - don't do anything, but do invoke the callback - * * If timestamp is non-NULL, file timestamps will be set accordingly. * * If callback is non-NULL, it will be invoked with each unpacked file. * * Returns true on success, false on failure. */ -enum { MZ_EXTRACT_FILES_ONLY = 1, MZ_EXTRACT_DRY_RUN = 2 }; bool mzExtractRecursive(const ZipArchive *pArchive, const char *zipDir, const char *targetDir, - int flags, const struct utimbuf *timestamp, + const struct utimbuf *timestamp, void (*callback)(const char *fn, void*), void *cookie, struct selabel_handle *sehnd); -- cgit v1.2.3 From 9ad9d66f818fc9a86eacaac988b1ad72c6b27696 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Wed, 8 Apr 2015 12:08:32 -0700 Subject: Remove a couple of unused inlines from minzip/Zip.h. Change-Id: I805883e3863673416898bdef39c5703ca33f18e0 --- minzip/Zip.h | 10 ---------- 1 file changed, 10 deletions(-) (limited to 'minzip') diff --git a/minzip/Zip.h b/minzip/Zip.h index a2b2c26fc..86d8db597 100644 --- a/minzip/Zip.h +++ b/minzip/Zip.h @@ -85,22 +85,12 @@ void mzCloseZipArchive(ZipArchive* pArchive); const ZipEntry* mzFindZipEntry(const ZipArchive* pArchive, const char* entryName); -/* - * Get the number of entries in the Zip archive. - */ -INLINE unsigned int mzZipEntryCount(const ZipArchive* pArchive) { - return pArchive->numEntries; -} - INLINE long mzGetZipEntryOffset(const ZipEntry* pEntry) { return pEntry->offset; } INLINE long mzGetZipEntryUncompLen(const ZipEntry* pEntry) { return pEntry->uncompLen; } -INLINE long mzGetZipEntryCrc32(const ZipEntry* pEntry) { - return pEntry->crc32; -} /* * Type definition for the callback function used by -- cgit v1.2.3 From 2f5feedf1d705b53e5bf90c8b5207dd91f4522f1 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Tue, 28 Apr 2015 17:24:24 -0700 Subject: Check all lseek calls succeed. Also add missing TEMP_FAILURE_RETRYs on read, write, and lseek. Bug: http://b/20625546 Change-Id: I03b198e11c1921b35518ee2dd005a7cfcf4fd94b (cherry picked from commit 7bad7c4646ee8fd8d6e6ed0ffd3ddbb0c1b41a2f) --- minzip/SysUtil.c | 10 ++++++---- minzip/Zip.c | 6 ++---- 2 files changed, 8 insertions(+), 8 deletions(-) (limited to 'minzip') diff --git a/minzip/SysUtil.c b/minzip/SysUtil.c index ac6f5c33f..b160c9e3d 100644 --- a/minzip/SysUtil.c +++ b/minzip/SysUtil.c @@ -27,11 +27,13 @@ static int getFileStartAndLength(int fd, off_t *start_, size_t *length_) assert(start_ != NULL); assert(length_ != NULL); - start = lseek(fd, 0L, SEEK_CUR); - end = lseek(fd, 0L, SEEK_END); - (void) lseek(fd, start, SEEK_SET); + // TODO: isn't start always 0 for the single call site? just use fstat instead? - if (start == (off_t) -1 || end == (off_t) -1) { + start = TEMP_FAILURE_RETRY(lseek(fd, 0L, SEEK_CUR)); + end = TEMP_FAILURE_RETRY(lseek(fd, 0L, SEEK_END)); + + if (TEMP_FAILURE_RETRY(lseek(fd, start, SEEK_SET)) == -1 || + start == (off_t) -1 || end == (off_t) -1) { LOGE("could not determine length of file\n"); return -1; } diff --git a/minzip/Zip.c b/minzip/Zip.c index d3ff79be6..40712e03a 100644 --- a/minzip/Zip.c +++ b/minzip/Zip.c @@ -675,13 +675,11 @@ static bool writeProcessFunction(const unsigned char *data, int dataLen, } ssize_t soFar = 0; while (true) { - ssize_t n = write(fd, data+soFar, dataLen-soFar); + ssize_t n = TEMP_FAILURE_RETRY(write(fd, data+soFar, dataLen-soFar)); if (n <= 0) { LOGE("Error writing %zd bytes from zip file from %p: %s\n", dataLen-soFar, data+soFar, strerror(errno)); - if (errno != EINTR) { - return false; - } + return false; } else if (n > 0) { soFar += n; if (soFar == dataLen) return true; -- cgit v1.2.3