diff options
author | Kenny Root <kroot@google.com> | 2010-07-12 09:02:18 -0700 |
---|---|---|
committer | Kenny Root <kroot@google.com> | 2010-07-12 09:11:24 -0700 |
commit | 87315aa1f0b64efcef416628215be89851cc4736 (patch) | |
tree | e7000fef6bfe18391398e4cd04399054f3a0c18e /libs | |
parent | e059b2784fcc89c6cd43301cc6af89cf3b8101f7 (diff) | |
download | frameworks_base-87315aa1f0b64efcef416628215be89851cc4736.zip frameworks_base-87315aa1f0b64efcef416628215be89851cc4736.tar.gz frameworks_base-87315aa1f0b64efcef416628215be89851cc4736.tar.bz2 |
Tweak ObbFile class
* Move error messages around to clarify the errors.
* Add extra error check when reading a file.
* Seek to the end of a file when writing the signature so the users of
the API don't have to remember to do it.
Change-Id: I2337051b9f9fa8147c5900237deec790dcd92436
Diffstat (limited to 'libs')
-rw-r--r-- | libs/utils/ObbFile.cpp | 38 | ||||
-rw-r--r-- | libs/utils/tests/ObbFile_test.cpp | 15 |
2 files changed, 36 insertions, 17 deletions
diff --git a/libs/utils/ObbFile.cpp b/libs/utils/ObbFile.cpp index 3a4a03a..fe49300 100644 --- a/libs/utils/ObbFile.cpp +++ b/libs/utils/ObbFile.cpp @@ -91,22 +91,24 @@ bool ObbFile::readFrom(const char* filename) fd = ::open(filename, O_RDONLY); if (fd < 0) { + LOGW("couldn't open file %s: %s", filename, strerror(errno)); goto out; } success = readFrom(fd); close(fd); -out: if (!success) { - LOGW("failed to read from %s\n", filename); + LOGW("failed to read from %s (fd=%d)\n", filename, fd); } + +out: return success; } bool ObbFile::readFrom(int fd) { if (fd < 0) { - LOGW("failed to read file\n"); + LOGW("attempt to read from invalid fd\n"); return false; } @@ -149,10 +151,16 @@ bool ObbFile::parseObbFile(int fd) footerSize = get4LE((unsigned char*)footer); if (footerSize > (size_t)fileLength - kFooterTagSize || footerSize > kMaxBufSize) { - LOGW("claimed footer size is too large (0x%08lx; file size is 0x%08llx)\n", + LOGW("claimed footer size is too large (0x%08zx; file size is 0x%08llx)\n", footerSize, fileLength); return false; } + + if (footerSize < kFooterMinSize) { + LOGW("claimed footer size is too small (%08zx; minimum size is 0x%x)\n", + footerSize, kFooterMinSize); + return false; + } } my_off64_t fileOffset = fileLength - footerSize - kFooterTagSize; @@ -161,26 +169,22 @@ bool ObbFile::parseObbFile(int fd) return false; } - size_t readAmount = kMaxBufSize; - if (readAmount > footerSize) - readAmount = footerSize; - - char* scanBuf = (char*)malloc(readAmount); + char* scanBuf = (char*)malloc(footerSize); if (scanBuf == NULL) { LOGW("couldn't allocate scanBuf: %s\n", strerror(errno)); return false; } - actual = TEMP_FAILURE_RETRY(read(fd, scanBuf, readAmount)); + actual = TEMP_FAILURE_RETRY(read(fd, scanBuf, footerSize)); // readAmount is guaranteed to be less than kMaxBufSize - if (actual != (ssize_t)readAmount) { + if (actual != (ssize_t)footerSize) { LOGI("couldn't read ObbFile footer: %s\n", strerror(errno)); free(scanBuf); return false; } #ifdef DEBUG - for (int i = 0; i < readAmount; ++i) { + for (int i = 0; i < footerSize; ++i) { LOGI("char: 0x%02x", scanBuf[i]); } #endif @@ -197,7 +201,8 @@ bool ObbFile::parseObbFile(int fd) uint32_t packageNameLen = get4LE((unsigned char*)scanBuf + kPackageNameLenOffset); if (packageNameLen <= 0 || packageNameLen > (footerSize - kPackageNameOffset)) { - LOGW("bad ObbFile package name length (0x%08x)\n", packageNameLen); + LOGW("bad ObbFile package name length (0x%04x; 0x%04x possible)\n", + packageNameLen, footerSize - kPackageNameOffset); free(scanBuf); return false; } @@ -206,6 +211,11 @@ bool ObbFile::parseObbFile(int fd) mPackageName = String8(const_cast<char*>(packageName), packageNameLen); free(scanBuf); + +#ifdef DEBUG + LOGI("Obb scan succeeded: packageName=%s, version=%d\n", mPackageName.string(), mVersion); +#endif + return true; } @@ -234,6 +244,8 @@ bool ObbFile::writeTo(int fd) return false; } + my_lseek64(fd, 0, SEEK_END); + if (mPackageName.size() == 0 || mVersion == -1) { LOGW("tried to write uninitialized ObbFile data"); return false; diff --git a/libs/utils/tests/ObbFile_test.cpp b/libs/utils/tests/ObbFile_test.cpp index 05aaf08..29bb70a 100644 --- a/libs/utils/tests/ObbFile_test.cpp +++ b/libs/utils/tests/ObbFile_test.cpp @@ -22,6 +22,8 @@ #include <gtest/gtest.h> +#include <fcntl.h> + namespace android { #define TEST_FILENAME "/test.obb" @@ -39,6 +41,11 @@ protected: const int totalLen = strlen(mExternalStorage) + strlen(TEST_FILENAME) + 1; mFileName = new char[totalLen]; snprintf(mFileName, totalLen, "%s%s", mExternalStorage, TEST_FILENAME); + + int fd = ::open(mFileName, O_CREAT | O_TRUNC); + if (fd < 0) { + FAIL() << "Couldn't create " << mFileName << " for tests"; + } } virtual void TearDown() { @@ -46,8 +53,8 @@ protected: }; TEST_F(ObbFileTest, ReadFailure) { - EXPECT_FALSE(mObbFile->readFrom(-1)) - << "No failure on invalid file descriptor"; + EXPECT_FALSE(mObbFile->readFrom(-1)) + << "No failure on invalid file descriptor"; } TEST_F(ObbFileTest, WriteThenRead) { @@ -66,10 +73,10 @@ TEST_F(ObbFileTest, WriteThenRead) { << "couldn't read from fake .obb file"; EXPECT_EQ(versionNum, mObbFile->getVersion()) - << "version didn't come out the same as it went in"; + << "version didn't come out the same as it went in"; const char* currentPackageName = mObbFile->getPackageName().string(); EXPECT_STREQ(packageName, currentPackageName) - << "package name didn't come out the same as it went in"; + << "package name didn't come out the same as it went in"; } } |