From d483c20a7e459bd2f8e5e134355a923282175977 Mon Sep 17 00:00:00 2001 From: Yabin Cui Date: Wed, 3 Feb 2016 17:08:52 -0800 Subject: applypatch: fix memory leaks reported by static analysis. Bug: 26906416 Change-Id: I163df5a8f3abda3ba5d4ed81dfc8567054eceb27 --- applypatch/Android.mk | 2 +- applypatch/applypatch.cpp | 179 +++++++++++++++++++++------------------------- applypatch/applypatch.h | 11 +-- applypatch/bspatch.cpp | 37 ++++------ applypatch/freecache.cpp | 141 +++++++++++++----------------------- applypatch/imgpatch.cpp | 56 ++++++--------- applypatch/main.cpp | 79 +++++++------------- 7 files changed, 202 insertions(+), 303 deletions(-) diff --git a/applypatch/Android.mk b/applypatch/Android.mk index 22151941e..23d0f7bb1 100644 --- a/applypatch/Android.mk +++ b/applypatch/Android.mk @@ -55,7 +55,7 @@ LOCAL_CLANG := true LOCAL_SRC_FILES := main.cpp LOCAL_MODULE := applypatch LOCAL_C_INCLUDES += bootable/recovery -LOCAL_STATIC_LIBRARIES += libapplypatch libbase libmtdutils libcrypto_static libbz +LOCAL_STATIC_LIBRARIES += libapplypatch libbase libmtdutils libcrypto_static libbz libedify LOCAL_SHARED_LIBRARIES += libz libcutils libc include $(BUILD_EXECUTABLE) diff --git a/applypatch/applypatch.cpp b/applypatch/applypatch.cpp index 75ffe0f08..75d77372e 100644 --- a/applypatch/applypatch.cpp +++ b/applypatch/applypatch.cpp @@ -25,6 +25,9 @@ #include #include +#include +#include + #include #include "openssl/sha.h" @@ -67,25 +70,29 @@ int LoadFileContents(const char* filename, FileContents* file) { } file->size = file->st.st_size; - file->data = reinterpret_cast(malloc(file->size)); + file->data = nullptr; + + std::unique_ptr data( + static_cast(malloc(file->size)), free); + if (data == nullptr) { + printf("failed to allocate memory: %s\n", strerror(errno)); + return -1; + } FILE* f = fopen(filename, "rb"); if (f == NULL) { printf("failed to open \"%s\": %s\n", filename, strerror(errno)); - free(file->data); - file->data = NULL; return -1; } - size_t bytes_read = fread(file->data, 1, file->size, f); + size_t bytes_read = fread(data.get(), 1, file->size, f); if (bytes_read != static_cast(file->size)) { printf("short read of \"%s\" (%zu bytes of %zd)\n", filename, bytes_read, file->size); - free(file->data); - file->data = NULL; + fclose(f); return -1; } fclose(f); - + file->data = data.release(); SHA1(file->data, file->size, file->sha1); return 0; } @@ -185,7 +192,7 @@ static int LoadPartitionContents(const char* filename, FileContents* file) { uint8_t parsed_sha[SHA_DIGEST_LENGTH]; // Allocate enough memory to hold the largest size. - file->data = reinterpret_cast(malloc(size[index[pairs-1]])); + file->data = static_cast(malloc(size[index[pairs-1]])); char* p = (char*)file->data; file->size = 0; // # bytes read so far bool found = false; @@ -313,7 +320,7 @@ int SaveFileContents(const char* filename, const FileContents* file) { // "MTD:[:...]" or "EMMC:[:...]". The target name // might contain multiple colons, but WriteToPartition() only uses the first // two and ignores the rest. Return 0 on success. -int WriteToPartition(unsigned char* data, size_t len, const char* target) { +int WriteToPartition(const unsigned char* data, size_t len, const char* target) { std::string copy(target); std::vector pieces = android::base::Split(copy, ":"); @@ -352,7 +359,7 @@ int WriteToPartition(unsigned char* data, size_t len, const char* target) { return -1; } - size_t written = mtd_write_data(ctx, reinterpret_cast(data), len); + size_t written = mtd_write_data(ctx, reinterpret_cast(data), len); if (written != len) { printf("only wrote %zu of %zu bytes to MTD %s\n", written, len, partition); mtd_write_close(ctx); @@ -578,7 +585,7 @@ int ShowLicenses() { } ssize_t FileSink(const unsigned char* data, ssize_t len, void* token) { - int fd = *reinterpret_cast(token); + int fd = *static_cast(token); ssize_t done = 0; ssize_t wrote; while (done < len) { @@ -592,19 +599,9 @@ ssize_t FileSink(const unsigned char* data, ssize_t len, void* token) { return done; } -typedef struct { - unsigned char* buffer; - ssize_t size; - ssize_t pos; -} MemorySinkInfo; - ssize_t MemorySink(const unsigned char* data, ssize_t len, void* token) { - MemorySinkInfo* msi = reinterpret_cast(token); - if (msi->size - msi->pos < len) { - return -1; - } - memcpy(msi->buffer + msi->pos, data, len); - msi->pos += len; + std::string* s = static_cast(token); + s->append(reinterpret_cast(data), len); return len; } @@ -815,31 +812,52 @@ static int GenerateTarget(FileContents* source_file, const Value* bonus_data) { int retry = 1; SHA_CTX ctx; - int output; - MemorySinkInfo msi; + std::string memory_sink_str; FileContents* source_to_use; - char* outname; int made_copy = 0; + bool target_is_partition = (strncmp(target_filename, "MTD:", 4) == 0 || + strncmp(target_filename, "EMMC:", 5) == 0); + const std::string tmp_target_filename = std::string(target_filename) + ".patch"; + // assume that target_filename (eg "/system/app/Foo.apk") is located // on the same filesystem as its top-level directory ("/system"). // We need something that exists for calling statfs(). - char target_fs[strlen(target_filename)+1]; - char* slash = strchr(target_filename+1, '/'); - if (slash != NULL) { - int count = slash - target_filename; - strncpy(target_fs, target_filename, count); - target_fs[count] = '\0'; + std::string target_fs = target_filename; + auto slash_pos = target_fs.find('/', 1); + if (slash_pos != std::string::npos) { + target_fs.resize(slash_pos); + } + + const Value* patch; + if (source_patch_value != NULL) { + source_to_use = source_file; + patch = source_patch_value; } else { - strcpy(target_fs, target_filename); + source_to_use = copy_file; + patch = copy_patch_value; + } + if (patch->type != VAL_BLOB) { + printf("patch is not a blob\n"); + return 1; + } + char* header = patch->data; + ssize_t header_bytes_read = patch->size; + bool use_bsdiff = false; + if (header_bytes_read >= 8 && memcmp(header, "BSDIFF40", 8) == 0) { + use_bsdiff = true; + } else if (header_bytes_read >= 8 && memcmp(header, "IMGDIFF2", 8) == 0) { + use_bsdiff = false; + } else { + printf("Unknown patch file format\n"); + return 1; } do { // Is there enough room in the target filesystem to hold the patched // file? - if (strncmp(target_filename, "MTD:", 4) == 0 || - strncmp(target_filename, "EMMC:", 5) == 0) { + if (target_is_partition) { // If the target is a partition, we're actually going to // write the output to /tmp and then copy it to the // partition. statfs() always returns 0 blocks free for @@ -861,7 +879,7 @@ static int GenerateTarget(FileContents* source_file, } else { int enough_space = 0; if (retry > 0) { - size_t free_space = FreeSpaceForFile(target_fs); + size_t free_space = FreeSpaceForFile(target_fs.c_str()); enough_space = (free_space > (256 << 10)) && // 256k (two-block) minimum (free_space > (target_size * 3 / 2)); // 50% margin of error @@ -901,84 +919,53 @@ static int GenerateTarget(FileContents* source_file, made_copy = 1; unlink(source_filename); - size_t free_space = FreeSpaceForFile(target_fs); + size_t free_space = FreeSpaceForFile(target_fs.c_str()); printf("(now %zu bytes free for target) ", free_space); } } - const Value* patch; - if (source_patch_value != NULL) { - source_to_use = source_file; - patch = source_patch_value; - } else { - source_to_use = copy_file; - patch = copy_patch_value; - } - - if (patch->type != VAL_BLOB) { - printf("patch is not a blob\n"); - return 1; - } SinkFn sink = NULL; void* token = NULL; - output = -1; - outname = NULL; - if (strncmp(target_filename, "MTD:", 4) == 0 || - strncmp(target_filename, "EMMC:", 5) == 0) { + int output_fd = -1; + if (target_is_partition) { // We store the decoded output in memory. - msi.buffer = reinterpret_cast(malloc(target_size)); - if (msi.buffer == NULL) { - printf("failed to alloc %zu bytes for output\n", target_size); - return 1; - } - msi.pos = 0; - msi.size = target_size; sink = MemorySink; - token = &msi; + token = &memory_sink_str; } else { // We write the decoded output to ".patch". - outname = reinterpret_cast(malloc(strlen(target_filename) + 10)); - strcpy(outname, target_filename); - strcat(outname, ".patch"); - - output = open(outname, O_WRONLY | O_CREAT | O_TRUNC | O_SYNC, S_IRUSR | S_IWUSR); - if (output < 0) { - printf("failed to open output file %s: %s\n", - outname, strerror(errno)); + output_fd = open(tmp_target_filename.c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_SYNC, + S_IRUSR | S_IWUSR); + if (output_fd < 0) { + printf("failed to open output file %s: %s\n", tmp_target_filename.c_str(), + strerror(errno)); return 1; } sink = FileSink; - token = &output; + token = &output_fd; } - char* header = patch->data; - ssize_t header_bytes_read = patch->size; SHA1_Init(&ctx); int result; - - if (header_bytes_read >= 8 && - memcmp(header, "BSDIFF40", 8) == 0) { + if (use_bsdiff) { result = ApplyBSDiffPatch(source_to_use->data, source_to_use->size, patch, 0, sink, token, &ctx); - } else if (header_bytes_read >= 8 && - memcmp(header, "IMGDIFF2", 8) == 0) { + } else { result = ApplyImagePatch(source_to_use->data, source_to_use->size, patch, sink, token, &ctx, bonus_data); - } else { - printf("Unknown patch file format\n"); - return 1; } - if (output >= 0) { - if (fsync(output) != 0) { - printf("failed to fsync file \"%s\" (%s)\n", outname, strerror(errno)); + if (!target_is_partition) { + if (fsync(output_fd) != 0) { + printf("failed to fsync file \"%s\" (%s)\n", tmp_target_filename.c_str(), + strerror(errno)); result = 1; } - if (close(output) != 0) { - printf("failed to close file \"%s\" (%s)\n", outname, strerror(errno)); + if (close(output_fd) != 0) { + printf("failed to close file \"%s\" (%s)\n", tmp_target_filename.c_str(), + strerror(errno)); result = 1; } } @@ -990,8 +977,8 @@ static int GenerateTarget(FileContents* source_file, } else { printf("applying patch failed; retrying\n"); } - if (outname != NULL) { - unlink(outname); + if (!target_is_partition) { + unlink(tmp_target_filename.c_str()); } } else { // succeeded; no need to retry @@ -1008,27 +995,27 @@ static int GenerateTarget(FileContents* source_file, printf("now %s\n", short_sha1(target_sha1).c_str()); } - if (output < 0) { + if (target_is_partition) { // Copy the temp file to the partition. - if (WriteToPartition(msi.buffer, msi.pos, target_filename) != 0) { + if (WriteToPartition(reinterpret_cast(memory_sink_str.c_str()), + memory_sink_str.size(), target_filename) != 0) { printf("write of patched data to %s failed\n", target_filename); return 1; } - free(msi.buffer); } else { // Give the .patch file the same owner, group, and mode of the // original source file. - if (chmod(outname, source_to_use->st.st_mode) != 0) { - printf("chmod of \"%s\" failed: %s\n", outname, strerror(errno)); + if (chmod(tmp_target_filename.c_str(), source_to_use->st.st_mode) != 0) { + printf("chmod of \"%s\" failed: %s\n", tmp_target_filename.c_str(), strerror(errno)); return 1; } - if (chown(outname, source_to_use->st.st_uid, source_to_use->st.st_gid) != 0) { - printf("chown of \"%s\" failed: %s\n", outname, strerror(errno)); + if (chown(tmp_target_filename.c_str(), source_to_use->st.st_uid, source_to_use->st.st_gid) != 0) { + printf("chown of \"%s\" failed: %s\n", tmp_target_filename.c_str(), strerror(errno)); return 1; } // Finally, rename the .patch file to replace the target file. - if (rename(outname, target_filename) != 0) { + if (rename(tmp_target_filename.c_str(), target_filename) != 0) { printf("rename of .patch to \"%s\" failed: %s\n", target_filename, strerror(errno)); return 1; } diff --git a/applypatch/applypatch.h b/applypatch/applypatch.h index e0df104b5..14fb490ba 100644 --- a/applypatch/applypatch.h +++ b/applypatch/applypatch.h @@ -18,6 +18,9 @@ #define _APPLYPATCH_H #include + +#include + #include "openssl/sha.h" #include "edify/expr.h" @@ -68,22 +71,22 @@ void FreeFileContents(FileContents* file); int FindMatchingPatch(uint8_t* sha1, char* const * const patch_sha1_str, int num_patches); -// bsdiff.c +// bsdiff.cpp void ShowBSDiffLicense(); int ApplyBSDiffPatch(const unsigned char* old_data, ssize_t old_size, const Value* patch, ssize_t patch_offset, SinkFn sink, void* token, SHA_CTX* ctx); int ApplyBSDiffPatchMem(const unsigned char* old_data, ssize_t old_size, const Value* patch, ssize_t patch_offset, - unsigned char** new_data, ssize_t* new_size); + std::vector* new_data); -// imgpatch.c +// imgpatch.cpp int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size, const Value* patch, SinkFn sink, void* token, SHA_CTX* ctx, const Value* bonus_data); -// freecache.c +// freecache.cpp int MakeFreeSpaceOnCache(size_t bytes_needed); #endif diff --git a/applypatch/bspatch.cpp b/applypatch/bspatch.cpp index 25171170a..ebb55f1d1 100644 --- a/applypatch/bspatch.cpp +++ b/applypatch/bspatch.cpp @@ -24,7 +24,6 @@ #include #include #include -#include #include #include @@ -103,26 +102,22 @@ int ApplyBSDiffPatch(const unsigned char* old_data, ssize_t old_size, const Value* patch, ssize_t patch_offset, SinkFn sink, void* token, SHA_CTX* ctx) { - unsigned char* new_data; - ssize_t new_size; - if (ApplyBSDiffPatchMem(old_data, old_size, patch, patch_offset, - &new_data, &new_size) != 0) { + std::vector new_data; + if (ApplyBSDiffPatchMem(old_data, old_size, patch, patch_offset, &new_data) != 0) { return -1; } - if (sink(new_data, new_size, token) < new_size) { + if (sink(new_data.data(), new_data.size(), token) < static_cast(new_data.size())) { printf("short write of output: %d (%s)\n", errno, strerror(errno)); return 1; } - if (ctx) SHA1_Update(ctx, new_data, new_size); - free(new_data); - + if (ctx) SHA1_Update(ctx, new_data.data(), new_data.size()); return 0; } int ApplyBSDiffPatchMem(const unsigned char* old_data, ssize_t old_size, const Value* patch, ssize_t patch_offset, - unsigned char** new_data, ssize_t* new_size) { + std::vector* new_data) { // Patch data format: // 0 8 "BSDIFF40" // 8 8 X @@ -141,12 +136,12 @@ int ApplyBSDiffPatchMem(const unsigned char* old_data, ssize_t old_size, return 1; } - ssize_t ctrl_len, data_len; + ssize_t ctrl_len, data_len, new_size; ctrl_len = offtin(header+8); data_len = offtin(header+16); - *new_size = offtin(header+24); + new_size = offtin(header+24); - if (ctrl_len < 0 || data_len < 0 || *new_size < 0) { + if (ctrl_len < 0 || data_len < 0 || new_size < 0) { printf("corrupt patch file header (data lengths)\n"); return 1; } @@ -183,18 +178,14 @@ int ApplyBSDiffPatchMem(const unsigned char* old_data, ssize_t old_size, printf("failed to bzinit extra stream (%d)\n", bzerr); } - *new_data = reinterpret_cast(malloc(*new_size)); - if (*new_data == NULL) { - printf("failed to allocate %zd bytes of memory for output file\n", *new_size); - return 1; - } + new_data->resize(new_size); off_t oldpos = 0, newpos = 0; off_t ctrl[3]; off_t len_read; int i; unsigned char buf[24]; - while (newpos < *new_size) { + while (newpos < new_size) { // Read control data if (FillBuffer(buf, 24, &cstream) != 0) { printf("error while reading control stream\n"); @@ -210,13 +201,13 @@ int ApplyBSDiffPatchMem(const unsigned char* old_data, ssize_t old_size, } // Sanity check - if (newpos + ctrl[0] > *new_size) { + if (newpos + ctrl[0] > new_size) { printf("corrupt patch (new file overrun)\n"); return 1; } // Read diff string - if (FillBuffer(*new_data + newpos, ctrl[0], &dstream) != 0) { + if (FillBuffer(new_data->data() + newpos, ctrl[0], &dstream) != 0) { printf("error while reading diff stream\n"); return 1; } @@ -233,13 +224,13 @@ int ApplyBSDiffPatchMem(const unsigned char* old_data, ssize_t old_size, oldpos += ctrl[0]; // Sanity check - if (newpos + ctrl[1] > *new_size) { + if (newpos + ctrl[1] > new_size) { printf("corrupt patch (new file overrun)\n"); return 1; } // Read extra string - if (FillBuffer(*new_data + newpos, ctrl[1], &estream) != 0) { + if (FillBuffer(new_data->data() + newpos, ctrl[1], &estream) != 0) { printf("error while reading extra stream\n"); return 1; } diff --git a/applypatch/freecache.cpp b/applypatch/freecache.cpp index 2eb2f55ef..c84f42797 100644 --- a/applypatch/freecache.cpp +++ b/applypatch/freecache.cpp @@ -25,119 +25,90 @@ #include #include +#include +#include +#include + +#include +#include + #include "applypatch.h" -static int EliminateOpenFiles(char** files, int file_count) { - DIR* d; - struct dirent* de; - d = opendir("/proc"); - if (d == NULL) { +static int EliminateOpenFiles(std::set* files) { + std::unique_ptr d(opendir("/proc"), closedir); + if (!d) { printf("error opening /proc: %s\n", strerror(errno)); return -1; } - while ((de = readdir(d)) != 0) { - int i; - for (i = 0; de->d_name[i] != '\0' && isdigit(de->d_name[i]); ++i); - if (de->d_name[i]) continue; - - // de->d_name[i] is numeric - - char path[FILENAME_MAX]; - strcpy(path, "/proc/"); - strcat(path, de->d_name); - strcat(path, "/fd/"); + struct dirent* de; + while ((de = readdir(d.get())) != 0) { + unsigned int pid; + if (!android::base::ParseUint(de->d_name, &pid)) { + continue; + } + std::string path = android::base::StringPrintf("/proc/%s/fd/", de->d_name); - DIR* fdd; struct dirent* fdde; - fdd = opendir(path); - if (fdd == NULL) { - printf("error opening %s: %s\n", path, strerror(errno)); + std::unique_ptr fdd(opendir(path.c_str()), closedir); + if (!fdd) { + printf("error opening %s: %s\n", path.c_str(), strerror(errno)); continue; } - while ((fdde = readdir(fdd)) != 0) { - char fd_path[FILENAME_MAX]; + while ((fdde = readdir(fdd.get())) != 0) { + std::string fd_path = path + fdde->d_name; char link[FILENAME_MAX]; - strcpy(fd_path, path); - strcat(fd_path, fdde->d_name); - int count; - count = readlink(fd_path, link, sizeof(link)-1); + int count = readlink(fd_path.c_str(), link, sizeof(link)-1); if (count >= 0) { link[count] = '\0'; - - // This is inefficient, but it should only matter if there are - // lots of files in /cache, and lots of them are open (neither - // of which should be true, especially in recovery). if (strncmp(link, "/cache/", 7) == 0) { - int j; - for (j = 0; j < file_count; ++j) { - if (files[j] && strcmp(files[j], link) == 0) { - printf("%s is open by %s\n", link, de->d_name); - free(files[j]); - files[j] = NULL; - } + if (files->erase(link) > 0) { + printf("%s is open by %s\n", link, de->d_name); } } } } - closedir(fdd); } - closedir(d); - return 0; } -int FindExpendableFiles(char*** names, int* entries) { - DIR* d; - struct dirent* de; - int size = 32; - *entries = 0; - *names = reinterpret_cast(malloc(size * sizeof(char*))); - - char path[FILENAME_MAX]; - +static std::set FindExpendableFiles() { + std::set files; // We're allowed to delete unopened regular files in any of these // directories. const char* dirs[2] = {"/cache", "/cache/recovery/otatest"}; for (size_t i = 0; i < sizeof(dirs)/sizeof(dirs[0]); ++i) { - d = opendir(dirs[i]); - if (d == NULL) { + std::unique_ptr d(opendir(dirs[i]), closedir); + if (!d) { printf("error opening %s: %s\n", dirs[i], strerror(errno)); continue; } // Look for regular files in the directory (not in any subdirectories). - while ((de = readdir(d)) != 0) { - strcpy(path, dirs[i]); - strcat(path, "/"); - strcat(path, de->d_name); + struct dirent* de; + while ((de = readdir(d.get())) != 0) { + std::string path = std::string(dirs[i]) + "/" + de->d_name; // We can't delete CACHE_TEMP_SOURCE; if it's there we might have // restarted during installation and could be depending on it to // be there. - if (strcmp(path, CACHE_TEMP_SOURCE) == 0) continue; + if (path == CACHE_TEMP_SOURCE) { + continue; + } struct stat st; - if (stat(path, &st) == 0 && S_ISREG(st.st_mode)) { - if (*entries >= size) { - size *= 2; - *names = reinterpret_cast(realloc(*names, size * sizeof(char*))); - } - (*names)[(*entries)++] = strdup(path); + if (stat(path.c_str(), &st) == 0 && S_ISREG(st.st_mode)) { + files.insert(path); } } - - closedir(d); } - printf("%d regular files in deletable directories\n", *entries); - - if (EliminateOpenFiles(*names, *entries) < 0) { - return -1; + printf("%zu regular files in deletable directories\n", files.size()); + if (EliminateOpenFiles(&files) < 0) { + return std::set(); } - - return 0; + return files; } int MakeFreeSpaceOnCache(size_t bytes_needed) { @@ -147,15 +118,8 @@ int MakeFreeSpaceOnCache(size_t bytes_needed) { if (free_now >= bytes_needed) { return 0; } - - char** names; - int entries; - - if (FindExpendableFiles(&names, &entries) < 0) { - return -1; - } - - if (entries == 0) { + std::set files = FindExpendableFiles(); + if (files.empty()) { // nothing we can delete to free up space! printf("no files can be deleted to free space on /cache\n"); return -1; @@ -167,20 +131,13 @@ int MakeFreeSpaceOnCache(size_t bytes_needed) { // // Instead, we'll be dumb. - int i; - for (i = 0; i < entries && free_now < bytes_needed; ++i) { - if (names[i]) { - unlink(names[i]); - free_now = FreeSpaceForFile("/cache"); - printf("deleted %s; now %zu bytes free\n", names[i], free_now); - free(names[i]); + for (const auto& file : files) { + unlink(file.c_str()); + free_now = FreeSpaceForFile("/cache"); + printf("deleted %s; now %zu bytes free\n", file.c_str(), free_now); + if (free_now < bytes_needed) { + break; } } - - for (; i < entries; ++i) { - free(names[i]); - } - free(names); - return (free_now >= bytes_needed) ? 0 : -1; } diff --git a/applypatch/imgpatch.cpp b/applypatch/imgpatch.cpp index c9944dfc1..8824038ea 100644 --- a/applypatch/imgpatch.cpp +++ b/applypatch/imgpatch.cpp @@ -21,10 +21,11 @@ #include #include #include -#include #include #include +#include + #include "zlib.h" #include "openssl/sha.h" #include "applypatch.h" @@ -129,7 +130,6 @@ int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size, size_t src_len = Read8(deflate_header+8); size_t patch_offset = Read8(deflate_header+16); size_t expanded_len = Read8(deflate_header+24); - size_t target_len = Read8(deflate_header+32); int level = Read4(deflate_header+40); int method = Read4(deflate_header+44); int windowBits = Read4(deflate_header+48); @@ -150,13 +150,7 @@ int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size, // must be appended from the bonus_data value. size_t bonus_size = (i == 1 && bonus_data != NULL) ? bonus_data->size : 0; - unsigned char* expanded_source = reinterpret_cast(malloc(expanded_len)); - if (expanded_source == NULL) { - printf("failed to allocate %zu bytes for expanded_source\n", - expanded_len); - return -1; - } - + std::vector expanded_source(expanded_len); z_stream strm; strm.zalloc = Z_NULL; strm.zfree = Z_NULL; @@ -164,7 +158,7 @@ int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size, strm.avail_in = src_len; strm.next_in = (unsigned char*)(old_data + src_start); strm.avail_out = expanded_len; - strm.next_out = expanded_source; + strm.next_out = expanded_source.data(); int ret; ret = inflateInit2(&strm, -15); @@ -189,18 +183,16 @@ int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size, inflateEnd(&strm); if (bonus_size) { - memcpy(expanded_source + (expanded_len - bonus_size), + memcpy(expanded_source.data() + (expanded_len - bonus_size), bonus_data->data, bonus_size); } // Next, apply the bsdiff patch (in memory) to the uncompressed // data. - unsigned char* uncompressed_target_data; - ssize_t uncompressed_target_size; - if (ApplyBSDiffPatchMem(expanded_source, expanded_len, + std::vector uncompressed_target_data; + if (ApplyBSDiffPatchMem(expanded_source.data(), expanded_len, patch, patch_offset, - &uncompressed_target_data, - &uncompressed_target_size) != 0) { + &uncompressed_target_data) != 0) { return -1; } @@ -208,40 +200,36 @@ int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size, // we're done with the expanded_source data buffer, so we'll // reuse that memory to receive the output of deflate. - unsigned char* temp_data = expanded_source; - ssize_t temp_size = expanded_len; - if (temp_size < 32768) { - // ... unless the buffer is too small, in which case we'll - // allocate a fresh one. - free(temp_data); - temp_data = reinterpret_cast(malloc(32768)); - temp_size = 32768; + if (expanded_source.size() < 32768U) { + expanded_source.resize(32768U); } + std::vector& temp_data = expanded_source; // now the deflate stream strm.zalloc = Z_NULL; strm.zfree = Z_NULL; strm.opaque = Z_NULL; - strm.avail_in = uncompressed_target_size; - strm.next_in = uncompressed_target_data; + strm.avail_in = uncompressed_target_data.size(); + strm.next_in = uncompressed_target_data.data(); ret = deflateInit2(&strm, level, method, windowBits, memLevel, strategy); + if (ret != Z_OK) { + printf("failed to init uncompressed data deflation: %d\n", ret); + return -1; + } do { - strm.avail_out = temp_size; - strm.next_out = temp_data; + strm.avail_out = temp_data.size(); + strm.next_out = temp_data.data(); ret = deflate(&strm, Z_FINISH); - ssize_t have = temp_size - strm.avail_out; + ssize_t have = temp_data.size() - strm.avail_out; - if (sink(temp_data, have, token) != have) { + if (sink(temp_data.data(), have, token) != have) { printf("failed to write %ld compressed bytes to output\n", (long)have); return -1; } - if (ctx) SHA1_Update(ctx, temp_data, have); + if (ctx) SHA1_Update(ctx, temp_data.data(), have); } while (ret != Z_STREAM_END); deflateEnd(&strm); - - free(temp_data); - free(uncompressed_target_data); } else { printf("patch chunk %d is unknown type %d\n", i, type); return -1; diff --git a/applypatch/main.cpp b/applypatch/main.cpp index 445a7fee7..7606d5d3c 100644 --- a/applypatch/main.cpp +++ b/applypatch/main.cpp @@ -19,6 +19,9 @@ #include #include +#include +#include + #include "applypatch.h" #include "edify/expr.h" #include "openssl/sha.h" @@ -47,16 +50,11 @@ static int SpaceMode(int argc, char** argv) { // ":" into the new parallel arrays *sha1s and // *patches (loading file contents into the patches). Returns true on // success. -static bool ParsePatchArgs(int argc, char** argv, char*** sha1s, - Value*** patches, int* num_patches) { - *num_patches = argc; - *sha1s = reinterpret_cast(malloc(*num_patches * sizeof(char*))); - *patches = reinterpret_cast(malloc(*num_patches * sizeof(Value*))); - memset(*patches, 0, *num_patches * sizeof(Value*)); - +static bool ParsePatchArgs(int argc, char** argv, std::vector* sha1s, + std::vector>* patches) { uint8_t digest[SHA_DIGEST_LENGTH]; - for (int i = 0; i < *num_patches; ++i) { + for (int i = 0; i < argc; ++i) { char* colon = strchr(argv[i], ':'); if (colon != NULL) { *colon = '\0'; @@ -68,34 +66,22 @@ static bool ParsePatchArgs(int argc, char** argv, char*** sha1s, return false; } - (*sha1s)[i] = argv[i]; + sha1s->push_back(argv[i]); if (colon == NULL) { - (*patches)[i] = NULL; + patches->emplace_back(nullptr, FreeValue); } else { FileContents fc; if (LoadFileContents(colon, &fc) != 0) { - goto abort; + return false; } - (*patches)[i] = reinterpret_cast(malloc(sizeof(Value))); - (*patches)[i]->type = VAL_BLOB; - (*patches)[i]->size = fc.size; - (*patches)[i]->data = reinterpret_cast(fc.data); + std::unique_ptr value(new Value, FreeValue); + value->type = VAL_BLOB; + value->size = fc.size; + value->data = reinterpret_cast(fc.data); + patches->push_back(std::move(value)); } } - return true; - - abort: - for (int i = 0; i < *num_patches; ++i) { - Value* p = (*patches)[i]; - if (p != NULL) { - free(p->data); - free(p); - } - } - free(*sha1s); - free(*patches); - return false; } static int FlashMode(const char* src_filename, const char* tgt_filename, @@ -104,17 +90,17 @@ static int FlashMode(const char* src_filename, const char* tgt_filename, } static int PatchMode(int argc, char** argv) { - Value* bonus = NULL; + std::unique_ptr bonus(nullptr, FreeValue); if (argc >= 3 && strcmp(argv[1], "-b") == 0) { FileContents fc; if (LoadFileContents(argv[2], &fc) != 0) { printf("failed to load bonus file %s\n", argv[2]); return 1; } - bonus = reinterpret_cast(malloc(sizeof(Value))); + bonus.reset(new Value); bonus->type = VAL_BLOB; bonus->size = fc.size; - bonus->data = (char*)fc.data; + bonus->data = reinterpret_cast(fc.data); argc -= 2; argv += 2; } @@ -140,33 +126,20 @@ static int PatchMode(int argc, char** argv) { } - char** sha1s; - Value** patches; - int num_patches; - if (!ParsePatchArgs(argc-5, argv+5, &sha1s, &patches, &num_patches)) { + std::vector sha1s; + std::vector> patches; + if (!ParsePatchArgs(argc-5, argv+5, &sha1s, &patches)) { printf("failed to parse patch args\n"); return 1; } - int result = applypatch(argv[1], argv[2], argv[3], target_size, - num_patches, sha1s, patches, bonus); - - int i; - for (i = 0; i < num_patches; ++i) { - Value* p = patches[i]; - if (p != NULL) { - free(p->data); - free(p); - } - } - if (bonus) { - free(bonus->data); - free(bonus); + std::vector patch_ptrs; + for (const auto& p : patches) { + patch_ptrs.push_back(p.get()); } - free(sha1s); - free(patches); - - return result; + return applypatch(argv[1], argv[2], argv[3], target_size, + patch_ptrs.size(), sha1s.data(), + patch_ptrs.data(), bonus.get()); } // This program applies binary patches to files in a way that is safe -- cgit v1.2.3