From aced5d9e4e438ba478ce54f9217166b8ab7cc24f Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Wed, 12 Oct 2016 10:55:04 -0700 Subject: Change StringValue to use std::string Changing the field of 'Value' in edify to std::string from char*. Meanwhile cleaning up the users of 'Value' and switching them to cpp style. Test: compontent tests passed. Bug: 31713288 Change-Id: Iec5a7d601b1e4ca40935bf1c70d325dafecec235 --- applypatch/applypatch.cpp | 28 ++- applypatch/bspatch.cpp | 12 +- applypatch/imgpatch.cpp | 44 ++-- applypatch/include/applypatch/applypatch.h | 10 +- applypatch/main.cpp | 42 ++-- applypatch/utils.cpp | 12 +- applypatch/utils.h | 6 +- edify/edify_parser.cpp | 6 +- edify/expr.cpp | 328 +++++++++++++++-------------- edify/expr.h | 44 ++-- tests/component/applypatch_test.cpp | 132 ++++++------ tests/component/edify_test.cpp | 8 +- tests/component/updater_test.cpp | 8 +- updater/blockimg.cpp | 154 +++++++------- updater/install.cpp | 191 +++++++---------- updater/updater.cpp | 8 +- 16 files changed, 505 insertions(+), 528 deletions(-) diff --git a/applypatch/applypatch.cpp b/applypatch/applypatch.cpp index e52ef99dc..cf155607b 100644 --- a/applypatch/applypatch.cpp +++ b/applypatch/applypatch.cpp @@ -408,11 +408,10 @@ int ParseSha1(const char* str, uint8_t* digest) { // Search an array of sha1 strings for one matching the given sha1. // Return the index of the match on success, or -1 if no match is // found. -int FindMatchingPatch(uint8_t* sha1, char* const * const patch_sha1_str, - int num_patches) { - uint8_t patch_sha1[SHA_DIGEST_LENGTH]; - for (int i = 0; i < num_patches; ++i) { - if (ParseSha1(patch_sha1_str[i], patch_sha1) == 0 && +int FindMatchingPatch(uint8_t* sha1, const std::vector& patch_sha1_str) { + for (size_t i = 0; i < patch_sha1_str.size(); ++i) { + uint8_t patch_sha1[SHA_DIGEST_LENGTH]; + if (ParseSha1(patch_sha1_str[i].c_str(), patch_sha1) == 0 && memcmp(patch_sha1, sha1, SHA_DIGEST_LENGTH) == 0) { return i; } @@ -423,8 +422,7 @@ int FindMatchingPatch(uint8_t* sha1, char* const * const patch_sha1_str, // Returns 0 if the contents of the file (argv[2]) or the cached file // match any of the sha1's on the command line (argv[3:]). Returns // nonzero otherwise. -int applypatch_check(const char* filename, int num_patches, - char** const patch_sha1_str) { +int applypatch_check(const char* filename, const std::vector& patch_sha1_str) { FileContents file; // It's okay to specify no sha1s; the check will pass if the @@ -432,8 +430,7 @@ int applypatch_check(const char* filename, int num_patches, // partitions, where the filename encodes the sha1s; no need to // check them twice.) if (LoadFileContents(filename, &file) != 0 || - (num_patches > 0 && - FindMatchingPatch(file.sha1, patch_sha1_str, num_patches) < 0)) { + FindMatchingPatch(file.sha1, patch_sha1_str) < 0) { printf("file \"%s\" doesn't have any of expected " "sha1 sums; checking cache\n", filename); @@ -448,7 +445,7 @@ int applypatch_check(const char* filename, int num_patches, return 1; } - if (FindMatchingPatch(file.sha1, patch_sha1_str, num_patches) < 0) { + if (FindMatchingPatch(file.sha1, patch_sha1_str) < 0) { printf("cache bits don't match any sha1 for \"%s\"\n", filename); return 1; } @@ -532,8 +529,7 @@ int applypatch(const char* source_filename, const char* target_filename, const char* target_sha1_str, size_t target_size, - int num_patches, - char** const patch_sha1_str, + const std::vector& patch_sha1_str, Value** patch_data, Value* bonus_data) { printf("patch %s: ", source_filename); @@ -573,7 +569,7 @@ int applypatch(const char* source_filename, } if (!source_file.data.empty()) { - int to_use = FindMatchingPatch(source_file.sha1, patch_sha1_str, num_patches); + int to_use = FindMatchingPatch(source_file.sha1, patch_sha1_str); if (to_use >= 0) { source_patch_value = patch_data[to_use]; } @@ -589,7 +585,7 @@ int applypatch(const char* source_filename, return 1; } - int to_use = FindMatchingPatch(copy_file.sha1, patch_sha1_str, num_patches); + int to_use = FindMatchingPatch(copy_file.sha1, patch_sha1_str); if (to_use >= 0) { copy_patch_value = patch_data[to_use]; } @@ -701,8 +697,8 @@ static int GenerateTarget(FileContents* source_file, printf("patch is not a blob\n"); return 1; } - char* header = patch->data; - ssize_t header_bytes_read = patch->size; + const char* header = &patch->data[0]; + size_t header_bytes_read = patch->data.size(); bool use_bsdiff = false; if (header_bytes_read >= 8 && memcmp(header, "BSDIFF40", 8) == 0) { use_bsdiff = true; diff --git a/applypatch/bspatch.cpp b/applypatch/bspatch.cpp index a4945da28..eb45e9ce9 100644 --- a/applypatch/bspatch.cpp +++ b/applypatch/bspatch.cpp @@ -64,7 +64,7 @@ void ShowBSDiffLicense() { ); } -static off_t offtin(u_char *buf) +static off_t offtin(const u_char *buf) { off_t y; @@ -130,7 +130,7 @@ int ApplyBSDiffPatchMem(const unsigned char* old_data, ssize_t old_size, // from oldfile to x bytes from the diff block; copy y bytes from the // extra block; seek forwards in oldfile by z bytes". - unsigned char* header = (unsigned char*) patch->data + patch_offset; + const unsigned char* header = reinterpret_cast(&patch->data[patch_offset]); if (memcmp(header, "BSDIFF40", 8) != 0) { printf("corrupt bsdiff patch file header (magic number)\n"); return 1; @@ -149,7 +149,7 @@ int ApplyBSDiffPatchMem(const unsigned char* old_data, ssize_t old_size, int bzerr; bz_stream cstream; - cstream.next_in = patch->data + patch_offset + 32; + cstream.next_in = const_cast(&patch->data[patch_offset + 32]); cstream.avail_in = ctrl_len; cstream.bzalloc = NULL; cstream.bzfree = NULL; @@ -159,7 +159,7 @@ int ApplyBSDiffPatchMem(const unsigned char* old_data, ssize_t old_size, } bz_stream dstream; - dstream.next_in = patch->data + patch_offset + 32 + ctrl_len; + dstream.next_in = const_cast(&patch->data[patch_offset + 32 + ctrl_len]); dstream.avail_in = data_len; dstream.bzalloc = NULL; dstream.bzfree = NULL; @@ -169,8 +169,8 @@ int ApplyBSDiffPatchMem(const unsigned char* old_data, ssize_t old_size, } bz_stream estream; - estream.next_in = patch->data + patch_offset + 32 + ctrl_len + data_len; - estream.avail_in = patch->size - (patch_offset + 32 + ctrl_len + data_len); + estream.next_in = const_cast(&patch->data[patch_offset + 32 + ctrl_len + data_len]); + estream.avail_in = patch->data.size() - (patch_offset + 32 + ctrl_len + data_len); estream.bzalloc = NULL; estream.bzfree = NULL; estream.opaque = NULL; diff --git a/applypatch/imgpatch.cpp b/applypatch/imgpatch.cpp index 0c06d6b1e..1c4409e36 100644 --- a/applypatch/imgpatch.cpp +++ b/applypatch/imgpatch.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include "zlib.h" @@ -35,10 +36,10 @@ int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size, const unsigned char* patch_data, ssize_t patch_size, SinkFn sink, void* token) { - Value patch = {VAL_BLOB, patch_size, - reinterpret_cast(const_cast(patch_data))}; - return ApplyImagePatch( - old_data, old_size, &patch, sink, token, nullptr, nullptr); + Value patch(VAL_BLOB, std::string( + reinterpret_cast(patch_data), patch_size)); + + return ApplyImagePatch(old_data, old_size, &patch, sink, token, nullptr, nullptr); } /* @@ -51,9 +52,7 @@ 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) { - ssize_t pos = 12; - char* header = patch->data; - if (patch->size < 12) { + if (patch->data.size() < 12) { printf("patch too short to contain header\n"); return -1; } @@ -61,6 +60,8 @@ int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size, // IMGDIFF2 uses CHUNK_NORMAL, CHUNK_DEFLATE, and CHUNK_RAW. // (IMGDIFF1, which is no longer supported, used CHUNK_NORMAL and // CHUNK_GZIP.) + size_t pos = 12; + const char* header = &patch->data[0]; if (memcmp(header, "IMGDIFF2", 8) != 0) { printf("corrupt patch file header (magic number)\n"); return -1; @@ -68,20 +69,19 @@ int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size, int num_chunks = Read4(header+8); - int i; - for (i = 0; i < num_chunks; ++i) { + for (int i = 0; i < num_chunks; ++i) { // each chunk's header record starts with 4 bytes. - if (pos + 4 > patch->size) { + if (pos + 4 > patch->data.size()) { printf("failed to read chunk %d record\n", i); return -1; } - int type = Read4(patch->data + pos); + int type = Read4(&patch->data[pos]); pos += 4; if (type == CHUNK_NORMAL) { - char* normal_header = patch->data + pos; + const char* normal_header = &patch->data[pos]; pos += 24; - if (pos > patch->size) { + if (pos > patch->data.size()) { printf("failed to read chunk %d normal header data\n", i); return -1; } @@ -97,21 +97,21 @@ int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size, ApplyBSDiffPatch(old_data + src_start, src_len, patch, patch_offset, sink, token, ctx); } else if (type == CHUNK_RAW) { - char* raw_header = patch->data + pos; + const char* raw_header = &patch->data[pos]; pos += 4; - if (pos > patch->size) { + if (pos > patch->data.size()) { printf("failed to read chunk %d raw header data\n", i); return -1; } ssize_t data_len = Read4(raw_header); - if (pos + data_len > patch->size) { + if (pos + data_len > patch->data.size()) { printf("failed to read chunk %d raw data\n", i); return -1; } - if (ctx) SHA1_Update(ctx, patch->data + pos, data_len); - if (sink((unsigned char*)patch->data + pos, + if (ctx) SHA1_Update(ctx, &patch->data[pos], data_len); + if (sink(reinterpret_cast(&patch->data[pos]), data_len, token) != data_len) { printf("failed to write chunk %d raw data\n", i); return -1; @@ -119,9 +119,9 @@ int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size, pos += data_len; } else if (type == CHUNK_DEFLATE) { // deflate chunks have an additional 60 bytes in their chunk header. - char* deflate_header = patch->data + pos; + const char* deflate_header = &patch->data[pos]; pos += 60; - if (pos > patch->size) { + if (pos > patch->data.size()) { printf("failed to read chunk %d deflate header data\n", i); return -1; } @@ -149,7 +149,7 @@ int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size, // the patch was constructed with bonus data. The // deflation will come up 'bonus_size' bytes short; these // must be appended from the bonus_data value. - size_t bonus_size = (i == 1 && bonus_data != NULL) ? bonus_data->size : 0; + size_t bonus_size = (i == 1 && bonus_data != NULL) ? bonus_data->data.size() : 0; std::vector expanded_source(expanded_len); @@ -189,7 +189,7 @@ int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size, if (bonus_size) { memcpy(expanded_source.data() + (expanded_len - bonus_size), - bonus_data->data, bonus_size); + &bonus_data->data[0], bonus_size); } } diff --git a/applypatch/include/applypatch/applypatch.h b/applypatch/include/applypatch/applypatch.h index 9ee39d293..80d681638 100644 --- a/applypatch/include/applypatch/applypatch.h +++ b/applypatch/include/applypatch/applypatch.h @@ -20,6 +20,7 @@ #include #include +#include #include #include "openssl/sha.h" @@ -52,19 +53,16 @@ int applypatch(const char* source_filename, const char* target_filename, const char* target_sha1_str, size_t target_size, - int num_patches, - char** const patch_sha1_str, + const std::vector& patch_sha1_str, Value** patch_data, Value* bonus_data); int applypatch_check(const char* filename, - int num_patches, - char** const patch_sha1_str); + const std::vector& patch_sha1_str); int LoadFileContents(const char* filename, FileContents* file); int SaveFileContents(const char* filename, const FileContents* file); void FreeFileContents(FileContents* file); -int FindMatchingPatch(uint8_t* sha1, char* const * const patch_sha1_str, - int num_patches); +int FindMatchingPatch(uint8_t* sha1, const std::vector& patch_sha1_str); // bsdiff.cpp void ShowBSDiffLicense(); diff --git a/applypatch/main.cpp b/applypatch/main.cpp index 1968ae48f..a3a45d06f 100644 --- a/applypatch/main.cpp +++ b/applypatch/main.cpp @@ -20,6 +20,7 @@ #include #include +#include #include #include "applypatch/applypatch.h" @@ -30,7 +31,12 @@ static int CheckMode(int argc, char** argv) { if (argc < 3) { return 2; } - return applypatch_check(argv[2], argc-3, argv+3); + std::vector sha1; + for (int i = 3; i < argc; i++) { + sha1.push_back(argv[i]); + } + + return applypatch_check(argv[2], sha1); } static int SpaceMode(int argc, char** argv) { @@ -49,11 +55,13 @@ static int SpaceMode(int argc, char** argv) { // Parse arguments (which should be of the form ":" // into the new parallel arrays *sha1s and *files.Returns true on // success. -static bool ParsePatchArgs(int argc, char** argv, std::vector* sha1s, +static bool ParsePatchArgs(int argc, char** argv, std::vector* sha1s, std::vector* files) { - uint8_t digest[SHA_DIGEST_LENGTH]; - + if (sha1s == nullptr) { + return false; + } for (int i = 0; i < argc; ++i) { + uint8_t digest[SHA_DIGEST_LENGTH]; char* colon = strchr(argv[i], ':'); if (colon == nullptr) { printf("no ':' in patch argument \"%s\"\n", argv[i]); @@ -83,18 +91,15 @@ static int FlashMode(const char* src_filename, const char* tgt_filename, static int PatchMode(int argc, char** argv) { FileContents bonusFc; - Value bonusValue; - Value* bonus = nullptr; + Value bonus(VAL_INVALID, ""); if (argc >= 3 && strcmp(argv[1], "-b") == 0) { if (LoadFileContents(argv[2], &bonusFc) != 0) { printf("failed to load bonus file %s\n", argv[2]); return 1; } - bonus = &bonusValue; - bonus->type = VAL_BLOB; - bonus->size = bonusFc.data.size(); - bonus->data = reinterpret_cast(bonusFc.data.data()); + bonus.type = VAL_BLOB; + bonus.data = reinterpret_cast(bonusFc.data.data()); argc -= 2; argv += 2; } @@ -112,29 +117,26 @@ static int PatchMode(int argc, char** argv) { // If no : is provided, it is in flash mode. if (argc == 5) { - if (bonus != nullptr) { + if (bonus.type != VAL_INVALID) { printf("bonus file not supported in flash mode\n"); return 1; } return FlashMode(argv[1], argv[2], argv[3], target_size); } - std::vector sha1s; + std::vector sha1s; std::vector files; if (!ParsePatchArgs(argc-5, argv+5, &sha1s, &files)) { printf("failed to parse patch args\n"); return 1; } - std::vector patches(files.size()); - std::vector patch_ptrs(files.size()); + std::vector patches; + std::vector patch_ptrs; for (size_t i = 0; i < files.size(); ++i) { - patches[i].type = VAL_BLOB; - patches[i].size = files[i].data.size(); - patches[i].data = reinterpret_cast(files[i].data.data()); - patch_ptrs[i] = &patches[i]; + patches.push_back(Value(VAL_BLOB, reinterpret_cast(files[i].data.data()))); + patch_ptrs.push_back(&patches[i]); } return applypatch(argv[1], argv[2], argv[3], target_size, - patch_ptrs.size(), sha1s.data(), - patch_ptrs.data(), bonus); + sha1s, patch_ptrs.data(), &bonus); } // This program applies binary patches to files in a way that is safe diff --git a/applypatch/utils.cpp b/applypatch/utils.cpp index fef250f01..450dc8d76 100644 --- a/applypatch/utils.cpp +++ b/applypatch/utils.cpp @@ -38,22 +38,22 @@ void Write8(int64_t value, FILE* f) { fputc((value >> 56) & 0xff, f); } -int Read2(void* pv) { - unsigned char* p = reinterpret_cast(pv); +int Read2(const void* pv) { + const unsigned char* p = reinterpret_cast(pv); return (int)(((unsigned int)p[1] << 8) | (unsigned int)p[0]); } -int Read4(void* pv) { - unsigned char* p = reinterpret_cast(pv); +int Read4(const void* pv) { + const unsigned char* p = reinterpret_cast(pv); return (int)(((unsigned int)p[3] << 24) | ((unsigned int)p[2] << 16) | ((unsigned int)p[1] << 8) | (unsigned int)p[0]); } -int64_t Read8(void* pv) { - unsigned char* p = reinterpret_cast(pv); +int64_t Read8(const void* pv) { + const unsigned char* p = reinterpret_cast(pv); return (int64_t)(((uint64_t)p[7] << 56) | ((uint64_t)p[6] << 48) | ((uint64_t)p[5] << 40) | diff --git a/applypatch/utils.h b/applypatch/utils.h index 1c34edd97..c7c8e90e2 100644 --- a/applypatch/utils.h +++ b/applypatch/utils.h @@ -24,8 +24,8 @@ void Write4(int value, FILE* f); void Write8(int64_t value, FILE* f); -int Read2(void* p); -int Read4(void* p); -int64_t Read8(void* p); +int Read2(const void* p); +int Read4(const void* p); +int64_t Read8(const void* p); #endif // _BUILD_TOOLS_APPLYPATCH_UTILS_H diff --git a/edify/edify_parser.cpp b/edify/edify_parser.cpp index 1b89cf21d..908fcf13b 100644 --- a/edify/edify_parser.cpp +++ b/edify/edify_parser.cpp @@ -66,12 +66,12 @@ int main(int argc, char** argv) { ExprDump(0, root, buffer); State state(buffer, nullptr); - char* result = Evaluate(&state, root); - if (result == NULL) { + std::string result; + if (!Evaluate(&state, root, &result)) { printf("result was NULL, message is: %s\n", (state.errmsg.empty() ? "(NULL)" : state.errmsg.c_str())); } else { - printf("result is [%s]\n", result); + printf("result is [%s]\n", result.c_str()); } } return 0; diff --git a/edify/expr.cpp b/edify/expr.cpp index 4000bc4db..ec2409752 100644 --- a/edify/expr.cpp +++ b/edify/expr.cpp @@ -22,184 +22,164 @@ #include #include +#include #include #include +#include +#include #include #include // Functions should: // // - return a malloc()'d string -// - if Evaluate() on any argument returns NULL, return NULL. +// - if Evaluate() on any argument returns nullptr, return nullptr. -int BooleanString(const char* s) { - return s[0] != '\0'; +static bool BooleanString(const std::string& s) { + return !s.empty(); } -char* Evaluate(State* state, Expr* expr) { - Value* v = expr->fn(expr->name, state, expr->argc, expr->argv); - if (v == NULL) return NULL; +bool Evaluate(State* state, Expr* expr, std::string* result) { + if (result == nullptr) { + return false; + } + + std::unique_ptr v(expr->fn(expr->name, state, expr->argc, expr->argv)); + if (!v) { + return false; + } if (v->type != VAL_STRING) { ErrorAbort(state, kArgsParsingFailure, "expecting string, got value type %d", v->type); - FreeValue(v); - return NULL; + return false; } - char* result = v->data; - free(v); - return result; + + *result = v->data; + return true; } Value* EvaluateValue(State* state, Expr* expr) { return expr->fn(expr->name, state, expr->argc, expr->argv); } -Value* StringValue(char* str) { - if (str == NULL) return NULL; - Value* v = reinterpret_cast(malloc(sizeof(Value))); - v->type = VAL_STRING; - v->size = strlen(str); - v->data = str; - return v; +Value* StringValue(const char* str) { + if (str == nullptr) { + return nullptr; + } + return new Value(VAL_STRING, str); } -void FreeValue(Value* v) { - if (v == NULL) return; - free(v->data); - free(v); +Value* StringValue(const std::string& str) { + return StringValue(str.c_str()); } Value* ConcatFn(const char* name, State* state, int argc, Expr* argv[]) { if (argc == 0) { - return StringValue(strdup("")); + return StringValue(""); } - char** strings = reinterpret_cast(malloc(argc * sizeof(char*))); - int i; - for (i = 0; i < argc; ++i) { - strings[i] = NULL; - } - char* result = NULL; - int length = 0; - for (i = 0; i < argc; ++i) { - strings[i] = Evaluate(state, argv[i]); - if (strings[i] == NULL) { - goto done; + std::string result; + for (int i = 0; i < argc; ++i) { + std::string str; + if (!Evaluate(state, argv[i], &str)) { + return nullptr; } - length += strlen(strings[i]); + result += str; } - result = reinterpret_cast(malloc(length+1)); - int p; - p = 0; - for (i = 0; i < argc; ++i) { - strcpy(result+p, strings[i]); - p += strlen(strings[i]); - } - result[p] = '\0'; - - done: - for (i = 0; i < argc; ++i) { - free(strings[i]); - } - free(strings); return StringValue(result); } Value* IfElseFn(const char* name, State* state, int argc, Expr* argv[]) { if (argc != 2 && argc != 3) { state->errmsg = "ifelse expects 2 or 3 arguments"; - return NULL; + return nullptr; } - char* cond = Evaluate(state, argv[0]); - if (cond == NULL) { - return NULL; + + std::string cond; + if (!Evaluate(state, argv[0], &cond)) { + return nullptr; } - if (BooleanString(cond) == true) { - free(cond); + if (!cond.empty()) { return EvaluateValue(state, argv[1]); - } else { - if (argc == 3) { - free(cond); - return EvaluateValue(state, argv[2]); - } else { - return StringValue(cond); - } + } else if (argc == 3) { + return EvaluateValue(state, argv[2]); } + + return StringValue(""); } Value* AbortFn(const char* name, State* state, int argc, Expr* argv[]) { - char* msg = NULL; - if (argc > 0) { - msg = Evaluate(state, argv[0]); - } - if (msg) { + std::string msg; + if (argc > 0 && Evaluate(state, argv[0], &msg)) { state->errmsg = msg; } else { state->errmsg = "called abort()"; } - return NULL; + return nullptr; } Value* AssertFn(const char* name, State* state, int argc, Expr* argv[]) { - int i; - for (i = 0; i < argc; ++i) { - char* v = Evaluate(state, argv[i]); - if (v == NULL) { - return NULL; + for (int i = 0; i < argc; ++i) { + std::string result; + if (!Evaluate(state, argv[i], &result)) { + return nullptr; } - int b = BooleanString(v); - free(v); - if (!b) { + if (result.empty()) { int len = argv[i]->end - argv[i]->start; state->errmsg = "assert failed: " + state->script.substr(argv[i]->start, len); - return NULL; + return nullptr; } } - return StringValue(strdup("")); + return StringValue(""); } Value* SleepFn(const char* name, State* state, int argc, Expr* argv[]) { - char* val = Evaluate(state, argv[0]); - if (val == NULL) { - return NULL; + std::string val; + if (!Evaluate(state, argv[0], &val)) { + return nullptr; + } + + int v; + if (!android::base::ParseInt(val.c_str(), &v, 0)) { + return nullptr; } - int v = strtol(val, NULL, 10); sleep(v); + return StringValue(val); } Value* StdoutFn(const char* name, State* state, int argc, Expr* argv[]) { - int i; - for (i = 0; i < argc; ++i) { - char* v = Evaluate(state, argv[i]); - if (v == NULL) { - return NULL; + for (int i = 0; i < argc; ++i) { + std::string v; + if (!Evaluate(state, argv[i], &v)) { + return nullptr; } - fputs(v, stdout); - free(v); + fputs(v.c_str(), stdout); } - return StringValue(strdup("")); + return StringValue(""); } Value* LogicalAndFn(const char* name, State* state, int argc, Expr* argv[]) { - char* left = Evaluate(state, argv[0]); - if (left == NULL) return NULL; - if (BooleanString(left) == true) { - free(left); + std::string left; + if (!Evaluate(state, argv[0], &left)) { + return nullptr; + } + if (BooleanString(left)) { return EvaluateValue(state, argv[1]); } else { - return StringValue(left); + return StringValue(""); } } Value* LogicalOrFn(const char* name, State* state, int argc, Expr* argv[]) { - char* left = Evaluate(state, argv[0]); - if (left == NULL) return NULL; - if (BooleanString(left) == false) { - free(left); + std::string left; + if (!Evaluate(state, argv[0], &left)) { + return nullptr; + } + if (!BooleanString(left)) { return EvaluateValue(state, argv[1]); } else { return StringValue(left); @@ -208,75 +188,75 @@ Value* LogicalOrFn(const char* name, State* state, Value* LogicalNotFn(const char* name, State* state, int argc, Expr* argv[]) { - char* val = Evaluate(state, argv[0]); - if (val == NULL) return NULL; - bool bv = BooleanString(val); - free(val); - return StringValue(strdup(bv ? "" : "t")); + std::string val; + if (!Evaluate(state, argv[0], &val)) { + return nullptr; + } + + return StringValue(BooleanString(val) ? "" : "t"); } Value* SubstringFn(const char* name, State* state, int argc, Expr* argv[]) { - char* needle = Evaluate(state, argv[0]); - if (needle == NULL) return NULL; - char* haystack = Evaluate(state, argv[1]); - if (haystack == NULL) { - free(needle); - return NULL; + std::string needle; + if (!Evaluate(state, argv[0], &needle)) { + return nullptr; + } + + std::string haystack; + if (!Evaluate(state, argv[1], &haystack)) { + return nullptr; } - char* result = strdup(strstr(haystack, needle) ? "t" : ""); - free(needle); - free(haystack); + std::string result = (haystack.find(needle) != std::string::npos) ? "t" : ""; return StringValue(result); } Value* EqualityFn(const char* name, State* state, int argc, Expr* argv[]) { - char* left = Evaluate(state, argv[0]); - if (left == NULL) return NULL; - char* right = Evaluate(state, argv[1]); - if (right == NULL) { - free(left); - return NULL; + std::string left; + if (!Evaluate(state, argv[0], &left)) { + return nullptr; + } + std::string right; + if (!Evaluate(state, argv[1], &right)) { + return nullptr; } - char* result = strdup(strcmp(left, right) == 0 ? "t" : ""); - free(left); - free(right); + const char* result = (left == right) ? "t" : ""; return StringValue(result); } Value* InequalityFn(const char* name, State* state, int argc, Expr* argv[]) { - char* left = Evaluate(state, argv[0]); - if (left == NULL) return NULL; - char* right = Evaluate(state, argv[1]); - if (right == NULL) { - free(left); - return NULL; + std::string left; + if (!Evaluate(state, argv[0], &left)) { + return nullptr; + } + std::string right; + if (!Evaluate(state, argv[1], &right)) { + return nullptr; } - char* result = strdup(strcmp(left, right) != 0 ? "t" : ""); - free(left); - free(right); + const char* result = (left != right) ? "t" : ""; return StringValue(result); } Value* SequenceFn(const char* name, State* state, int argc, Expr* argv[]) { - Value* left = EvaluateValue(state, argv[0]); - if (left == NULL) return NULL; - FreeValue(left); + std::unique_ptr left(EvaluateValue(state, argv[0])); + if (!left) { + return nullptr; + } return EvaluateValue(state, argv[1]); } Value* LessThanIntFn(const char* name, State* state, int argc, Expr* argv[]) { if (argc != 2) { state->errmsg = "less_than_int expects 2 arguments"; - return NULL; + return nullptr; } char* left; char* right; - if (ReadArgs(state, argv, 2, &left, &right) < 0) return NULL; + if (ReadArgs(state, argv, 2, &left, &right) < 0) return nullptr; bool result = false; char* end; @@ -298,14 +278,14 @@ Value* LessThanIntFn(const char* name, State* state, int argc, Expr* argv[]) { done: free(left); free(right); - return StringValue(strdup(result ? "t" : "")); + return StringValue(result ? "t" : ""); } Value* GreaterThanIntFn(const char* name, State* state, int argc, Expr* argv[]) { if (argc != 2) { state->errmsg = "greater_than_int expects 2 arguments"; - return NULL; + return nullptr; } Expr* temp[2]; @@ -316,7 +296,7 @@ Value* GreaterThanIntFn(const char* name, State* state, } Value* Literal(const char* name, State* state, int argc, Expr* argv[]) { - return StringValue(strdup(name)); + return StringValue(name); } // ----------------------------------------------------------------- @@ -355,6 +335,43 @@ void RegisterBuiltins() { // convenience methods for functions // ----------------------------------------------------------------- +// Evaluate the expressions in argv, and put the results of strings in +// args. If any expression evaluates to nullptr, free the rest and return +// false. Return true on success. +bool ReadArgs(State* state, int argc, Expr* argv[], std::vector* args) { + if (args == nullptr) { + return false; + } + for (int i = 0; i < argc; ++i) { + std::string var; + if (!Evaluate(state, argv[i], &var)) { + args->clear(); + return false; + } + args->push_back(var); + } + return true; +} + +// Evaluate the expressions in argv, and put the results of Value* in +// args. If any expression evaluate to nullptr, free the rest and return +// false. Return true on success. +bool ReadValueArgs(State* state, int argc, Expr* argv[], + std::vector>* args) { + if (args == nullptr) { + return false; + } + for (int i = 0; i < argc; ++i) { + std::unique_ptr v(EvaluateValue(state, argv[i])); + if (!v) { + args->clear(); + return false; + } + args->push_back(std::move(v)); + } + return true; +} + // Evaluate the expressions in argv, giving 'count' char* (the ... is // zero or more char** to put them in). If any expression evaluates // to NULL, free the rest and return -1. Return 0 on success. @@ -364,8 +381,9 @@ int ReadArgs(State* state, Expr* argv[], int count, ...) { va_start(v, count); int i; for (i = 0; i < count; ++i) { - args[i] = Evaluate(state, argv[i]); - if (args[i] == NULL) { + std::string str; + if (!Evaluate(state, argv[i], &str) || + (args[i] = strdup(str.c_str())) == nullptr) { va_end(v); int j; for (j = 0; j < i; ++j) { @@ -385,25 +403,24 @@ int ReadArgs(State* state, Expr* argv[], int count, ...) { // zero or more Value** to put them in). If any expression evaluates // to NULL, free the rest and return -1. Return 0 on success. int ReadValueArgs(State* state, Expr* argv[], int count, ...) { - Value** args = reinterpret_cast(malloc(count * sizeof(Value*))); + Value** args = new Value*[count]; va_list v; va_start(v, count); - int i; - for (i = 0; i < count; ++i) { + for (int i = 0; i < count; ++i) { args[i] = EvaluateValue(state, argv[i]); if (args[i] == NULL) { va_end(v); int j; for (j = 0; j < i; ++j) { - FreeValue(args[j]); + delete args[j]; } - free(args); + delete[] args; return -1; } *(va_arg(v, Value**)) = args[i]; } va_end(v); - free(args); + delete[] args; return 0; } @@ -413,12 +430,11 @@ int ReadValueArgs(State* state, Expr* argv[], int count, ...) { // strings it contains. char** ReadVarArgs(State* state, int argc, Expr* argv[]) { char** args = (char**)malloc(argc * sizeof(char*)); - int i = 0; - for (i = 0; i < argc; ++i) { - args[i] = Evaluate(state, argv[i]); - if (args[i] == NULL) { - int j; - for (j = 0; j < i; ++j) { + for (int i = 0; i < argc; ++i) { + std::string str; + if (!Evaluate(state, argv[i], &str) || + (args[i] = strdup(str.c_str())) == nullptr) { + for (int j = 0; j < i; ++j) { free(args[j]); } free(args); @@ -433,16 +449,16 @@ char** ReadVarArgs(State* state, int argc, Expr* argv[]) { // The caller is responsible for freeing the returned array and the // Values it contains. Value** ReadValueVarArgs(State* state, int argc, Expr* argv[]) { - Value** args = (Value**)malloc(argc * sizeof(Value*)); + Value** args = new Value*[argc]; int i = 0; for (i = 0; i < argc; ++i) { args[i] = EvaluateValue(state, argv[i]); if (args[i] == NULL) { int j; for (j = 0; j < i; ++j) { - FreeValue(args[j]); + delete args[j]; } - free(args); + delete[] args; return NULL; } } diff --git a/edify/expr.h b/edify/expr.h index cd6139a18..85306542d 100644 --- a/edify/expr.h +++ b/edify/expr.h @@ -48,13 +48,19 @@ struct State { bool is_retry = false; }; -#define VAL_STRING 1 // data will be NULL-terminated; size doesn't count null -#define VAL_BLOB 2 +enum ValueType { + VAL_INVALID = -1, + VAL_STRING = 1, + VAL_BLOB = 2, +}; struct Value { - int type; - ssize_t size; - char* data; + ValueType type; + std::string data; + + Value(ValueType type, const std::string& str) : + type(type), + data(str) {} }; struct Expr; @@ -75,11 +81,11 @@ struct Expr { Value* EvaluateValue(State* state, Expr* expr); // Take one of the Expr*s passed to the function as an argument, -// evaluate it, assert that it is a string, and return the resulting -// char*. The caller takes ownership of the returned char*. This is -// a convenience function for older functions that want to deal only -// with strings. -char* Evaluate(State* state, Expr* expr); +// evaluate it, assert that it is a string, and update the result +// parameter. This function returns true if the evaluation succeeds. +// This is a convenience function for older functions that want to +// deal only with strings. +bool Evaluate(State* state, Expr* expr, std::string* result); // Glue to make an Expr out of a literal. Value* Literal(const char* name, State* state, int argc, Expr* argv[]); @@ -114,6 +120,17 @@ Function FindFunction(const std::string& name); // --- convenience functions for use in functions --- +// Evaluate the expressions in argv, and put the results of strings in +// args. If any expression evaluates to nullptr, free the rest and return +// false. Return true on success. +bool ReadArgs(State* state, int argc, Expr* argv[], std::vector* args); + +// Evaluate the expressions in argv, and put the results of Value* in +// args. If any expression evaluate to nullptr, free the rest and return +// false. Return true on success. +bool ReadValueArgs(State* state, int argc, Expr* argv[], + std::vector>* args); + // Evaluate the expressions in argv, giving 'count' char* (the ... is // zero or more char** to put them in). If any expression evaluates // to NULL, free the rest and return -1. Return 0 on success. @@ -146,11 +163,10 @@ Value* ErrorAbort(State* state, const char* format, ...) Value* ErrorAbort(State* state, CauseCode cause_code, const char* format, ...) __attribute__((format(printf, 3, 4))); -// Wrap a string into a Value, taking ownership of the string. -Value* StringValue(char* str); +// Copying the string into a Value. +Value* StringValue(const char* str); -// Free a Value object. -void FreeValue(Value* v); +Value* StringValue(const std::string& str); int parse_string(const char* str, Expr** root, int* error_count); diff --git a/tests/component/applypatch_test.cpp b/tests/component/applypatch_test.cpp index 2f7cb02f4..75f2e1014 100644 --- a/tests/component/applypatch_test.cpp +++ b/tests/component/applypatch_test.cpp @@ -153,25 +153,16 @@ class ApplyPatchFullTest : public ApplyPatchCacheTest { struct FileContents fc; ASSERT_EQ(0, LoadFileContents(&rand_file[0], &fc)); - Value* patch1 = new Value(); - patch1->type = VAL_BLOB; - patch1->size = fc.data.size(); - patch1->data = static_cast(malloc(fc.data.size())); - memcpy(patch1->data, fc.data.data(), fc.data.size()); + Value* patch1 = new Value(VAL_BLOB, std::string(fc.data.begin(), fc.data.end())); patches.push_back(patch1); ASSERT_EQ(0, LoadFileContents(&patch_file[0], &fc)); - Value* patch2 = new Value(); - patch2->type = VAL_BLOB; - patch2->size = fc.st.st_size; - patch2->data = static_cast(malloc(fc.data.size())); - memcpy(patch2->data, fc.data.data(), fc.data.size()); + Value* patch2 = new Value(VAL_BLOB, std::string(fc.data.begin(), fc.data.end())); patches.push_back(patch2); } static void TearDownTestCase() { delete output_f; for (auto it = patches.begin(); it != patches.end(); ++it) { - free((*it)->data); delete *it; } patches.clear(); @@ -210,88 +201,87 @@ TemporaryFile* ApplyPatchFullTest::output_f; std::string ApplyPatchFullTest::output_loc; TEST_F(ApplyPatchTest, CheckModeSingle) { - char* s = &old_sha1[0]; - ASSERT_EQ(0, applypatch_check(&old_file[0], 1, &s)); + std::vector sha1s = { old_sha1 }; + ASSERT_EQ(0, applypatch_check(&old_file[0], sha1s)); } TEST_F(ApplyPatchTest, CheckModeMultiple) { - char* argv[3] = { - &bad_sha1_a[0], - &old_sha1[0], - &bad_sha1_b[0] + std::vector sha1s = { + bad_sha1_a, + old_sha1, + bad_sha1_b }; - ASSERT_EQ(0, applypatch_check(&old_file[0], 3, argv)); + ASSERT_EQ(0, applypatch_check(&old_file[0], sha1s)); } TEST_F(ApplyPatchTest, CheckModeFailure) { - char* argv[2] = { - &bad_sha1_a[0], - &bad_sha1_b[0] + std::vector sha1s = { + bad_sha1_a, + bad_sha1_b }; - ASSERT_NE(0, applypatch_check(&old_file[0], 2, argv)); + ASSERT_NE(0, applypatch_check(&old_file[0], sha1s)); } TEST_F(ApplyPatchCacheTest, CheckCacheCorruptedSingle) { mangle_file(old_file); - char* s = &old_sha1[0]; - ASSERT_EQ(0, applypatch_check(&old_file[0], 1, &s)); + std::vector sha1s = { old_sha1 }; + ASSERT_EQ(0, applypatch_check(&old_file[0], sha1s)); } TEST_F(ApplyPatchCacheTest, CheckCacheCorruptedMultiple) { mangle_file(old_file); - char* argv[3] = { - &bad_sha1_a[0], - &old_sha1[0], - &bad_sha1_b[0] + std::vector sha1s = { + bad_sha1_a, + old_sha1, + bad_sha1_b }; - ASSERT_EQ(0, applypatch_check(&old_file[0], 3, argv)); + ASSERT_EQ(0, applypatch_check(&old_file[0], sha1s)); } TEST_F(ApplyPatchCacheTest, CheckCacheCorruptedFailure) { mangle_file(old_file); - char* argv[2] = { - &bad_sha1_a[0], - &bad_sha1_b[0] + std::vector sha1s = { + bad_sha1_a, + bad_sha1_b }; - ASSERT_NE(0, applypatch_check(&old_file[0], 2, argv)); + ASSERT_NE(0, applypatch_check(&old_file[0], sha1s)); } TEST_F(ApplyPatchCacheTest, CheckCacheMissingSingle) { unlink(&old_file[0]); - char* s = &old_sha1[0]; - ASSERT_EQ(0, applypatch_check(&old_file[0], 1, &s)); + std::vector sha1s = { old_sha1 }; + ASSERT_EQ(0, applypatch_check(&old_file[0], sha1s)); } TEST_F(ApplyPatchCacheTest, CheckCacheMissingMultiple) { unlink(&old_file[0]); - char* argv[3] = { - &bad_sha1_a[0], - &old_sha1[0], - &bad_sha1_b[0] + std::vector sha1s = { + bad_sha1_a, + old_sha1, + bad_sha1_b }; - ASSERT_EQ(0, applypatch_check(&old_file[0], 3, argv)); + ASSERT_EQ(0, applypatch_check(&old_file[0], sha1s)); } TEST_F(ApplyPatchCacheTest, CheckCacheMissingFailure) { unlink(&old_file[0]); - char* argv[2] = { - &bad_sha1_a[0], - &bad_sha1_b[0] + std::vector sha1s = { + bad_sha1_a, + bad_sha1_b }; - ASSERT_NE(0, applypatch_check(&old_file[0], 2, argv)); + ASSERT_NE(0, applypatch_check(&old_file[0], sha1s)); } TEST_F(ApplyPatchFullTest, ApplyInPlace) { - std::vector sha1s; - sha1s.push_back(&bad_sha1_a[0]); - sha1s.push_back(&old_sha1[0]); - + std::vector sha1s = { + bad_sha1_a, + old_sha1 + }; int ap_result = applypatch(&old_file[0], "-", &new_sha1[0], new_size, - 2, - sha1s.data(), + sha1s, patches.data(), nullptr); ASSERT_EQ(0, ap_result); @@ -301,8 +291,7 @@ TEST_F(ApplyPatchFullTest, ApplyInPlace) { "-", &new_sha1[0], new_size, - 2, - sha1s.data(), + sha1s, patches.data(), nullptr); ASSERT_EQ(0, ap_result); @@ -310,15 +299,15 @@ TEST_F(ApplyPatchFullTest, ApplyInPlace) { } TEST_F(ApplyPatchFullTest, ApplyInNewLocation) { - std::vector sha1s; - sha1s.push_back(&bad_sha1_a[0]); - sha1s.push_back(&old_sha1[0]); + std::vector sha1s = { + bad_sha1_a, + old_sha1 + }; int ap_result = applypatch(&old_file[0], &output_loc[0], &new_sha1[0], new_size, - 2, - sha1s.data(), + sha1s, patches.data(), nullptr); ASSERT_EQ(0, ap_result); @@ -327,8 +316,7 @@ TEST_F(ApplyPatchFullTest, ApplyInNewLocation) { &output_loc[0], &new_sha1[0], new_size, - 2, - sha1s.data(), + sha1s, patches.data(), nullptr); ASSERT_EQ(0, ap_result); @@ -337,15 +325,15 @@ TEST_F(ApplyPatchFullTest, ApplyInNewLocation) { TEST_F(ApplyPatchFullTest, ApplyCorruptedInNewLocation) { mangle_file(old_file); - std::vector sha1s; - sha1s.push_back(&bad_sha1_a[0]); - sha1s.push_back(&old_sha1[0]); + std::vector sha1s = { + bad_sha1_a, + old_sha1 + }; int ap_result = applypatch(&old_file[0], &output_loc[0], &new_sha1[0], new_size, - 2, - sha1s.data(), + sha1s, patches.data(), nullptr); ASSERT_EQ(0, ap_result); @@ -354,8 +342,7 @@ TEST_F(ApplyPatchFullTest, ApplyCorruptedInNewLocation) { &output_loc[0], &new_sha1[0], new_size, - 2, - sha1s.data(), + sha1s, patches.data(), nullptr); ASSERT_EQ(0, ap_result); @@ -366,15 +353,15 @@ TEST_F(ApplyPatchDoubleCacheTest, ApplyDoubleCorruptedInNewLocation) { mangle_file(old_file); mangle_file(cache_file); - std::vector sha1s; - sha1s.push_back(&bad_sha1_a[0]); - sha1s.push_back(&old_sha1[0]); + std::vector sha1s = { + bad_sha1_a, + old_sha1 + }; int ap_result = applypatch(&old_file[0], &output_loc[0], &new_sha1[0], new_size, - 2, - sha1s.data(), + sha1s, patches.data(), nullptr); ASSERT_NE(0, ap_result); @@ -383,8 +370,7 @@ TEST_F(ApplyPatchDoubleCacheTest, ApplyDoubleCorruptedInNewLocation) { &output_loc[0], &new_sha1[0], new_size, - 2, - sha1s.data(), + sha1s, patches.data(), nullptr); ASSERT_NE(0, ap_result); diff --git a/tests/component/edify_test.cpp b/tests/component/edify_test.cpp index a4dbb9fbe..287e40cc6 100644 --- a/tests/component/edify_test.cpp +++ b/tests/component/edify_test.cpp @@ -28,15 +28,15 @@ static void expect(const char* expr_str, const char* expected) { State state(expr_str, nullptr); - char* result = Evaluate(&state, e); + std::string result; + bool status = Evaluate(&state, e, &result); if (expected == nullptr) { - EXPECT_EQ(nullptr, result); + EXPECT_FALSE(status); } else { - EXPECT_STREQ(expected, result); + EXPECT_STREQ(expected, result.c_str()); } - free(result); } class EdifyTest : public ::testing::Test { diff --git a/tests/component/updater_test.cpp b/tests/component/updater_test.cpp index 64a6b37ce..a859f11c1 100644 --- a/tests/component/updater_test.cpp +++ b/tests/component/updater_test.cpp @@ -32,12 +32,13 @@ static void expect(const char* expected, const char* expr_str, CauseCode cause_c State state(expr_str, nullptr); - char* result = Evaluate(&state, e); + std::string result; + bool status = Evaluate(&state, e, &result); if (expected == nullptr) { - EXPECT_EQ(nullptr, result); + EXPECT_FALSE(status); } else { - EXPECT_STREQ(expected, result); + EXPECT_STREQ(expected, result.c_str()); } // Error code is set in updater/updater.cpp only, by parsing State.errmsg. @@ -46,7 +47,6 @@ static void expect(const char* expected, const char* expr_str, CauseCode cause_c // Cause code should always be available. EXPECT_EQ(cause_code, state.cause_code); - free(result); } class UpdaterTest : public ::testing::Test { diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp index 433d98026..5f9b437fe 100644 --- a/updater/blockimg.cpp +++ b/updater/blockimg.cpp @@ -1216,7 +1216,7 @@ static int PerformCommandDiff(CommandParameters& params) { size_t len; if (!android::base::ParseUint(params.tokens[params.cpos++].c_str(), &len)) { - fprintf(stderr, "invalid patch offset\n"); + fprintf(stderr, "invalid patch len\n"); return -1; } @@ -1248,10 +1248,8 @@ static int PerformCommandDiff(CommandParameters& params) { if (status == 0) { fprintf(stderr, "patching %zu blocks to %zu\n", blocks, tgt.size); - Value patch_value; - patch_value.type = VAL_BLOB; - patch_value.size = len; - patch_value.data = (char*) (params.patch_start + offset); + Value patch_value(VAL_BLOB, + std::string(reinterpret_cast(params.patch_start + offset), len)); RangeSinkState rss(tgt); rss.fd = params.fd; @@ -1398,64 +1396,62 @@ static Value* PerformBlockImageUpdate(const char* name, State* state, int /* arg Value* patch_data_fn = nullptr; if (ReadValueArgs(state, argv, 4, &blockdev_filename, &transfer_list_value, &new_data_fn, &patch_data_fn) < 0) { - return StringValue(strdup("")); + return StringValue(""); } - std::unique_ptr blockdev_filename_holder(blockdev_filename, - FreeValue); - std::unique_ptr transfer_list_value_holder(transfer_list_value, - FreeValue); - std::unique_ptr new_data_fn_holder(new_data_fn, FreeValue); - std::unique_ptr patch_data_fn_holder(patch_data_fn, FreeValue); + std::unique_ptr blockdev_filename_holder(blockdev_filename); + std::unique_ptr transfer_list_value_holder(transfer_list_value); + std::unique_ptr new_data_fn_holder(new_data_fn); + std::unique_ptr patch_data_fn_holder(patch_data_fn); if (blockdev_filename->type != VAL_STRING) { ErrorAbort(state, kArgsParsingFailure, "blockdev_filename argument to %s must be string", name); - return StringValue(strdup("")); + return StringValue(""); } if (transfer_list_value->type != VAL_BLOB) { ErrorAbort(state, kArgsParsingFailure, "transfer_list argument to %s must be blob", name); - return StringValue(strdup("")); + return StringValue(""); } if (new_data_fn->type != VAL_STRING) { ErrorAbort(state, kArgsParsingFailure, "new_data_fn argument to %s must be string", name); - return StringValue(strdup("")); + return StringValue(""); } if (patch_data_fn->type != VAL_STRING) { ErrorAbort(state, kArgsParsingFailure, "patch_data_fn argument to %s must be string", name); - return StringValue(strdup("")); + return StringValue(""); } UpdaterInfo* ui = reinterpret_cast(state->cookie); if (ui == nullptr) { - return StringValue(strdup("")); + return StringValue(""); } FILE* cmd_pipe = ui->cmd_pipe; ZipArchive* za = ui->package_zip; if (cmd_pipe == nullptr || za == nullptr) { - return StringValue(strdup("")); + return StringValue(""); } - const ZipEntry* patch_entry = mzFindZipEntry(za, patch_data_fn->data); + const ZipEntry* patch_entry = mzFindZipEntry(za, patch_data_fn->data.c_str()); if (patch_entry == nullptr) { - fprintf(stderr, "%s(): no file \"%s\" in package", name, patch_data_fn->data); - return StringValue(strdup("")); + fprintf(stderr, "%s(): no file \"%s\" in package", name, patch_data_fn->data.c_str()); + return StringValue(""); } params.patch_start = ui->package_zip_addr + mzGetZipEntryOffset(patch_entry); - const ZipEntry* new_entry = mzFindZipEntry(za, new_data_fn->data); + const ZipEntry* new_entry = mzFindZipEntry(za, new_data_fn->data.c_str()); if (new_entry == nullptr) { - fprintf(stderr, "%s(): no file \"%s\" in package", name, new_data_fn->data); - return StringValue(strdup("")); + fprintf(stderr, "%s(): no file \"%s\" in package", name, new_data_fn->data.c_str()); + return StringValue(""); } - params.fd.reset(TEMP_FAILURE_RETRY(ota_open(blockdev_filename->data, O_RDWR))); + params.fd.reset(TEMP_FAILURE_RETRY(ota_open(blockdev_filename->data.c_str(), O_RDWR))); if (params.fd == -1) { - fprintf(stderr, "open \"%s\" failed: %s\n", blockdev_filename->data, strerror(errno)); - return StringValue(strdup("")); + fprintf(stderr, "open \"%s\" failed: %s\n", blockdev_filename->data.c_str(), strerror(errno)); + return StringValue(""); } if (params.canwrite) { @@ -1471,24 +1467,21 @@ static Value* PerformBlockImageUpdate(const char* name, State* state, int /* arg int error = pthread_create(¶ms.thread, &attr, unzip_new_data, ¶ms.nti); if (error != 0) { fprintf(stderr, "pthread_create failed: %s\n", strerror(error)); - return StringValue(strdup("")); + return StringValue(""); } } - // Copy all the lines in transfer_list_value into std::string for - // processing. - const std::string transfer_list(transfer_list_value->data, transfer_list_value->size); - std::vector lines = android::base::Split(transfer_list, "\n"); + std::vector lines = android::base::Split(transfer_list_value->data, "\n"); if (lines.size() < 2) { ErrorAbort(state, kArgsParsingFailure, "too few lines in the transfer list [%zd]\n", lines.size()); - return StringValue(strdup("")); + return StringValue(""); } // First line in transfer list is the version number if (!android::base::ParseInt(lines[0].c_str(), ¶ms.version, 1, 4)) { fprintf(stderr, "unexpected transfer list version [%s]\n", lines[0].c_str()); - return StringValue(strdup("")); + return StringValue(""); } fprintf(stderr, "blockimg version is %d\n", params.version); @@ -1497,11 +1490,11 @@ static Value* PerformBlockImageUpdate(const char* name, State* state, int /* arg int total_blocks; if (!android::base::ParseInt(lines[1].c_str(), &total_blocks, 0)) { ErrorAbort(state, kArgsParsingFailure, "unexpected block count [%s]\n", lines[1].c_str()); - return StringValue(strdup("")); + return StringValue(""); } if (total_blocks == 0) { - return StringValue(strdup("t")); + return StringValue("t"); } size_t start = 2; @@ -1509,7 +1502,7 @@ static Value* PerformBlockImageUpdate(const char* name, State* state, int /* arg if (lines.size() < 4) { ErrorAbort(state, kArgsParsingFailure, "too few lines in the transfer list [%zu]\n", lines.size()); - return StringValue(strdup("")); + return StringValue(""); } // Third line is how many stash entries are needed simultaneously @@ -1520,12 +1513,12 @@ static Value* PerformBlockImageUpdate(const char* name, State* state, int /* arg if (!android::base::ParseInt(lines[3].c_str(), &stash_max_blocks, 0)) { ErrorAbort(state, kArgsParsingFailure, "unexpected maximum stash blocks [%s]\n", lines[3].c_str()); - return StringValue(strdup("")); + return StringValue(""); } - int res = CreateStash(state, stash_max_blocks, blockdev_filename->data, params.stashbase); + int res = CreateStash(state, stash_max_blocks, blockdev_filename->data.c_str(), params.stashbase); if (res == -1) { - return StringValue(strdup("")); + return StringValue(""); } params.createdstash = res; @@ -1589,7 +1582,7 @@ static Value* PerformBlockImageUpdate(const char* name, State* state, int /* arg fprintf(stderr, "stashed %zu blocks\n", params.stashed); fprintf(stderr, "max alloc needed was %zu\n", params.buffer.size()); - const char* partition = strrchr(blockdev_filename->data, '/'); + const char* partition = strrchr(blockdev_filename->data.c_str(), '/'); if (partition != nullptr && *(partition+1) != 0) { fprintf(cmd_pipe, "log bytes_written_%s: %zu\n", partition + 1, params.written * BLOCKSIZE); @@ -1623,7 +1616,7 @@ pbiudone: state->cause_code = failure_type; } - return StringValue(rc == 0 ? strdup("t") : strdup("")); + return StringValue(rc == 0 ? "t" : ""); } // The transfer list is a text file containing commands to @@ -1721,27 +1714,26 @@ Value* RangeSha1Fn(const char* name, State* state, int /* argc */, Expr* argv[]) Value* ranges; if (ReadValueArgs(state, argv, 2, &blockdev_filename, &ranges) < 0) { - return StringValue(strdup("")); + return StringValue(""); } - std::unique_ptr ranges_holder(ranges, FreeValue); - std::unique_ptr blockdev_filename_holder(blockdev_filename, - FreeValue); + std::unique_ptr ranges_holder(ranges); + std::unique_ptr blockdev_filename_holder(blockdev_filename); if (blockdev_filename->type != VAL_STRING) { ErrorAbort(state, kArgsParsingFailure, "blockdev_filename argument to %s must be string", name); - return StringValue(strdup("")); + return StringValue(""); } if (ranges->type != VAL_STRING) { ErrorAbort(state, kArgsParsingFailure, "ranges argument to %s must be string", name); - return StringValue(strdup("")); + return StringValue(""); } - android::base::unique_fd fd(ota_open(blockdev_filename->data, O_RDWR)); + android::base::unique_fd fd(ota_open(blockdev_filename->data.c_str(), O_RDWR)); if (fd == -1) { - ErrorAbort(state, kFileOpenFailure, "open \"%s\" failed: %s", blockdev_filename->data, - strerror(errno)); - return StringValue(strdup("")); + ErrorAbort(state, kFileOpenFailure, "open \"%s\" failed: %s", + blockdev_filename->data.c_str(), strerror(errno)); + return StringValue(""); } RangeSet rs; @@ -1753,16 +1745,16 @@ Value* RangeSha1Fn(const char* name, State* state, int /* argc */, Expr* argv[]) std::vector buffer(BLOCKSIZE); for (size_t i = 0; i < rs.count; ++i) { if (!check_lseek(fd, (off64_t)rs.pos[i*2] * BLOCKSIZE, SEEK_SET)) { - ErrorAbort(state, kLseekFailure, "failed to seek %s: %s", blockdev_filename->data, - strerror(errno)); - return StringValue(strdup("")); + ErrorAbort(state, kLseekFailure, "failed to seek %s: %s", + blockdev_filename->data.c_str(), strerror(errno)); + return StringValue(""); } for (size_t j = rs.pos[i*2]; j < rs.pos[i*2+1]; ++j) { if (read_all(fd, buffer, BLOCKSIZE) == -1) { - ErrorAbort(state, kFreadFailure, "failed to read %s: %s", blockdev_filename->data, - strerror(errno)); - return StringValue(strdup("")); + ErrorAbort(state, kFreadFailure, "failed to read %s: %s", + blockdev_filename->data.c_str(), strerror(errno)); + return StringValue(""); } SHA1_Update(&ctx, buffer.data(), BLOCKSIZE); @@ -1771,7 +1763,7 @@ Value* RangeSha1Fn(const char* name, State* state, int /* argc */, Expr* argv[]) uint8_t digest[SHA_DIGEST_LENGTH]; SHA1_Final(digest, &ctx); - return StringValue(strdup(print_sha1(digest).c_str())); + return StringValue(print_sha1(digest)); } // This function checks if a device has been remounted R/W prior to an incremental @@ -1785,27 +1777,27 @@ Value* CheckFirstBlockFn(const char* name, State* state, int argc, Expr* argv[]) if (ReadValueArgs(state, argv, 1, &arg_filename) < 0) { return nullptr; } - std::unique_ptr filename(arg_filename, FreeValue); + std::unique_ptr filename(arg_filename); if (filename->type != VAL_STRING) { ErrorAbort(state, kArgsParsingFailure, "filename argument to %s must be string", name); - return StringValue(strdup("")); + return StringValue(""); } - android::base::unique_fd fd(ota_open(arg_filename->data, O_RDONLY)); + android::base::unique_fd fd(ota_open(arg_filename->data.c_str(), O_RDONLY)); if (fd == -1) { - ErrorAbort(state, kFileOpenFailure, "open \"%s\" failed: %s", arg_filename->data, + ErrorAbort(state, kFileOpenFailure, "open \"%s\" failed: %s", arg_filename->data.c_str(), strerror(errno)); - return StringValue(strdup("")); + return StringValue(""); } RangeSet blk0 {1 /*count*/, 1/*size*/, std::vector {0, 1}/*position*/}; std::vector block0_buffer(BLOCKSIZE); if (ReadBlocks(blk0, block0_buffer, fd) == -1) { - ErrorAbort(state, kFreadFailure, "failed to read %s: %s", arg_filename->data, + ErrorAbort(state, kFreadFailure, "failed to read %s: %s", arg_filename->data.c_str(), strerror(errno)); - return StringValue(strdup("")); + return StringValue(""); } // https://ext4.wiki.kernel.org/index.php/Ext4_Disk_Layout @@ -1823,7 +1815,7 @@ Value* CheckFirstBlockFn(const char* name, State* state, int argc, Expr* argv[]) uiPrintf(state, "Last remount happened on %s", ctime(&mount_time)); } - return StringValue(strdup("t")); + return StringValue("t"); } @@ -1835,40 +1827,40 @@ Value* BlockImageRecoverFn(const char* name, State* state, int argc, Expr* argv[ return NULL; } - std::unique_ptr filename(arg_filename, FreeValue); - std::unique_ptr ranges(arg_ranges, FreeValue); + std::unique_ptr filename(arg_filename); + std::unique_ptr ranges(arg_ranges); if (filename->type != VAL_STRING) { ErrorAbort(state, kArgsParsingFailure, "filename argument to %s must be string", name); - return StringValue(strdup("")); + return StringValue(""); } if (ranges->type != VAL_STRING) { ErrorAbort(state, kArgsParsingFailure, "ranges argument to %s must be string", name); - return StringValue(strdup("")); + return StringValue(""); } // Output notice to log when recover is attempted - fprintf(stderr, "%s image corrupted, attempting to recover...\n", filename->data); + fprintf(stderr, "%s image corrupted, attempting to recover...\n", filename->data.c_str()); // When opened with O_RDWR, libfec rewrites corrupted blocks when they are read - fec::io fh(filename->data, O_RDWR); + fec::io fh(filename->data.c_str(), O_RDWR); if (!fh) { - ErrorAbort(state, kLibfecFailure, "fec_open \"%s\" failed: %s", filename->data, + ErrorAbort(state, kLibfecFailure, "fec_open \"%s\" failed: %s", filename->data.c_str(), strerror(errno)); - return StringValue(strdup("")); + return StringValue(""); } if (!fh.has_ecc() || !fh.has_verity()) { ErrorAbort(state, kLibfecFailure, "unable to use metadata to correct errors"); - return StringValue(strdup("")); + return StringValue(""); } fec_status status; if (!fh.get_status(status)) { ErrorAbort(state, kLibfecFailure, "failed to read FEC status"); - return StringValue(strdup("")); + return StringValue(""); } RangeSet rs; @@ -1885,8 +1877,8 @@ Value* BlockImageRecoverFn(const char* name, State* state, int argc, Expr* argv[ if (fh.pread(buffer, BLOCKSIZE, (off64_t)j * BLOCKSIZE) != BLOCKSIZE) { ErrorAbort(state, kLibfecFailure, "failed to recover %s (block %zu): %s", - filename->data, j, strerror(errno)); - return StringValue(strdup("")); + filename->data.c_str(), j, strerror(errno)); + return StringValue(""); } // If we want to be able to recover from a situation where rewriting a corrected @@ -1901,8 +1893,8 @@ Value* BlockImageRecoverFn(const char* name, State* state, int argc, Expr* argv[ // read and check if the errors field value has increased. } } - fprintf(stderr, "...%s image recovered successfully.\n", filename->data); - return StringValue(strdup("t")); + fprintf(stderr, "...%s image recovered successfully.\n", filename->data.c_str()); + return StringValue("t"); } void RegisterBlockImageFunctions() { diff --git a/updater/install.cpp b/updater/install.cpp index 3546968d7..d723b3880 100644 --- a/updater/install.cpp +++ b/updater/install.cpp @@ -116,7 +116,7 @@ static int make_parents(char* name) { // // fs_type="ext4" partition_type="EMMC" location=device Value* MountFn(const char* name, State* state, int argc, Expr* argv[]) { - char* result = NULL; + char* result = nullptr; if (argc != 4 && argc != 5) { return ErrorAbort(state, kArgsParsingFailure, "%s() expects 4-5 args, got %d", name, argc); } @@ -197,7 +197,7 @@ done: // is_mounted(mount_point) Value* IsMountedFn(const char* name, State* state, int argc, Expr* argv[]) { - char* result = NULL; + char* result = nullptr; if (argc != 1) { return ErrorAbort(state, kArgsParsingFailure, "%s() expects 1 arg, got %d", name, argc); } @@ -227,7 +227,7 @@ done: Value* UnmountFn(const char* name, State* state, int argc, Expr* argv[]) { - char* result = NULL; + char* result = nullptr; if (argc != 1) { return ErrorAbort(state, kArgsParsingFailure, "%s() expects 1 arg, got %d", name, argc); } @@ -284,7 +284,7 @@ static int exec_cmd(const char* path, char* const argv[]) { // if fs_size > 0, that is the size to use // if fs_size < 0, then reserve that many bytes at the end of the partition (not for "f2fs") Value* FormatFn(const char* name, State* state, int argc, Expr* argv[]) { - char* result = NULL; + char* result = nullptr; if (argc != 5) { return ErrorAbort(state, kArgsParsingFailure, "%s() expects 5 args, got %d", name, argc); } @@ -358,7 +358,7 @@ done: } Value* RenameFn(const char* name, State* state, int argc, Expr* argv[]) { - char* result = NULL; + char* result = nullptr; if (argc != 2) { return ErrorAbort(state, kArgsParsingFailure, "%s() expects 2 args, got %d", name, argc); } @@ -397,15 +397,10 @@ done: } Value* DeleteFn(const char* name, State* state, int argc, Expr* argv[]) { - char** paths = reinterpret_cast(malloc(argc * sizeof(char*))); + std::vector paths; for (int i = 0; i < argc; ++i) { - paths[i] = Evaluate(state, argv[i]); - if (paths[i] == NULL) { - for (int j = 0; j < i; ++j) { - free(paths[j]); - } - free(paths); - return NULL; + if (!Evaluate(state, argv[i], &paths[i])) { + return nullptr; } } @@ -413,15 +408,12 @@ Value* DeleteFn(const char* name, State* state, int argc, Expr* argv[]) { int success = 0; for (int i = 0; i < argc; ++i) { - if ((recursive ? dirUnlinkHierarchy(paths[i]) : unlink(paths[i])) == 0) + if ((recursive ? dirUnlinkHierarchy(paths[i].c_str()) : unlink(paths[i].c_str())) == 0) { ++success; - free(paths[i]); + } } - free(paths); - char buffer[10]; - sprintf(buffer, "%d", success); - return StringValue(strdup(buffer)); + return StringValue(android::base::StringPrintf("%d", success)); } @@ -483,7 +475,7 @@ Value* PackageExtractDirFn(const char* name, State* state, NULL, NULL, sehandle); free(zip_path); free(dest_path); - return StringValue(strdup(success ? "t" : "")); + return StringValue(success ? "t" : ""); } @@ -536,7 +528,7 @@ Value* PackageExtractFileFn(const char* name, State* state, done2: free(zip_path); free(dest_path); - return StringValue(strdup(success ? "t" : "")); + return StringValue(success ? "t" : ""); } else { // The one-argument version returns the contents of the file // as the result. @@ -544,10 +536,7 @@ Value* PackageExtractFileFn(const char* name, State* state, char* zip_path; if (ReadArgs(state, argv, 1, &zip_path) < 0) return NULL; - Value* v = reinterpret_cast(malloc(sizeof(Value))); - v->type = VAL_BLOB; - v->size = -1; - v->data = NULL; + Value* v = new Value(VAL_INVALID, ""); ZipArchive* za = ((UpdaterInfo*)(state->cookie))->package_zip; const ZipEntry* entry = mzFindZipEntry(za, zip_path); @@ -556,23 +545,16 @@ Value* PackageExtractFileFn(const char* name, State* state, goto done1; } - v->size = mzGetZipEntryUncompLen(entry); - v->data = reinterpret_cast(malloc(v->size)); - if (v->data == NULL) { - printf("%s: failed to allocate %zd bytes for %s\n", - name, v->size, zip_path); - goto done1; - } - + v->data.resize(mzGetZipEntryUncompLen(entry)); success = mzExtractZipEntryToBuffer(za, entry, - (unsigned char *)v->data); + reinterpret_cast(&v->data[0])); done1: free(zip_path); if (!success) { - free(v->data); - v->data = NULL; - v->size = -1; + v->data.clear(); + } else { + v->type = VAL_BLOB; } return v; } @@ -584,13 +566,13 @@ Value* SymlinkFn(const char* name, State* state, int argc, Expr* argv[]) { if (argc == 0) { return ErrorAbort(state, kArgsParsingFailure, "%s() expects 1+ args, got %d", name, argc); } - char* target; - target = Evaluate(state, argv[0]); - if (target == NULL) return NULL; + std::string target; + if (!Evaluate(state, argv[0], &target)) { + return nullptr; + } char** srcs = ReadVarArgs(state, argc-1, argv+1); if (srcs == NULL) { - free(target); return NULL; } @@ -606,12 +588,12 @@ Value* SymlinkFn(const char* name, State* state, int argc, Expr* argv[]) { } if (make_parents(srcs[i])) { printf("%s: failed to symlink %s to %s: making parents failed\n", - name, srcs[i], target); + name, srcs[i], target.c_str()); ++bad; } - if (symlink(target, srcs[i]) < 0) { + if (symlink(target.c_str(), srcs[i]) < 0) { printf("%s: failed to symlink %s to %s: %s\n", - name, srcs[i], target, strerror(errno)); + name, srcs[i], target.c_str(), strerror(errno)); ++bad; } free(srcs[i]); @@ -620,7 +602,7 @@ Value* SymlinkFn(const char* name, State* state, int argc, Expr* argv[]) { if (bad) { return ErrorAbort(state, kSymlinkFailure, "%s: some symlinks failed", name); } - return StringValue(strdup("")); + return StringValue(""); } struct perm_parsed_args { @@ -883,20 +865,20 @@ done: return ErrorAbort(state, kSetMetadataFailure, "%s: some changes failed", name); } - return StringValue(strdup("")); + return StringValue(""); } Value* GetPropFn(const char* name, State* state, int argc, Expr* argv[]) { if (argc != 1) { return ErrorAbort(state, kArgsParsingFailure, "%s() expects 1 arg, got %d", name, argc); } - char* key = Evaluate(state, argv[0]); - if (key == NULL) return NULL; - + std::string key; + if (!Evaluate(state, argv[0], &key)) { + return nullptr; + } std::string value = android::base::GetProperty(key, ""); - free(key); - return StringValue(strdup(value.c_str())); + return StringValue(value); } @@ -1015,7 +997,7 @@ Value* ApplyPatchSpaceFn(const char* name, State* state, return nullptr; } - return StringValue(strdup(CacheSizeCheck(bytes) ? "" : "t")); + return StringValue(CacheSizeCheck(bytes) ? "" : "t"); } // apply_patch(file, size, init_sha1, tgt_sha1, patch) @@ -1047,17 +1029,16 @@ Value* ApplyPatchFn(const char* name, State* state, int argc, Expr* argv[]) { } int patchcount = (argc-4) / 2; - std::unique_ptr arg_values(ReadValueVarArgs(state, argc-4, argv+4), - free); + std::unique_ptr arg_values(ReadValueVarArgs(state, argc-4, argv+4)); if (!arg_values) { return nullptr; } - std::vector> patch_shas; - std::vector> patches; + std::vector> patch_shas; + std::vector> patches; // Protect values by unique_ptrs first to get rid of memory leak. for (int i = 0; i < patchcount * 2; i += 2) { - patch_shas.emplace_back(arg_values.get()[i], FreeValue); - patches.emplace_back(arg_values.get()[i+1], FreeValue); + patch_shas.emplace_back(arg_values.get()[i]); + patches.emplace_back(arg_values.get()[i+1]); } for (int i = 0; i < patchcount; ++i) { @@ -1071,7 +1052,7 @@ Value* ApplyPatchFn(const char* name, State* state, int argc, Expr* argv[]) { } } - std::vector patch_sha_str; + std::vector patch_sha_str; std::vector patch_ptrs; for (int i = 0; i < patchcount; ++i) { patch_sha_str.push_back(patch_shas[i]->data); @@ -1080,9 +1061,9 @@ Value* ApplyPatchFn(const char* name, State* state, int argc, Expr* argv[]) { int result = applypatch(source_filename, target_filename, target_sha1, target_size, - patchcount, patch_sha_str.data(), patch_ptrs.data(), NULL); + patch_sha_str, patch_ptrs.data(), NULL); - return StringValue(strdup(result == 0 ? "t" : "")); + return StringValue(result == 0 ? "t" : ""); } // apply_patch_check(file, [sha1_1, ...]) @@ -1095,21 +1076,17 @@ Value* ApplyPatchCheckFn(const char* name, State* state, char* filename; if (ReadArgs(state, argv, 1, &filename) < 0) { - return NULL; + return nullptr; } - int patchcount = argc-1; - char** sha1s = ReadVarArgs(state, argc-1, argv+1); - - int result = applypatch_check(filename, patchcount, sha1s); - - int i; - for (i = 0; i < patchcount; ++i) { - free(sha1s[i]); + std::vector sha1s; + if (!ReadArgs(state, argc-1, argv+1, &sha1s)) { + return nullptr; } - free(sha1s); - return StringValue(strdup(result == 0 ? "t" : "")); + int result = applypatch_check(filename, sha1s); + + return StringValue(result == 0 ? "t" : ""); } // This is the updater side handler for ui_print() in edify script. Contents @@ -1129,7 +1106,7 @@ Value* UIPrintFn(const char* name, State* state, int argc, Expr* argv[]) { buffer += "\n"; uiPrint(state, buffer); - return StringValue(strdup(buffer.c_str())); + return StringValue(buffer); } Value* WipeCacheFn(const char* name, State* state, int argc, Expr* argv[]) { @@ -1137,7 +1114,7 @@ Value* WipeCacheFn(const char* name, State* state, int argc, Expr* argv[]) { return ErrorAbort(state, kArgsParsingFailure, "%s() expects no args, got %d", name, argc); } fprintf(((UpdaterInfo*)(state->cookie))->cmd_pipe, "wipe_cache\n"); - return StringValue(strdup("t")); + return StringValue("t"); } Value* RunProgramFn(const char* name, State* state, int argc, Expr* argv[]) { @@ -1180,10 +1157,7 @@ Value* RunProgramFn(const char* name, State* state, int argc, Expr* argv[]) { free(args); free(args2); - char buffer[20]; - sprintf(buffer, "%d", status); - - return StringValue(strdup(buffer)); + return StringValue(android::base::StringPrintf("%d", status)); } // sha1_check(data) @@ -1199,32 +1173,32 @@ Value* Sha1CheckFn(const char* name, State* state, int argc, Expr* argv[]) { return ErrorAbort(state, kArgsParsingFailure, "%s() expects at least 1 arg", name); } - std::unique_ptr arg_values(ReadValueVarArgs(state, argc, argv), free); + std::unique_ptr arg_values(ReadValueVarArgs(state, argc, argv)); if (arg_values == nullptr) { return nullptr; } - std::vector> args; + std::vector> args; for (int i = 0; i < argc; ++i) { - args.emplace_back(arg_values.get()[i], FreeValue); + args.emplace_back(arg_values.get()[i]); } - if (args[0]->size < 0) { - return StringValue(strdup("")); + if (args[0]->type == VAL_INVALID) { + return StringValue(""); } uint8_t digest[SHA_DIGEST_LENGTH]; - SHA1(reinterpret_cast(args[0]->data), args[0]->size, digest); + SHA1(reinterpret_cast(args[0]->data.c_str()), args[0]->data.size(), digest); if (argc == 1) { - return StringValue(strdup(print_sha1(digest).c_str())); + return StringValue(print_sha1(digest)); } for (int i = 1; i < argc; ++i) { uint8_t arg_digest[SHA_DIGEST_LENGTH]; if (args[i]->type != VAL_STRING) { printf("%s(): arg %d is not a string; skipping", name, i); - } else if (ParseSha1(args[i]->data, arg_digest) != 0) { + } else if (ParseSha1(args[i]->data.c_str(), arg_digest) != 0) { // Warn about bad args and skip them. - printf("%s(): error parsing \"%s\" as sha-1; skipping", name, args[i]->data); + printf("%s(): error parsing \"%s\" as sha-1; skipping", name, args[i]->data.c_str()); } else if (memcmp(digest, arg_digest, SHA_DIGEST_LENGTH) == 0) { // Found a match. return args[i].release(); @@ -1232,7 +1206,7 @@ Value* Sha1CheckFn(const char* name, State* state, int argc, Expr* argv[]) { } // Didn't match any of the hex strings; return false. - return StringValue(strdup("")); + return StringValue(""); } // Read a local file and return its contents (the Value* returned @@ -1244,21 +1218,12 @@ Value* ReadFileFn(const char* name, State* state, int argc, Expr* argv[]) { char* filename; if (ReadArgs(state, argv, 1, &filename) < 0) return NULL; - Value* v = static_cast(malloc(sizeof(Value))); - if (v == nullptr) { - return nullptr; - } - v->type = VAL_BLOB; - v->size = -1; - v->data = nullptr; + Value* v = new Value(VAL_INVALID, ""); FileContents fc; if (LoadFileContents(filename, &fc) == 0) { - v->data = static_cast(malloc(fc.data.size())); - if (v->data != nullptr) { - memcpy(v->data, fc.data.data(), fc.data.size()); - v->size = fc.data.size(); - } + v->type = VAL_BLOB; + v->data = std::string(fc.data.begin(), fc.data.end()); } free(filename); return v; @@ -1326,16 +1291,19 @@ Value* SetStageFn(const char* name, State* state, int argc, Expr* argv[]) { // package installation. FILE* f = ota_fopen(filename, "r+b"); fseek(f, offsetof(struct bootloader_message, stage), SEEK_SET); - int to_write = strlen(stagestr)+1; - int max_size = sizeof(((struct bootloader_message*)0)->stage); + size_t to_write = strlen(stagestr) + 1; + size_t max_size = sizeof(((struct bootloader_message*)0)->stage); if (to_write > max_size) { to_write = max_size; - stagestr[max_size-1] = 0; + stagestr[max_size - 1] = 0; } - ota_fwrite(stagestr, to_write, 1, f); + size_t status = ota_fwrite(stagestr, to_write, 1, f); ota_fclose(f); free(stagestr); + if (status != to_write) { + return StringValue(""); + } return StringValue(filename); } @@ -1352,11 +1320,14 @@ Value* GetStageFn(const char* name, State* state, int argc, Expr* argv[]) { char buffer[sizeof(((struct bootloader_message*)0)->stage)]; FILE* f = ota_fopen(filename, "rb"); fseek(f, offsetof(struct bootloader_message, stage), SEEK_SET); - ota_fread(buffer, sizeof(buffer), 1, f); + size_t status = ota_fread(buffer, sizeof(buffer), 1, f); ota_fclose(f); - buffer[sizeof(buffer)-1] = '\0'; + if (status != sizeof(buffer)) { + return StringValue(""); + } - return StringValue(strdup(buffer)); + buffer[sizeof(buffer)-1] = '\0'; + return StringValue(buffer); } Value* WipeBlockDeviceFn(const char* name, State* state, int argc, Expr* argv[]) { @@ -1378,7 +1349,7 @@ Value* WipeBlockDeviceFn(const char* name, State* state, int argc, Expr* argv[]) ota_close(fd); - return StringValue(strdup(success ? "t" : "")); + return StringValue(success ? "t" : ""); } Value* EnableRebootFn(const char* name, State* state, int argc, Expr* argv[]) { @@ -1387,7 +1358,7 @@ Value* EnableRebootFn(const char* name, State* state, int argc, Expr* argv[]) { } UpdaterInfo* ui = (UpdaterInfo*)(state->cookie); fprintf(ui->cmd_pipe, "enable_reboot\n"); - return StringValue(strdup("t")); + return StringValue("t"); } Value* Tune2FsFn(const char* name, State* state, int argc, Expr* argv[]) { @@ -1418,7 +1389,7 @@ Value* Tune2FsFn(const char* name, State* state, int argc, Expr* argv[]) { return ErrorAbort(state, kTune2FsFailure, "%s() returned error code %d", name, result); } - return StringValue(strdup("t")); + return StringValue("t"); } void RegisterInstallFunctions() { diff --git a/updater/updater.cpp b/updater/updater.cpp index c752ebbf3..47696b80c 100644 --- a/updater/updater.cpp +++ b/updater/updater.cpp @@ -151,13 +151,14 @@ int main(int argc, char** argv) { } } - char* result = Evaluate(&state, root); + std::string result; + bool status = Evaluate(&state, root, &result); if (have_eio_error) { fprintf(cmd_pipe, "retry_update\n"); } - if (result == NULL) { + if (!status) { if (state.errmsg.empty()) { printf("script aborted (no error message)\n"); fprintf(cmd_pipe, "ui_print script aborted (no error message)\n"); @@ -188,8 +189,7 @@ int main(int argc, char** argv) { return 7; } else { - fprintf(cmd_pipe, "ui_print script succeeded: result was [%s]\n", result); - free(result); + fprintf(cmd_pipe, "ui_print script succeeded: result was [%s]\n", result.c_str()); } if (updater_info.package_zip) { -- cgit v1.2.3