From 511d75962789837231459e53e86eaaa6a1ff6e06 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Tue, 19 Jun 2018 15:56:49 -0700 Subject: edify: Remove VAL_INVALID and move ValueType into Value class. Test: mmma -j bootable/recovery Test: Run recovery_component_test and recovery_unit_test on marlin. Change-Id: I4b240e3e771c387b9694be9c0f2f74e0265ab4cb --- applypatch/applypatch.cpp | 2 +- applypatch/applypatch_modes.cpp | 77 ++++++++++++++++++++-------------------- applypatch/imgpatch.cpp | 14 ++++---- edify/expr.cpp | 8 ++--- edify/include/edify/expr.h | 19 +++++----- tests/component/updater_test.cpp | 4 +-- updater/blockimg.cpp | 21 +++++------ updater/install.cpp | 22 ++++++------ 8 files changed, 82 insertions(+), 85 deletions(-) diff --git a/applypatch/applypatch.cpp b/applypatch/applypatch.cpp index fc29493b4..b1f5607a6 100644 --- a/applypatch/applypatch.cpp +++ b/applypatch/applypatch.cpp @@ -553,7 +553,7 @@ int applypatch_flash(const char* source_filename, const char* target_filename, static int GenerateTarget(const FileContents& source_file, const std::unique_ptr& patch, const std::string& target_filename, const uint8_t target_sha1[SHA_DIGEST_LENGTH], const Value* bonus_data) { - if (patch->type != VAL_BLOB) { + if (patch->type != Value::Type::BLOB) { LOG(ERROR) << "patch is not a blob"; return 1; } diff --git a/applypatch/applypatch_modes.cpp b/applypatch/applypatch_modes.cpp index 6437e1be6..ec95325fc 100644 --- a/applypatch/applypatch_modes.cpp +++ b/applypatch/applypatch_modes.cpp @@ -81,52 +81,51 @@ static int FlashMode(const char* src_filename, const char* tgt_filename, } static int PatchMode(int argc, const char** argv) { - FileContents bonusFc; - Value bonus(VAL_INVALID, ""); - - if (argc >= 3 && strcmp(argv[1], "-b") == 0) { - if (LoadFileContents(argv[2], &bonusFc) != 0) { - LOG(ERROR) << "Failed to load bonus file " << argv[2]; - return 1; - } - bonus.type = VAL_BLOB; - bonus.data = std::string(bonusFc.data.cbegin(), bonusFc.data.cend()); - argc -= 2; - argv += 2; - } - - if (argc < 4) { - return 2; - } - - size_t target_size; - if (!android::base::ParseUint(argv[4], &target_size) || target_size == 0) { - LOG(ERROR) << "Failed to parse \"" << argv[4] << "\" as byte count"; + std::unique_ptr bonus; + if (argc >= 3 && strcmp(argv[1], "-b") == 0) { + FileContents bonus_fc; + if (LoadFileContents(argv[2], &bonus_fc) != 0) { + LOG(ERROR) << "Failed to load bonus file " << argv[2]; return 1; } + bonus = std::make_unique(Value::Type::BLOB, + std::string(bonus_fc.data.cbegin(), bonus_fc.data.cend())); + argc -= 2; + argv += 2; + } - // If no : is provided, it is in flash mode. - if (argc == 5) { - if (bonus.type != VAL_INVALID) { - LOG(ERROR) << "bonus file not supported in flash mode"; - return 1; - } - return FlashMode(argv[1], argv[2], argv[3], target_size); - } + if (argc < 4) { + return 2; + } - std::vector sha1s; - std::vector files; - if (!ParsePatchArgs(argc-5, argv+5, &sha1s, &files)) { - LOG(ERROR) << "Failed to parse patch args"; + size_t target_size; + if (!android::base::ParseUint(argv[4], &target_size) || target_size == 0) { + LOG(ERROR) << "Failed to parse \"" << argv[4] << "\" as byte count"; + return 1; + } + + // If no : is provided, it is in flash mode. + if (argc == 5) { + if (bonus) { + LOG(ERROR) << "bonus file not supported in flash mode"; return 1; } + return FlashMode(argv[1], argv[2], argv[3], target_size); + } - std::vector> patches; - for (size_t i = 0; i < files.size(); ++i) { - patches.push_back(std::make_unique( - VAL_BLOB, std::string(files[i].data.cbegin(), files[i].data.cend()))); - } - return applypatch(argv[1], argv[2], argv[3], target_size, sha1s, patches, &bonus); + std::vector sha1s; + std::vector files; + if (!ParsePatchArgs(argc - 5, argv + 5, &sha1s, &files)) { + LOG(ERROR) << "Failed to parse patch args"; + return 1; + } + + std::vector> patches; + for (const auto& file : files) { + patches.push_back(std::make_unique(Value::Type::BLOB, + std::string(file.data.cbegin(), file.data.cend()))); + } + return applypatch(argv[1], argv[2], argv[3], target_size, sha1s, patches, bonus.get()); } // This program (applypatch) applies binary patches to files in a way that diff --git a/applypatch/imgpatch.cpp b/applypatch/imgpatch.cpp index 2f8f4851d..da7569219 100644 --- a/applypatch/imgpatch.cpp +++ b/applypatch/imgpatch.cpp @@ -151,7 +151,8 @@ static bool ApplyBSDiffPatchAndStreamOutput(const uint8_t* src_data, size_t src_ int ApplyImagePatch(const unsigned char* old_data, size_t old_size, const unsigned char* patch_data, size_t patch_size, SinkFn sink) { - Value patch(VAL_BLOB, std::string(reinterpret_cast(patch_data), patch_size)); + Value patch(Value::Type::BLOB, + std::string(reinterpret_cast(patch_data), patch_size)); return ApplyImagePatch(old_data, old_size, patch, sink, nullptr); } @@ -246,11 +247,10 @@ int ApplyImagePatch(const unsigned char* old_data, size_t old_size, const Value& // Decompress the source data; the chunk header tells us exactly // how big we expect it to be when decompressed. - // Note: expanded_len will include the bonus data size if - // 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->data.size() : 0; + // Note: expanded_len will include the bonus data size if 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 != nullptr) ? bonus_data->data.size() : 0; std::vector expanded_source(expanded_len); @@ -288,7 +288,7 @@ int ApplyImagePatch(const unsigned char* old_data, size_t old_size, const Value& inflateEnd(&strm); if (bonus_size) { - memcpy(expanded_source.data() + (expanded_len - bonus_size), &bonus_data->data[0], + memcpy(expanded_source.data() + (expanded_len - bonus_size), bonus_data->data.data(), bonus_size); } } diff --git a/edify/expr.cpp b/edify/expr.cpp index 6823b7339..c090eb28a 100644 --- a/edify/expr.cpp +++ b/edify/expr.cpp @@ -51,9 +51,9 @@ bool Evaluate(State* state, const std::unique_ptr& expr, std::string* resu if (!v) { return false; } - if (v->type != VAL_STRING) { - ErrorAbort(state, kArgsParsingFailure, "expecting string, got value type %d", v->type); - return false; + if (v->type != Value::Type::STRING) { + ErrorAbort(state, kArgsParsingFailure, "expecting string, got value type %d", v->type); + return false; } *result = v->data; @@ -68,7 +68,7 @@ Value* StringValue(const char* str) { if (str == nullptr) { return nullptr; } - return new Value(VAL_STRING, str); + return new Value(Value::Type::STRING, str); } Value* StringValue(const std::string& str) { diff --git a/edify/include/edify/expr.h b/edify/include/edify/expr.h index 770d1cf0d..ccd200778 100644 --- a/edify/include/edify/expr.h +++ b/edify/include/edify/expr.h @@ -53,19 +53,16 @@ struct State { bool is_retry = false; }; -enum ValueType { - VAL_INVALID = -1, - VAL_STRING = 1, - VAL_BLOB = 2, -}; - struct Value { - ValueType type; - std::string data; + enum class Type { + STRING = 1, + BLOB = 2, + }; + + Value(Type type, const std::string& str) : type(type), data(str) {} - Value(ValueType type, const std::string& str) : - type(type), - data(str) {} + Type type; + std::string data; }; struct Expr; diff --git a/tests/component/updater_test.cpp b/tests/component/updater_test.cpp index f50e861b0..8e520f337 100644 --- a/tests/component/updater_test.cpp +++ b/tests/component/updater_test.cpp @@ -149,11 +149,11 @@ static Value* BlobToString(const char* name, State* state, return nullptr; } - if (args[0]->type != VAL_BLOB) { + if (args[0]->type != Value::Type::BLOB) { return ErrorAbort(state, kArgsParsingFailure, "%s() expects a BLOB argument", name); } - args[0]->type = VAL_STRING; + args[0]->type = Value::Type::STRING; return args[0].release(); } diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp index fc713859b..6a6236b1b 100644 --- a/updater/blockimg.cpp +++ b/updater/blockimg.cpp @@ -1403,7 +1403,8 @@ static int PerformCommandDiff(CommandParameters& params) { if (status == 0) { LOG(INFO) << "patching " << blocks << " blocks to " << tgt.blocks(); Value patch_value( - VAL_BLOB, std::string(reinterpret_cast(params.patch_start + offset), len)); + Value::Type::BLOB, + std::string(reinterpret_cast(params.patch_start + offset), len)); RangeSinkWriter writer(params.fd, tgt); if (params.cmdname[0] == 'i') { // imgdiff @@ -1531,19 +1532,19 @@ static Value* PerformBlockImageUpdate(const char* name, State* state, const std::unique_ptr& new_data_fn = args[2]; const std::unique_ptr& patch_data_fn = args[3]; - if (blockdev_filename->type != VAL_STRING) { + if (blockdev_filename->type != Value::Type::STRING) { ErrorAbort(state, kArgsParsingFailure, "blockdev_filename argument to %s must be string", name); return StringValue(""); } - if (transfer_list_value->type != VAL_BLOB) { + if (transfer_list_value->type != Value::Type::BLOB) { ErrorAbort(state, kArgsParsingFailure, "transfer_list argument to %s must be blob", name); return StringValue(""); } - if (new_data_fn->type != VAL_STRING) { + if (new_data_fn->type != Value::Type::STRING) { ErrorAbort(state, kArgsParsingFailure, "new_data_fn argument to %s must be string", name); return StringValue(""); } - if (patch_data_fn->type != VAL_STRING) { + if (patch_data_fn->type != Value::Type::STRING) { ErrorAbort(state, kArgsParsingFailure, "patch_data_fn argument to %s must be string", name); return StringValue(""); } @@ -1944,11 +1945,11 @@ Value* RangeSha1Fn(const char* name, State* state, const std::vector& blockdev_filename = args[0]; const std::unique_ptr& ranges = args[1]; - if (blockdev_filename->type != VAL_STRING) { + if (blockdev_filename->type != Value::Type::STRING) { ErrorAbort(state, kArgsParsingFailure, "blockdev_filename argument to %s must be string", name); return StringValue(""); } - if (ranges->type != VAL_STRING) { + if (ranges->type != Value::Type::STRING) { ErrorAbort(state, kArgsParsingFailure, "ranges argument to %s must be string", name); return StringValue(""); } @@ -2010,7 +2011,7 @@ Value* CheckFirstBlockFn(const char* name, State* state, const std::unique_ptr& arg_filename = args[0]; - if (arg_filename->type != VAL_STRING) { + if (arg_filename->type != Value::Type::STRING) { ErrorAbort(state, kArgsParsingFailure, "filename argument to %s must be string", name); return StringValue(""); } @@ -2065,11 +2066,11 @@ Value* BlockImageRecoverFn(const char* name, State* state, const std::unique_ptr& filename = args[0]; const std::unique_ptr& ranges = args[1]; - if (filename->type != VAL_STRING) { + if (filename->type != Value::Type::STRING) { ErrorAbort(state, kArgsParsingFailure, "filename argument to %s must be string", name); return StringValue(""); } - if (ranges->type != VAL_STRING) { + if (ranges->type != Value::Type::STRING) { ErrorAbort(state, kArgsParsingFailure, "ranges argument to %s must be string", name); return StringValue(""); } diff --git a/updater/install.cpp b/updater/install.cpp index 9d108e0ed..02a6fe7c5 100644 --- a/updater/install.cpp +++ b/updater/install.cpp @@ -191,7 +191,7 @@ Value* PackageExtractFileFn(const char* name, State* state, zip_path.c_str(), buffer.size(), ErrorCodeString(ret)); } - return new Value(VAL_BLOB, buffer); + return new Value(Value::Type::BLOB, buffer); } } @@ -238,10 +238,10 @@ Value* ApplyPatchFn(const char* name, State* state, } for (int i = 0; i < patchcount; ++i) { - if (arg_values[i * 2]->type != VAL_STRING) { + if (arg_values[i * 2]->type != Value::Type::STRING) { return ErrorAbort(state, kArgsParsingFailure, "%s(): sha-1 #%d is not string", name, i * 2); } - if (arg_values[i * 2 + 1]->type != VAL_BLOB) { + if (arg_values[i * 2 + 1]->type != Value::Type::BLOB) { return ErrorAbort(state, kArgsParsingFailure, "%s(): patch #%d is not blob", name, i * 2 + 1); } } @@ -741,8 +741,8 @@ Value* RunProgramFn(const char* name, State* state, const std::vector>& argv) { if (argv.size() != 1) { return ErrorAbort(state, kArgsParsingFailure, "%s() expects 1 arg, got %zu", name, argv.size()); @@ -750,18 +750,18 @@ Value* ReadFileFn(const char* name, State* state, const std::vector args; if (!ReadArgs(state, argv, &args)) { - return ErrorAbort(state, kArgsParsingFailure, "%s() Failed to parse the argument(s)", name); + return ErrorAbort(state, kArgsParsingFailure, "%s(): Failed to parse the argument(s)", name); } const std::string& filename = args[0]; - Value* v = new Value(VAL_INVALID, ""); - FileContents fc; if (LoadFileContents(filename.c_str(), &fc) == 0) { - v->type = VAL_BLOB; - v->data = std::string(fc.data.begin(), fc.data.end()); + return new Value(Value::Type::BLOB, std::string(fc.data.cbegin(), fc.data.cend())); } - return v; + + // Leave it to caller to handle the failure. + LOG(ERROR) << name << ": Failed to read " << filename; + return StringValue(""); } // write_value(value, filename) -- cgit v1.2.3