From 596271fa71d79e3eec03c7cf6ac76cb026dd8578 Mon Sep 17 00:00:00 2001 From: Doug Zongker Date: Wed, 29 Apr 2009 16:52:04 -0700 Subject: handle short writes when unzipping files minzip fails if write() doesn't write all the data in one call. Apparently this was good enough before, but it causes OTAs to fail all the time now (maybe due to the recently-submitted kernel)? Change code to attempt continuing after short writes. --- minzip/Zip.c | 38 +++++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 11 deletions(-) (limited to 'minzip') diff --git a/minzip/Zip.c b/minzip/Zip.c index 100c833fe..ead899390 100644 --- a/minzip/Zip.c +++ b/minzip/Zip.c @@ -41,7 +41,7 @@ enum { CENSIZ = 20, CENLEN = 24, CENNAM = 28, - CENEXT = 30, + CENEXT = 30, CENCOM = 32, CENDSK = 34, CENATT = 36, @@ -66,13 +66,13 @@ enum { LOCSIG = 0x04034b50, // PK34 LOCHDR = 30, - + LOCVER = 4, LOCFLG = 6, LOCHOW = 8, LOCTIM = 10, LOCCRC = 14, - LOCSIZ = 18, + LOCSIZ = 18, LOCLEN = 22, LOCNAM = 26, LOCEXT = 28, @@ -757,7 +757,7 @@ bool mzReadZipEntry(const ZipArchive* pArchive, const ZipEntry* pEntry, { CopyProcessArgs args; bool ret; - + args.buf = buf; args.bufLen = bufLen; ret = mzProcessZipEntryContents(pArchive, pEntry, copyProcessFunction, @@ -772,13 +772,29 @@ bool mzReadZipEntry(const ZipArchive* pArchive, const ZipEntry* pEntry, static bool writeProcessFunction(const unsigned char *data, int dataLen, void *fd) { - ssize_t n = write((int)fd, data, dataLen); - if (n != dataLen) { - LOGE("Can't write %d bytes (only %ld) from zip file: %s\n", - dataLen, n, strerror(errno)); - return false; - } - return true; + int zeroWrites = 0; + ssize_t soFar = 0; + do { + ssize_t n = write((int)fd, data+soFar, dataLen-soFar); + if (n < 0) { + LOGE("Error writing %ld bytes from zip file: %s\n", + dataLen-soFar, strerror(errno)); + return false; + } else if (n > 0) { + soFar += n; + if (soFar == dataLen) return true; + if (soFar > dataLen) { + LOGE("write overrun? (%ld bytes instead of %d)\n", + soFar, dataLen); + return false; + } + zeroWrites = 0; + } else { + ++zeroWrites; + } + } while (zeroWrites < 5); + LOGE("too many consecutive zero-length writes\n"); + return false; } /* -- cgit v1.2.3 From 683c4628039a8cb6dad1a086fae23a7d71438414 Mon Sep 17 00:00:00 2001 From: Doug Zongker Date: Tue, 5 May 2009 17:50:21 -0700 Subject: align data passed to write() on 32k boundaries In donut, OTA installation often encounters the write() system call doing short writes -- which is legal but unexpected -- or failing with ENOSPC when plenty of space is available. Passing aligned memory buffers to write() appears to prevent (or at least reduce the frequency) of these problems. b/1833052 has been filed to look at the underlying problem, but this change aligns buffers we use with write() so we can OTA for now (or see if this problem still occurs). --- minzip/Zip.c | 62 ++++++++++++++++++++++++++++++++++++++++++++---------------- minzip/Zip.h | 10 +++++++++- 2 files changed, 55 insertions(+), 17 deletions(-) (limited to 'minzip') diff --git a/minzip/Zip.c b/minzip/Zip.c index ead899390..a601e7453 100644 --- a/minzip/Zip.c +++ b/minzip/Zip.c @@ -12,6 +12,7 @@ #include // for uintptr_t #include #include // for S_ISLNK() +#include #include #define LOG_TAG "minzip" @@ -84,6 +85,12 @@ enum { }; +/* The maximum zipped file write size we will align. */ +#define WRITE_SIZE 32768 +/* The boundary on which we will align it. */ +#define WRITE_ALIGNMENT 32768 + + /* * For debugging, dump the contents of a ZipEntry. */ @@ -770,17 +777,38 @@ bool mzReadZipEntry(const ZipArchive* pArchive, const ZipEntry* pEntry, } static bool writeProcessFunction(const unsigned char *data, int dataLen, - void *fd) + void *cookie) { - int zeroWrites = 0; + WriteInfo *wi = (WriteInfo*)cookie; + + if (dataLen <= WRITE_SIZE) { + memcpy(wi->aligned_buffer, data, dataLen); + data = wi->aligned_buffer; + } + ssize_t soFar = 0; - do { - ssize_t n = write((int)fd, data+soFar, dataLen-soFar); - if (n < 0) { - LOGE("Error writing %ld bytes from zip file: %s\n", - dataLen-soFar, strerror(errno)); + while (true) { + ssize_t n = write(wi->fd, data+soFar, dataLen-soFar); + if (n <= 0) { + LOGE("Error writing %ld bytes from zip file from %p: %s\n", + dataLen-soFar, data+soFar, strerror(errno)); + if (errno == ENOSPC) { + struct statfs sf; + if (statfs("/system", &sf) != 0) { + LOGE("failed to statfs /system: %s\n", strerror(errno)); + } else { + LOGE("statfs said: %ld * %ld = %ld\n", + (long)sf.f_bsize, (long)sf.f_bfree, + (long)sf.f_bsize * (long)sf.f_bfree); + } + } return false; } else if (n > 0) { + if (n < dataLen-soFar) { + LOGE("short write: %d bytes of %d from %p\n", + (int)n, (int)(dataLen-soFar), + data+soFar); + } soFar += n; if (soFar == dataLen) return true; if (soFar > dataLen) { @@ -788,23 +816,18 @@ static bool writeProcessFunction(const unsigned char *data, int dataLen, soFar, dataLen); return false; } - zeroWrites = 0; - } else { - ++zeroWrites; } - } while (zeroWrites < 5); - LOGE("too many consecutive zero-length writes\n"); - return false; + } } /* * Uncompress "pEntry" in "pArchive" to "fd" at the current offset. */ bool mzExtractZipEntryToFile(const ZipArchive *pArchive, - const ZipEntry *pEntry, int fd) + const ZipEntry *pEntry, WriteInfo *wi) { bool ret = mzProcessZipEntryContents(pArchive, pEntry, writeProcessFunction, - (void *)fd); + wi); if (!ret) { LOGE("Can't extract entry to file.\n"); return false; @@ -906,6 +929,11 @@ bool mzExtractRecursive(const ZipArchive *pArchive, return false; } + unsigned char* buffer = malloc(WRITE_SIZE+WRITE_ALIGNMENT); + WriteInfo wi; + wi.aligned_buffer = buffer + WRITE_ALIGNMENT - + ((long)buffer % WRITE_ALIGNMENT); + unsigned int zipDirLen; char *zpath; @@ -1086,7 +1114,8 @@ bool mzExtractRecursive(const ZipArchive *pArchive, break; } - bool ok = mzExtractZipEntryToFile(pArchive, pEntry, fd); + wi.fd = fd; + bool ok = mzExtractZipEntryToFile(pArchive, pEntry, &wi); close(fd); if (!ok) { LOGE("Error extracting \"%s\"\n", targetFile); @@ -1109,6 +1138,7 @@ bool mzExtractRecursive(const ZipArchive *pArchive, free(helper.buf); free(zpath); + free(buffer); return ok; } diff --git a/minzip/Zip.h b/minzip/Zip.h index 1c1df2fae..57c0abd2c 100644 --- a/minzip/Zip.h +++ b/minzip/Zip.h @@ -55,6 +55,14 @@ typedef struct { size_t len; } UnterminatedString; +/* + * The information we pass down to writeProcessFunction. + */ +typedef struct { + int fd; + unsigned char* aligned_buffer; +} WriteInfo; + /* * Open a Zip archive. * @@ -166,7 +174,7 @@ bool mzIsZipEntryIntact(const ZipArchive *pArchive, const ZipEntry *pEntry); * Inflate and write an entry to a file. */ bool mzExtractZipEntryToFile(const ZipArchive *pArchive, - const ZipEntry *pEntry, int fd); + const ZipEntry *pEntry, WriteInfo *wi); /* * Inflate all entries under zipDir to the directory specified by -- cgit v1.2.3 From 1c4ceae38f3fd7eb1e451d430acb5d99f257b0f9 Mon Sep 17 00:00:00 2001 From: Doug Zongker Date: Fri, 8 May 2009 09:43:28 -0700 Subject: undo temporary alignment hack Remove the memory alignment that mysteriously made OTA installs work, in anticipation of a kernel that fixes the actual problem. Handle EINTR properly. --- minzip/Zip.c | 46 +++++++--------------------------------------- minzip/Zip.h | 10 +--------- 2 files changed, 8 insertions(+), 48 deletions(-) (limited to 'minzip') diff --git a/minzip/Zip.c b/minzip/Zip.c index a601e7453..8cdb89874 100644 --- a/minzip/Zip.c +++ b/minzip/Zip.c @@ -12,7 +12,6 @@ #include // for uintptr_t #include #include // for S_ISLNK() -#include #include #define LOG_TAG "minzip" @@ -85,12 +84,6 @@ enum { }; -/* The maximum zipped file write size we will align. */ -#define WRITE_SIZE 32768 -/* The boundary on which we will align it. */ -#define WRITE_ALIGNMENT 32768 - - /* * For debugging, dump the contents of a ZipEntry. */ @@ -779,36 +772,18 @@ bool mzReadZipEntry(const ZipArchive* pArchive, const ZipEntry* pEntry, static bool writeProcessFunction(const unsigned char *data, int dataLen, void *cookie) { - WriteInfo *wi = (WriteInfo*)cookie; - - if (dataLen <= WRITE_SIZE) { - memcpy(wi->aligned_buffer, data, dataLen); - data = wi->aligned_buffer; - } + int fd = (int)cookie; ssize_t soFar = 0; while (true) { - ssize_t n = write(wi->fd, data+soFar, dataLen-soFar); + ssize_t n = write(fd, data+soFar, dataLen-soFar); if (n <= 0) { LOGE("Error writing %ld bytes from zip file from %p: %s\n", dataLen-soFar, data+soFar, strerror(errno)); - if (errno == ENOSPC) { - struct statfs sf; - if (statfs("/system", &sf) != 0) { - LOGE("failed to statfs /system: %s\n", strerror(errno)); - } else { - LOGE("statfs said: %ld * %ld = %ld\n", - (long)sf.f_bsize, (long)sf.f_bfree, - (long)sf.f_bsize * (long)sf.f_bfree); - } + if (errno != EINTR) { + return false; } - return false; } else if (n > 0) { - if (n < dataLen-soFar) { - LOGE("short write: %d bytes of %d from %p\n", - (int)n, (int)(dataLen-soFar), - data+soFar); - } soFar += n; if (soFar == dataLen) return true; if (soFar > dataLen) { @@ -824,10 +799,10 @@ static bool writeProcessFunction(const unsigned char *data, int dataLen, * Uncompress "pEntry" in "pArchive" to "fd" at the current offset. */ bool mzExtractZipEntryToFile(const ZipArchive *pArchive, - const ZipEntry *pEntry, WriteInfo *wi) + const ZipEntry *pEntry, int fd) { bool ret = mzProcessZipEntryContents(pArchive, pEntry, writeProcessFunction, - wi); + (void*)fd); if (!ret) { LOGE("Can't extract entry to file.\n"); return false; @@ -929,11 +904,6 @@ bool mzExtractRecursive(const ZipArchive *pArchive, return false; } - unsigned char* buffer = malloc(WRITE_SIZE+WRITE_ALIGNMENT); - WriteInfo wi; - wi.aligned_buffer = buffer + WRITE_ALIGNMENT - - ((long)buffer % WRITE_ALIGNMENT); - unsigned int zipDirLen; char *zpath; @@ -1114,8 +1084,7 @@ bool mzExtractRecursive(const ZipArchive *pArchive, break; } - wi.fd = fd; - bool ok = mzExtractZipEntryToFile(pArchive, pEntry, &wi); + bool ok = mzExtractZipEntryToFile(pArchive, pEntry, fd); close(fd); if (!ok) { LOGE("Error extracting \"%s\"\n", targetFile); @@ -1138,7 +1107,6 @@ bool mzExtractRecursive(const ZipArchive *pArchive, free(helper.buf); free(zpath); - free(buffer); return ok; } diff --git a/minzip/Zip.h b/minzip/Zip.h index 57c0abd2c..1c1df2fae 100644 --- a/minzip/Zip.h +++ b/minzip/Zip.h @@ -55,14 +55,6 @@ typedef struct { size_t len; } UnterminatedString; -/* - * The information we pass down to writeProcessFunction. - */ -typedef struct { - int fd; - unsigned char* aligned_buffer; -} WriteInfo; - /* * Open a Zip archive. * @@ -174,7 +166,7 @@ bool mzIsZipEntryIntact(const ZipArchive *pArchive, const ZipEntry *pEntry); * Inflate and write an entry to a file. */ bool mzExtractZipEntryToFile(const ZipArchive *pArchive, - const ZipEntry *pEntry, WriteInfo *wi); + const ZipEntry *pEntry, int fd); /* * Inflate all entries under zipDir to the directory specified by -- cgit v1.2.3