From 14b1e6d63dcff248fff545bff17ce9f9fc7c66ec Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Mon, 18 Sep 2017 10:41:14 -0700 Subject: [PATCH 1/2] bootstat: clang-format rebase Code style has drifted too far from our current clang-format settings. Test: compile Side-Effects: none Bug: 63736262 Change-Id: Ie390459db123cfadf09aa06c8dafb1ae69a72af8 --- bootstat/boot_event_record_store.cpp | 18 +- bootstat/boot_event_record_store.h | 4 +- bootstat/boot_event_record_store_test.cpp | 18 +- bootstat/bootstat.cpp | 280 +++++++++++----------- 4 files changed, 154 insertions(+), 166 deletions(-) diff --git a/bootstat/boot_event_record_store.cpp b/bootstat/boot_event_record_store.cpp index 99d94057c..e2a4b0403 100644 --- a/bootstat/boot_event_record_store.cpp +++ b/bootstat/boot_event_record_store.cpp @@ -58,16 +58,15 @@ BootEventRecordStore::BootEventRecordStore() { } void BootEventRecordStore::AddBootEvent(const std::string& event) { - auto uptime = std::chrono::duration_cast( - android::base::boot_clock::now().time_since_epoch()); - AddBootEventWithValue(event, uptime.count()); + auto uptime = std::chrono::duration_cast( + android::base::boot_clock::now().time_since_epoch()); + AddBootEventWithValue(event, uptime.count()); } // The implementation of AddBootEventValue makes use of the mtime file // attribute to store the value associated with a boot event in order to // optimize on-disk size requirements and small-file thrashing. -void BootEventRecordStore::AddBootEventWithValue( - const std::string& event, int32_t value) { +void BootEventRecordStore::AddBootEventWithValue(const std::string& event, int32_t value) { std::string record_path = GetBootEventPath(event); int record_fd = creat(record_path.c_str(), S_IRUSR | S_IWUSR); if (record_fd == -1) { @@ -96,8 +95,7 @@ void BootEventRecordStore::AddBootEventWithValue( close(record_fd); } -bool BootEventRecordStore::GetBootEvent( - const std::string& event, BootEventRecord* record) const { +bool BootEventRecordStore::GetBootEvent(const std::string& event, BootEventRecord* record) const { CHECK_NE(static_cast(nullptr), record); CHECK(!event.empty()); @@ -112,8 +110,7 @@ bool BootEventRecordStore::GetBootEvent( return true; } -std::vector BootEventRecordStore:: - GetAllBootEvents() const { +std::vector BootEventRecordStore::GetAllBootEvents() const { std::vector events; std::unique_ptr dir(opendir(store_path_.c_str()), closedir); @@ -147,8 +144,7 @@ void BootEventRecordStore::SetStorePath(const std::string& path) { store_path_ = path; } -std::string BootEventRecordStore::GetBootEventPath( - const std::string& event) const { +std::string BootEventRecordStore::GetBootEventPath(const std::string& event) const { DCHECK_EQ('/', store_path_.back()); return store_path_ + event; } diff --git a/bootstat/boot_event_record_store.h b/bootstat/boot_event_record_store.h index a2b83184e..f872c85c8 100644 --- a/bootstat/boot_event_record_store.h +++ b/bootstat/boot_event_record_store.h @@ -17,12 +17,12 @@ #ifndef BOOT_EVENT_RECORD_STORE_H_ #define BOOT_EVENT_RECORD_STORE_H_ +#include +#include #include #include #include #include -#include -#include // BootEventRecordStore manages the persistence of boot events to the record // store and the retrieval of all boot event records from the store. diff --git a/bootstat/boot_event_record_store_test.cpp b/bootstat/boot_event_record_store_test.cpp index d98169b87..4b7ab36ee 100644 --- a/bootstat/boot_event_record_store_test.cpp +++ b/bootstat/boot_event_record_store_test.cpp @@ -94,20 +94,16 @@ void DeleteDirectory(const std::string& path) { // Returns the time in seconds since boot. time_t GetUptimeSeconds() { - return std::chrono::duration_cast( - android::base::boot_clock::now().time_since_epoch()) - .count(); + return std::chrono::duration_cast( + android::base::boot_clock::now().time_since_epoch()) + .count(); } class BootEventRecordStoreTest : public ::testing::Test { public: - BootEventRecordStoreTest() { - store_path_ = std::string(store_dir_.path) + "/"; - } + BootEventRecordStoreTest() { store_path_ = std::string(store_dir_.path) + "/"; } - const std::string& GetStorePathForTesting() const { - return store_path_; - } + const std::string& GetStorePathForTesting() const { return store_path_; } private: void TearDown() { @@ -159,9 +155,7 @@ TEST_F(BootEventRecordStoreTest, AddMultipleBootEvents) { store.AddBootEvent("triassic"); const std::string EXPECTED_NAMES[] = { - "cretaceous", - "jurassic", - "triassic", + "cretaceous", "jurassic", "triassic", }; auto events = store.GetAllBootEvents(); diff --git a/bootstat/bootstat.cpp b/bootstat/bootstat.cpp index d4c45a248..ce28059be 100644 --- a/bootstat/bootstat.cpp +++ b/bootstat/bootstat.cpp @@ -19,8 +19,8 @@ // uploaded to Android log storage via Tron. #include -#include #include +#include #include #include @@ -61,8 +61,7 @@ void LogBootEvents() { // Records the named boot |event| to the record store. If |value| is non-empty // and is a proper string representation of an integer value, the converted // integer value is associated with the boot event. -void RecordBootEventFromCommandLine( - const std::string& event, const std::string& value_str) { +void RecordBootEventFromCommandLine(const std::string& event, const std::string& value_str) { BootEventRecordStore boot_event_store; if (!value_str.empty()) { int32_t value = 0; @@ -85,7 +84,7 @@ void PrintBootEvents() { } } -void ShowHelp(const char *cmd) { +void ShowHelp(const char* cmd) { fprintf(stderr, "Usage: %s [options]\n", cmd); fprintf(stderr, "options include:\n" @@ -101,7 +100,7 @@ void ShowHelp(const char *cmd) { // Constructs a readable, printable string from the givencommand line // arguments. -std::string GetCommandLine(int argc, char **argv) { +std::string GetCommandLine(int argc, char** argv) { std::string cmd; for (int i = 0; i < argc; ++i) { cmd += argv[i]; @@ -137,73 +136,73 @@ constexpr int32_t kUnknownBootReason = 1; // the boot_reason metric may refer to this mapping to discern the histogram // values. const std::map kBootReasonMap = { - {"unknown", kUnknownBootReason}, - {"normal", 2}, - {"recovery", 3}, - {"reboot", 4}, - {"PowerKey", 5}, - {"hard_reset", 6}, - {"kernel_panic", 7}, - {"rpm_err", 8}, - {"hw_reset", 9}, - {"tz_err", 10}, - {"adsp_err", 11}, - {"modem_err", 12}, - {"mba_err", 13}, - {"Watchdog", 14}, - {"Panic", 15}, - {"power_key", 16}, - {"power_on", 17}, - {"Reboot", 18}, - {"rtc", 19}, - {"edl", 20}, - {"oem_pon1", 21}, - {"oem_powerkey", 22}, - {"oem_unknown_reset", 23}, - {"srto: HWWDT reset SC", 24}, - {"srto: HWWDT reset platform", 25}, - {"srto: bootloader", 26}, - {"srto: kernel panic", 27}, - {"srto: kernel watchdog reset", 28}, - {"srto: normal", 29}, - {"srto: reboot", 30}, - {"srto: reboot-bootloader", 31}, - {"srto: security watchdog reset", 32}, - {"srto: wakesrc", 33}, - {"srto: watchdog", 34}, - {"srto:1-1", 35}, - {"srto:omap_hsmm", 36}, - {"srto:phy0", 37}, - {"srto:rtc0", 38}, - {"srto:touchpad", 39}, - {"watchdog", 40}, - {"watchdogr", 41}, - {"wdog_bark", 42}, - {"wdog_bite", 43}, - {"wdog_reset", 44}, - {"shutdown,", 45}, // Trailing comma is intentional. - {"shutdown,userrequested", 46}, - {"reboot,bootloader", 47}, - {"reboot,cold", 48}, - {"reboot,recovery", 49}, - {"thermal_shutdown", 50}, - {"s3_wakeup", 51}, - {"kernel_panic,sysrq", 52}, - {"kernel_panic,NULL", 53}, - {"kernel_panic,BUG", 54}, - {"bootloader", 55}, - {"cold", 56}, - {"hard", 57}, - {"warm", 58}, - {"recovery", 59}, - {"thermal-shutdown", 60}, - {"shutdown,thermal", 61}, - {"shutdown,battery", 62}, - {"reboot,ota", 63}, - {"reboot,factory_reset", 64}, - {"reboot,", 65}, - {"reboot,shell", 66}, - {"reboot,adb", 67}, + {"unknown", kUnknownBootReason}, + {"normal", 2}, + {"recovery", 3}, + {"reboot", 4}, + {"PowerKey", 5}, + {"hard_reset", 6}, + {"kernel_panic", 7}, + {"rpm_err", 8}, + {"hw_reset", 9}, + {"tz_err", 10}, + {"adsp_err", 11}, + {"modem_err", 12}, + {"mba_err", 13}, + {"Watchdog", 14}, + {"Panic", 15}, + {"power_key", 16}, + {"power_on", 17}, + {"Reboot", 18}, + {"rtc", 19}, + {"edl", 20}, + {"oem_pon1", 21}, + {"oem_powerkey", 22}, + {"oem_unknown_reset", 23}, + {"srto: HWWDT reset SC", 24}, + {"srto: HWWDT reset platform", 25}, + {"srto: bootloader", 26}, + {"srto: kernel panic", 27}, + {"srto: kernel watchdog reset", 28}, + {"srto: normal", 29}, + {"srto: reboot", 30}, + {"srto: reboot-bootloader", 31}, + {"srto: security watchdog reset", 32}, + {"srto: wakesrc", 33}, + {"srto: watchdog", 34}, + {"srto:1-1", 35}, + {"srto:omap_hsmm", 36}, + {"srto:phy0", 37}, + {"srto:rtc0", 38}, + {"srto:touchpad", 39}, + {"watchdog", 40}, + {"watchdogr", 41}, + {"wdog_bark", 42}, + {"wdog_bite", 43}, + {"wdog_reset", 44}, + {"shutdown,", 45}, // Trailing comma is intentional. + {"shutdown,userrequested", 46}, + {"reboot,bootloader", 47}, + {"reboot,cold", 48}, + {"reboot,recovery", 49}, + {"thermal_shutdown", 50}, + {"s3_wakeup", 51}, + {"kernel_panic,sysrq", 52}, + {"kernel_panic,NULL", 53}, + {"kernel_panic,BUG", 54}, + {"bootloader", 55}, + {"cold", 56}, + {"hard", 57}, + {"warm", 58}, + {"recovery", 59}, + {"thermal-shutdown", 60}, + {"shutdown,thermal", 61}, + {"shutdown,battery", 62}, + {"reboot,ota", 63}, + {"reboot,factory_reset", 64}, + {"reboot,", 65}, + {"reboot,shell", 66}, + {"reboot,adb", 67}, }; // Converts a string value representing the reason the system booted to an @@ -221,23 +220,25 @@ int32_t BootReasonStrToEnum(const std::string& boot_reason) { // Canonical list of supported primary reboot reasons. const std::vector knownReasons = { - // kernel - "watchdog", - "kernel_panic", - // strong - "recovery", // Should not happen from ro.boot.bootreason - "bootloader", // Should not happen from ro.boot.bootreason - // blunt - "cold", - "hard", - "warm", - "shutdown", // Can not happen from ro.boot.bootreason - "reboot", // Default catch-all for anything unknown + // clang-format off + // kernel + "watchdog", + "kernel_panic", + // strong + "recovery", // Should not happen from ro.boot.bootreason + "bootloader", // Should not happen from ro.boot.bootreason + // blunt + "cold", + "hard", + "warm", + "shutdown", // Can not happen from ro.boot.bootreason + "reboot", // Default catch-all for anything unknown + // clang-format on }; // Returns true if the supplied reason prefix is considered detailed enough. bool isStrongRebootReason(const std::string& r) { - for (auto &s : knownReasons) { + for (auto& s : knownReasons) { if (s == "cold") break; // Prefix defined as terminated by a nul or comma (,). if (android::base::StartsWith(r, s.c_str()) && @@ -250,7 +251,7 @@ bool isStrongRebootReason(const std::string& r) { // Returns true if the supplied reason prefix is associated with the kernel. bool isKernelRebootReason(const std::string& r) { - for (auto &s : knownReasons) { + for (auto& s : knownReasons) { if (s == "recovery") break; // Prefix defined as terminated by a nul or comma (,). if (android::base::StartsWith(r, s.c_str()) && @@ -263,7 +264,7 @@ bool isKernelRebootReason(const std::string& r) { // Returns true if the supplied reason prefix is considered known. bool isKnownRebootReason(const std::string& r) { - for (auto &s : knownReasons) { + for (auto& s : knownReasons) { // Prefix defined as terminated by a nul or comma (,). if (android::base::StartsWith(r, s.c_str()) && ((r.length() == s.length()) || (r[s.length()] == ','))) { @@ -277,7 +278,7 @@ bool isKnownRebootReason(const std::string& r) { bool isBluntRebootReason(const std::string& r) { if (isStrongRebootReason(r)) return false; - if (!isKnownRebootReason(r)) return true; // Can not support unknown as detail + if (!isKnownRebootReason(r)) return true; // Can not support unknown as detail size_t pos = 0; while ((pos = r.find(',', pos)) != std::string::npos) { @@ -285,8 +286,8 @@ bool isBluntRebootReason(const std::string& r) { std::string next(r.substr(pos)); if (next.length() == 0) break; if (next[0] == ',') continue; - if (!isKnownRebootReason(next)) return false; // Unknown subreason is good. - if (isStrongRebootReason(next)) return false; // eg: reboot,reboot + if (!isKnownRebootReason(next)) return false; // Unknown subreason is good. + if (isStrongRebootReason(next)) return false; // eg: reboot,reboot } return true; } @@ -320,8 +321,12 @@ bool addKernelPanicSubReason(const std::string& console, std::string& ret) { // std::transform Helper callback functions: // Converts a string value representing the reason the system booted to a // string complying with Android system standard reason. -char tounderline(char c) { return ::isblank(c) ? '_' : c; } -char toprintable(char c) { return ::isprint(c) ? c : '?'; } +char tounderline(char c) { + return ::isblank(c) ? '_' : c; +} +char toprintable(char c) { + return ::isprint(c) ? c : '?'; +} const char system_reboot_reason_property[] = "sys.boot.reason"; const char last_reboot_reason_property[] = LAST_REBOOT_REASON_PROPERTY; @@ -345,9 +350,9 @@ std::string BootReasonStrToReason(const std::string& boot_reason) { if (ret == "") { // Is the bootloader boot reason ro.boot.bootreason known? std::vector words(android::base::Split(reason, ",_-")); - for (auto &s : knownReasons) { + for (auto& s : knownReasons) { std::string blunt; - for (auto &r : words) { + for (auto& r : words) { if (r == s) { if (isBluntRebootReason(s)) { blunt = s; @@ -368,17 +373,17 @@ std::string BootReasonStrToReason(const std::string& boot_reason) { // sense. In an ideal world, we would require those bootloaders // to behave and follow our standards. static const std::vector> aliasReasons = { - {"watchdog", "wdog"}, - {"cold,powerkey", "powerkey"}, - {"kernel_panic", "panic"}, - {"shutdown,thermal", "thermal"}, - {"warm,s3_wakeup", "s3_wakeup"}, - {"hard,hw_reset", "hw_reset"}, - {"bootloader", ""}, + {"watchdog", "wdog"}, + {"cold,powerkey", "powerkey"}, + {"kernel_panic", "panic"}, + {"shutdown,thermal", "thermal"}, + {"warm,s3_wakeup", "s3_wakeup"}, + {"hard,hw_reset", "hw_reset"}, + {"bootloader", ""}, }; // Either the primary or alias is found _somewhere_ in the reason string. - for (auto &s : aliasReasons) { + for (auto& s : aliasReasons) { if (reason.find(s.first) != std::string::npos) { ret = s.first; break; @@ -453,7 +458,7 @@ std::string BootReasonStrToReason(const std::string& boot_reason) { if (isBluntRebootReason(ret)) { // Heuristic to determine if shutdown possibly because of a dead battery? // Really a hail-mary pass to find it in last klog content ... - static const int battery_dead_threshold = 2; // percent + static const int battery_dead_threshold = 2; // percent static const char battery[] = "healthd: battery l="; size_t pos = content.rfind(battery); // last one if (pos != std::string::npos) { @@ -462,14 +467,14 @@ std::string BootReasonStrToReason(const std::string& boot_reason) { if (level <= battery_dead_threshold) { ret = "shutdown,battery"; } - } else { // Most likely + } else { // Most likely // Content buffer no longer will have console data. Beware if more // checks added below, that depend on parsing console content. content = ""; LOG(DEBUG) << "Can not find last low battery in last console messages"; android_logcat_context ctx = create_android_logcat(); - FILE *fp = android_logcat_popen(&ctx, "logcat -b kernel -v brief -d"); + FILE* fp = android_logcat_popen(&ctx, "logcat -b kernel -v brief -d"); if (fp != nullptr) { android::base::ReadFdToString(fileno(fp), &content); } @@ -482,7 +487,7 @@ std::string BootReasonStrToReason(const std::string& boot_reason) { // Service logd.klog not running, go to smaller buffer in the kernel. int rc = klogctl(KLOG_SIZE_BUFFER, nullptr, 0); if (rc > 0) { - ssize_t len = rc + 1024; // 1K Margin should it grow between calls. + ssize_t len = rc + 1024; // 1K Margin should it grow between calls. std::unique_ptr buf(new char[len]); rc = klogctl(KLOG_READ_ALL, buf.get(), len); if (rc < len) { @@ -494,7 +499,7 @@ std::string BootReasonStrToReason(const std::string& boot_reason) { match = battery; } - pos = content.find(match); // The first one it finds. + pos = content.find(match); // The first one it finds. if (pos != std::string::npos) { pos += strlen(match); int level = atoi(content.substr(pos).c_str()); @@ -580,7 +585,7 @@ std::string CalculateBootCompletePrefix() { if (!boot_event_store.GetBootEvent(kBuildDateKey, &record)) { boot_complete_prefix = "factory_reset_" + boot_complete_prefix; boot_event_store.AddBootEventWithValue(kBuildDateKey, build_date); - LOG(INFO) << "Canonical boot reason: " << "reboot,factory_reset"; + LOG(INFO) << "Canonical boot reason: reboot,factory_reset"; SetProperty(system_reboot_reason_property, "reboot,factory_reset"); if (GetProperty(bootloader_reboot_reason_property) == "") { SetProperty(bootloader_reboot_reason_property, "reboot,factory_reset"); @@ -588,7 +593,7 @@ std::string CalculateBootCompletePrefix() { } else if (build_date != record.second) { boot_complete_prefix = "ota_" + boot_complete_prefix; boot_event_store.AddBootEventWithValue(kBuildDateKey, build_date); - LOG(INFO) << "Canonical boot reason: " << "reboot,ota"; + LOG(INFO) << "Canonical boot reason: reboot,ota"; SetProperty(system_reboot_reason_property, "reboot,ota"); if (GetProperty(bootloader_reboot_reason_property) == "") { SetProperty(bootloader_reboot_reason_property, "reboot,ota"); @@ -599,8 +604,7 @@ std::string CalculateBootCompletePrefix() { } // Records the value of a given ro.boottime.init property in milliseconds. -void RecordInitBootTimeProp( - BootEventRecordStore* boot_event_store, const char* property) { +void RecordInitBootTimeProp(BootEventRecordStore* boot_event_store, const char* property) { std::string value = GetProperty(property); int32_t time_in_ms; @@ -685,10 +689,8 @@ void RecordBootComplete() { if (boot_event_store.GetBootEvent("last_boot_time_utc", &record)) { time_t last_boot_time_utc = record.second; - time_t time_since_last_boot = difftime(current_time_utc, - last_boot_time_utc); - boot_event_store.AddBootEventWithValue("time_since_last_boot", - time_since_last_boot); + time_t time_since_last_boot = difftime(current_time_utc, last_boot_time_utc); + boot_event_store.AddBootEventWithValue("time_since_last_boot", time_since_last_boot); } boot_event_store.AddBootEventWithValue("last_boot_time_utc", current_time_utc); @@ -714,8 +716,7 @@ void RecordBootComplete() { boot_event_store.AddBootEventWithValue(boot_complete_prefix + "_post_decrypt", boot_complete.count()); } else { - boot_event_store.AddBootEventWithValue(boot_complete_prefix + "_no_encryption", - uptime.count()); + boot_event_store.AddBootEventWithValue(boot_complete_prefix + "_no_encryption", uptime.count()); } // Record the total time from device startup to boot complete, regardless of @@ -738,8 +739,7 @@ void RecordBootComplete() { void RecordBootReason() { const std::string reason(GetProperty(bootloader_reboot_reason_property)); android::metricslogger::LogMultiAction(android::metricslogger::ACTION_BOOT, - android::metricslogger::FIELD_PLATFORM_REASON, - reason); + android::metricslogger::FIELD_PLATFORM_REASON, reason); // Log the raw bootloader_boot_reason property value. int32_t boot_reason = BootReasonStrToEnum(reason); @@ -769,21 +769,20 @@ void RecordFactoryReset() { if (current_time_utc < 0) { // UMA does not display negative values in buckets, so convert to positive. - android::metricslogger::LogHistogram( - "factory_reset_current_time_failure", std::abs(current_time_utc)); + android::metricslogger::LogHistogram("factory_reset_current_time_failure", + std::abs(current_time_utc)); // Logging via BootEventRecordStore to see if using android::metricslogger::LogHistogram // is losing records somehow. - boot_event_store.AddBootEventWithValue( - "factory_reset_current_time_failure", std::abs(current_time_utc)); + boot_event_store.AddBootEventWithValue("factory_reset_current_time_failure", + std::abs(current_time_utc)); return; } else { android::metricslogger::LogHistogram("factory_reset_current_time", current_time_utc); // Logging via BootEventRecordStore to see if using android::metricslogger::LogHistogram // is losing records somehow. - boot_event_store.AddBootEventWithValue( - "factory_reset_current_time", current_time_utc); + boot_event_store.AddBootEventWithValue("factory_reset_current_time", current_time_utc); } // The factory_reset boot event does not exist after the device is reset, so @@ -803,18 +802,15 @@ void RecordFactoryReset() { // Logging via BootEventRecordStore to see if using android::metricslogger::LogHistogram // is losing records somehow. - boot_event_store.AddBootEventWithValue( - "factory_reset_record_value", factory_reset_utc); + boot_event_store.AddBootEventWithValue("factory_reset_record_value", factory_reset_utc); - time_t time_since_factory_reset = difftime(current_time_utc, - factory_reset_utc); - boot_event_store.AddBootEventWithValue("time_since_factory_reset", - time_since_factory_reset); + time_t time_since_factory_reset = difftime(current_time_utc, factory_reset_utc); + boot_event_store.AddBootEventWithValue("time_since_factory_reset", time_since_factory_reset); } } // namespace -int main(int argc, char **argv) { +int main(int argc, char** argv) { android::base::InitLogging(argv); const std::string cmd_line = GetCommandLine(argc, argv); @@ -826,15 +822,17 @@ int main(int argc, char **argv) { static const char boot_reason_str[] = "record_boot_reason"; static const char factory_reset_str[] = "record_time_since_factory_reset"; static const struct option long_options[] = { - { "help", no_argument, NULL, 'h' }, - { "log", no_argument, NULL, 'l' }, - { "print", no_argument, NULL, 'p' }, - { "record", required_argument, NULL, 'r' }, - { value_str, required_argument, NULL, 0 }, - { boot_complete_str, no_argument, NULL, 0 }, - { boot_reason_str, no_argument, NULL, 0 }, - { factory_reset_str, no_argument, NULL, 0 }, - { NULL, 0, NULL, 0 } + // clang-format off + { "help", no_argument, NULL, 'h' }, + { "log", no_argument, NULL, 'l' }, + { "print", no_argument, NULL, 'p' }, + { "record", required_argument, NULL, 'r' }, + { value_str, required_argument, NULL, 0 }, + { boot_complete_str, no_argument, NULL, 0 }, + { boot_reason_str, no_argument, NULL, 0 }, + { factory_reset_str, no_argument, NULL, 0 }, + { NULL, 0, NULL, 0 } + // clang-format on }; std::string boot_event; From a16e437e9d86e3e2b01099a75fd919d4902363fa Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Wed, 20 Sep 2017 08:36:12 -0700 Subject: [PATCH 2/2] bootstat: validate last kmsg and bootreason content more carefully Establish a tighter trust relationship with digitization of the battery level. Validate the reboot reason collected from the last kmsg. Validate the last reboot reason before using for enhancing system reboot reason. Test: system/core/bootstat/boot_reason_test.sh Bug: 63636262 Change-Id: I45fbac4a33c09295cda89de3b73c93a884033e3b --- bootstat/boot_reason_test.sh | 35 ++++++++++++++++++++++++++++++----- bootstat/bootstat.cpp | 36 +++++++++++++++++++++++++++++------- 2 files changed, 59 insertions(+), 12 deletions(-) diff --git a/bootstat/boot_reason_test.sh b/bootstat/boot_reason_test.sh index 4314271da..b76011a04 100755 --- a/bootstat/boot_reason_test.sh +++ b/bootstat/boot_reason_test.sh @@ -370,6 +370,7 @@ test_ota() { fast and fake (touch build_date on device to make it different)" ] test_optional_ota() { echo "INFO: expected duration of ${TEST} test about 45 seconds" >&2 + echo "WARNING: ${TEST} requires userdebug build" >&2 adb shell su root touch /data/misc/bootstat/build_date >&2 adb reboot ota wait_for_screen @@ -425,6 +426,7 @@ Decision to rummage through bootstat data files was made as a _real_ factory_reset is too destructive to the device." ] test_factory_reset() { echo "INFO: expected duration of ${TEST} test roughly 45 seconds" >&2 + echo "WARNING: ${TEST} requires userdebug build" >&2 adb shell su root rm /data/misc/bootstat/build_date >&2 adb reboot >&2 wait_for_screen @@ -479,7 +481,8 @@ test_hard() { [ "USAGE: test_battery battery test (trick): -- echo healthd: battery l=2 | adb shell su root tee /dev/kmsg ; adb reboot cold +- echo healthd: battery l=2 | adb shell su root tee /dev/kmsg +- adb reboot cold - (wait until screen is up, boot has completed) - adb shell getprop sys.boot.reason - NB: should report reboot,battery, unless healthd managed to log @@ -491,7 +494,7 @@ battery test (trick): + setprop logd.kernel false + rm /sys/fs/pstore/console-ramoops + rm /sys/fs/pstore/console-ramoops-0 - + write /dev/kmsg \"healthd: battery l=2 + + write /dev/kmsg \"healthd: battery l=2${SPACE} +\" - adb reboot fs - (wait until screen is up, boot has completed) @@ -500,9 +503,10 @@ battery test (trick): - (replace set logd.kernel true to the above, and retry test)" ] test_battery() { echo "INFO: expected duration of ${TEST} test roughly two minutes" >&2 + echo "WARNING: ${TEST} requires userdebug build" >&2 # Send it _many_ times to combat devices with flakey pstore for i in a b c d e f g h i j k l m n o p q r s t u v w x y z; do - echo healthd: battery l=2 | adb shell su root tee /dev/kmsg >/dev/null + echo 'healthd: battery l=2 ' | adb shell su root tee /dev/kmsg >/dev/null done adb reboot cold >&2 adb wait-for-device @@ -512,11 +516,11 @@ test_battery() { /proc/fs/pstore/console-ramoops-0 2>/dev/null | grep 'healthd: battery l=' | tail -1 | - grep 'healthd: battery l=2' >/dev/null || ( + grep 'healthd: battery l=2 ' >/dev/null || ( if ! EXPECT_PROPERTY sys.boot.reason reboot,battery >/dev/null 2>/dev/null; then # retry for i in a b c d e f g h i j k l m n o p q r s t u v w x y z; do - echo healthd: battery l=2 | adb shell su root tee /dev/kmsg >/dev/null + echo 'healthd: battery l=2 ' | adb shell su root tee /dev/kmsg >/dev/null done adb reboot cold >&2 adb wait-for-device @@ -550,6 +554,7 @@ kernel_panic test: - NB: should report kernel_panic,sysrq" ] test_kernel_panic() { echo "INFO: expected duration of ${TEST} test > 2 minutes" >&2 + echo "WARNING: ${TEST} requires userdebug build" >&2 echo c | adb shell su root tee /proc/sysrq-trigger >/dev/null wait_for_screen EXPECT_PROPERTY sys.boot.reason kernel_panic,sysrq @@ -640,6 +645,26 @@ test_adb_reboot() { report_bootstat_logs reboot,adb } +[ "USAGE: test_Its_Just_So_Hard_reboot + +Its Just So Hard reboot test: +- adb shell reboot 'Its Just So Hard' +- (wait until screen is up, boot has completed) +- adb shell getprop sys.boot.reason +- NB: should report reboot,its_just_so_hard +- NB: expect log \"... I bootstat: Unknown boot reason: reboot,its_just_so_hard\"" ] +test_Its_Just_So_Hard_reboot() { + echo "INFO: expected duration of ${TEST} test roughly 45 seconds" >&2 + echo "INFO: ${TEST} cleanup requires userdebug build" >&2 + adb shell 'reboot "Its Just So Hard"' + wait_for_screen + EXPECT_PROPERTY sys.boot.reason reboot,its_just_so_hard + EXPECT_PROPERTY persist.sys.boot.reason "reboot,Its Just So Hard" + adb shell su root setprop persist.sys.boot.reason reboot,its_just_so_hard + EXPECT_PROPERTY persist.sys.boot.reason reboot,its_just_so_hard + report_bootstat_logs reboot,its_just_so_hard +} + [ "USAGE: ${0##*/} [-s SERIAL] [tests] Mainline executive to run the above tests" ] diff --git a/bootstat/bootstat.cpp b/bootstat/bootstat.cpp index ce28059be..e2202fdef 100644 --- a/bootstat/bootstat.cpp +++ b/bootstat/bootstat.cpp @@ -334,6 +334,8 @@ const char bootloader_reboot_reason_property[] = "ro.boot.bootreason"; // Scrub, Sanitize, Standardize and Enhance the boot reason string supplied. std::string BootReasonStrToReason(const std::string& boot_reason) { + static const size_t max_reason_length = 256; + std::string ret(GetProperty(system_reboot_reason_property)); std::string reason(boot_reason); // If sys.boot.reason == ro.boot.bootreason, let's re-evaluate @@ -428,9 +430,15 @@ std::string BootReasonStrToReason(const std::string& boot_reason) { size_t pos = content.rfind(cmd); if (pos != std::string::npos) { pos += strlen(cmd); - std::string subReason(content.substr(pos)); - pos = subReason.find('\''); - if (pos != std::string::npos) subReason.erase(pos); + std::string subReason(content.substr(pos, max_reason_length)); + for (pos = 0; pos < subReason.length(); ++pos) { + char c = tounderline(subReason[pos]); + if (!::isprint(c) || (c == '\'')) { + subReason.erase(pos); + break; + } + subReason[pos] = ::tolower(c); + } if (subReason != "") { // Will not land "reboot" as that is too blunt. if (isKernelRebootReason(subReason)) { ret = "reboot," + subReason; // User space can't talk kernel reasons. @@ -461,13 +469,20 @@ std::string BootReasonStrToReason(const std::string& boot_reason) { static const int battery_dead_threshold = 2; // percent static const char battery[] = "healthd: battery l="; size_t pos = content.rfind(battery); // last one + std::string digits; if (pos != std::string::npos) { - int level = atoi(content.substr(pos + strlen(battery)).c_str()); + digits = content.substr(pos + strlen(battery)); + } + char* endptr = NULL; + unsigned long long level = strtoull(digits.c_str(), &endptr, 10); + if ((level <= 100) && (endptr != digits.c_str()) && (*endptr == ' ')) { LOG(INFO) << "Battery level at shutdown " << level << "%"; if (level <= battery_dead_threshold) { ret = "shutdown,battery"; } - } else { // Most likely + } else { // Most likely + digits = ""; // reset digits + // Content buffer no longer will have console data. Beware if more // checks added below, that depend on parsing console content. content = ""; @@ -501,8 +516,11 @@ std::string BootReasonStrToReason(const std::string& boot_reason) { pos = content.find(match); // The first one it finds. if (pos != std::string::npos) { - pos += strlen(match); - int level = atoi(content.substr(pos).c_str()); + digits = content.substr(pos + strlen(match)); + } + endptr = NULL; + level = strtoull(digits.c_str(), &endptr, 10); + if ((level <= 100) && (endptr != digits.c_str()) && (*endptr == ' ')) { LOG(INFO) << "Battery level at startup " << level << "%"; if (level <= battery_dead_threshold) { ret = "shutdown,battery"; @@ -518,6 +536,10 @@ std::string BootReasonStrToReason(const std::string& boot_reason) { // Content buffer no longer will have console data. Beware if more // checks added below, that depend on parsing console content. content = GetProperty(last_reboot_reason_property); + // Cleanup last_boot_reason regarding acceptable character set + std::transform(content.begin(), content.end(), content.begin(), ::tolower); + std::transform(content.begin(), content.end(), content.begin(), tounderline); + std::transform(content.begin(), content.end(), content.begin(), toprintable); // String is either "reboot," or "shutdown,". // We will set if default reasons, only override with detail if thermal.