From 5ad802839d84f7bcdfbb8aa5c48cb56b303ce5f0 Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Sun, 28 Jan 2018 15:37:48 -0800 Subject: Avoid overwrite of the error message in AbortFn The AbortFn() used to overwrite the error message, hiding the real failure reported in ErrorAbort(). And we will miss the failure in the script patterns like 'blockimageupdate() || abort()' We will ensure there's one line break at the end of ErrorAbort's error message; and append to the existing error message when calling abort(). Test: Message from ErrorAbort shows up in the log Change-Id: I3aebd06629c5129330250c7fe5e8cdead2ae85bc --- edify/expr.cpp | 19 +++++++++++-------- updater/blockimg.cpp | 18 +++++++++--------- updater/install.cpp | 12 ++++++------ 3 files changed, 26 insertions(+), 23 deletions(-) diff --git a/edify/expr.cpp b/edify/expr.cpp index 1b8623f03..6823b7339 100644 --- a/edify/expr.cpp +++ b/edify/expr.cpp @@ -114,9 +114,9 @@ Value* IfElseFn(const char* name, State* state, const std::vector>& argv) { std::string msg; if (!argv.empty() && Evaluate(state, argv[0], &msg)) { - state->errmsg = msg; + state->errmsg += msg; } else { - state->errmsg = "called abort()"; + state->errmsg += "called abort()"; } return nullptr; } @@ -410,12 +410,15 @@ Value* ErrorAbort(State* state, const char* format, ...) { } Value* ErrorAbort(State* state, CauseCode cause_code, const char* format, ...) { - va_list ap; - va_start(ap, format); - android::base::StringAppendV(&state->errmsg, format, ap); - va_end(ap); - state->cause_code = cause_code; - return nullptr; + std::string err_message; + va_list ap; + va_start(ap, format); + android::base::StringAppendV(&err_message, format, ap); + va_end(ap); + // Ensure that there's exactly one line break at the end of the error message. + state->errmsg = android::base::Trim(err_message) + "\n"; + state->cause_code = cause_code; + return nullptr; } State::State(const std::string& script, void* cookie) diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp index 08f9930ea..0e90e94a1 100644 --- a/updater/blockimg.cpp +++ b/updater/blockimg.cpp @@ -817,7 +817,7 @@ static int CreateStash(State* state, size_t maxblocks, const std::string& blockd size_t max_stash_size = maxblocks * BLOCKSIZE; if (res == -1 && errno != ENOENT) { - ErrorAbort(state, kStashCreationFailure, "stat \"%s\" failed: %s\n", dirname.c_str(), + ErrorAbort(state, kStashCreationFailure, "stat \"%s\" failed: %s", dirname.c_str(), strerror(errno)); return -1; } else if (res != 0) { @@ -825,19 +825,19 @@ static int CreateStash(State* state, size_t maxblocks, const std::string& blockd res = mkdir(dirname.c_str(), STASH_DIRECTORY_MODE); if (res != 0) { - ErrorAbort(state, kStashCreationFailure, "mkdir \"%s\" failed: %s\n", dirname.c_str(), + ErrorAbort(state, kStashCreationFailure, "mkdir \"%s\" failed: %s", dirname.c_str(), strerror(errno)); return -1; } if (chown(dirname.c_str(), AID_SYSTEM, AID_SYSTEM) != 0) { // system user - ErrorAbort(state, kStashCreationFailure, "chown \"%s\" failed: %s\n", dirname.c_str(), + ErrorAbort(state, kStashCreationFailure, "chown \"%s\" failed: %s", dirname.c_str(), strerror(errno)); return -1; } if (CacheSizeCheck(max_stash_size) != 0) { - ErrorAbort(state, kStashCreationFailure, "not enough space for stash (%zu needed)\n", + ErrorAbort(state, kStashCreationFailure, "not enough space for stash (%zu needed)", max_stash_size); return -1; } @@ -869,7 +869,7 @@ static int CreateStash(State* state, size_t maxblocks, const std::string& blockd if (max_stash_size > existing) { size_t needed = max_stash_size - existing; if (CacheSizeCheck(needed) != 0) { - ErrorAbort(state, kStashCreationFailure, "not enough space for stash (%zu more needed)\n", + ErrorAbort(state, kStashCreationFailure, "not enough space for stash (%zu more needed)", needed); return -1; } @@ -1517,7 +1517,7 @@ static Value* PerformBlockImageUpdate(const char* name, State* state, 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", + ErrorAbort(state, kArgsParsingFailure, "too few lines in the transfer list [%zd]", lines.size()); return StringValue(""); } @@ -1533,7 +1533,7 @@ static Value* PerformBlockImageUpdate(const char* name, State* state, // Second line in transfer list is the total number of blocks we expect to write. size_t total_blocks; if (!android::base::ParseUint(lines[1], &total_blocks)) { - ErrorAbort(state, kArgsParsingFailure, "unexpected block count [%s]\n", lines[1].c_str()); + ErrorAbort(state, kArgsParsingFailure, "unexpected block count [%s]", lines[1].c_str()); return StringValue(""); } @@ -1543,7 +1543,7 @@ static Value* PerformBlockImageUpdate(const char* name, State* state, size_t start = 2; if (lines.size() < 4) { - ErrorAbort(state, kArgsParsingFailure, "too few lines in the transfer list [%zu]\n", + ErrorAbort(state, kArgsParsingFailure, "too few lines in the transfer list [%zu]", lines.size()); return StringValue(""); } @@ -1554,7 +1554,7 @@ static Value* PerformBlockImageUpdate(const char* name, State* state, // Fourth line is the maximum number of blocks that will be stashed simultaneously size_t stash_max_blocks; if (!android::base::ParseUint(lines[3], &stash_max_blocks)) { - ErrorAbort(state, kArgsParsingFailure, "unexpected maximum stash blocks [%s]\n", + ErrorAbort(state, kArgsParsingFailure, "unexpected maximum stash blocks [%s]", lines[3].c_str()); return StringValue(""); } diff --git a/updater/install.cpp b/updater/install.cpp index b83d30ff3..eef252e30 100644 --- a/updater/install.cpp +++ b/updater/install.cpp @@ -268,7 +268,7 @@ Value* FormatFn(const char* name, State* state, const std::vector